[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55662#1338766 , @ahatanak wrote:

> In D55662#1337141 , @rjmccall wrote:
>
> > In D55662#1336835 , @ahatanak 
> > wrote:
> >
> > > In D55662#1335773 , @rjmccall 
> > > wrote:
> > >
> > > > Okay.  You may need to push an unevaluated context when doing that.
> > >
> > >
> > > Since I'm just moving the call to `CheckPlaceholderExpr` to the call 
> > > site, I don't think I have to push an unevaluated context there?
> >
> >
> > Hmm.  Right, for the `auto` inference specifically it's fine because the 
> > expression is in fact evaluated: we're not just eliminating placeholders in 
> > order to resolve `decltype`, we're eliminating placeholders to actually 
> > figure out what's going on with the initialization.
>
>
> clang currently diagnose the repeated use of weak in the following case (with 
> or without this patch):
>
>   auto __weak wp = b.weakProp; 
>
>
> I find this counterintuitive, but I guess this is the expected behavior?


No, that's not right. it's not a repeated use of the same weak entity.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55662



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3783
+parameters or local variables in ARC mode. When applied to parameters, it 
causes
+clang to omit the implicit calls to objc_retain and objc_release on function
+entry and exit respectivly. When applied to local variables, it causes clang to

These function names should be `typeset in mono`.



Comment at: clang/include/clang/Basic/AttrDocs.td:3784
+clang to omit the implicit calls to objc_retain and objc_release on function
+entry and exit respectivly. When applied to local variables, it causes clang to
+omit the call to retain on variable initialization, and release when the

Typo



Comment at: clang/include/clang/Basic/AttrDocs.td:3787
+variable goes out of scope. Parameters and variables annotated with this
+attribute are implicitly ``const``.
+

You should add a paragraph contrasting this with ``__unsafe_unretained``.  For 
example:

  Unlike an `__unsafe_unretained` variable, an externally-retained variable 
remains
  semantically `__strong`.  This affects features like block capture which copy 
the
  current value of the variable into a capture field of the same type: the 
block's
  capture field will still be `__strong`, and so the value will be retained as 
part of
  capturing it and released when the block is destroyed.  It also affects C++ 
features
  such as lambda capture, `decltype`, and template argument deduction.

You should also describe this attribute in AutomaticReferenceCounting.rst.
You can add it in a new `arc.misc.externally_retained` section immediately above
`arc.misc.self`.  You can borrow some of the existing wording from the two
following sections, `arc.misc.self` and `arc.misc.enumeration`, and then make
those sections refer back to the new concept.



Comment at: clang/include/clang/Basic/AttrDocs.td:3789
+
+This attribute is meant to be used to opt-out of the additional safety that ARC
+provides when the programmer knows that the safety isn't necessary. You can 
read

The hyphen is unneeded here.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3492
+def warn_ignored_objc_externally_retained : Warning<
+  "'objc_externally_retained' can only be applied to strong retainable "
+  "object pointer types with automatic storage">, InGroup;

erik.pilkington wrote:
> rjmccall wrote:
> > erik.pilkington wrote:
> > > aaron.ballman wrote:
> > > > This wording isn't quite right -- it doesn't apply to types, it applies 
> > > > to variables of those types. How about: `... to local variables or 
> > > > parameters of strong, retainable object pointer type`? or something 
> > > > along those lines?
> > > Sure, that is a bit more clear.
> > I'd split this into two diagnostics:
> > - "...can only be applied to local variables and parameters of retainable 
> > type" (if the type or decl kind is wrong)
> > - "...can only be applied to variables with strong ownership" (if the 
> > qualifier is wrong)
> Sure, I guess a lot of information is crammed into this one. I coalesced the 
> two you suggested into one with a %select.
WFM



Comment at: clang/lib/CodeGen/CGDecl.cpp:807
+// that we omit the retain, and causes non-autoreleased return values to be
+// immediatly released.
+LLVM_FALLTHROUGH;

typo



Comment at: clang/lib/CodeGen/CGDecl.cpp:2334
+  // If a parameter is pseudo-strong, either because of the attribute
+  // objc_externally_retained or because its 'self' in an non-init method,
+  // then we can omit the implicit retain.

I wouldn't enumerate the cases in this comment.  You might want to enumerate 
them in a comment on `isARCPseudoStrong()`, though.


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

https://reviews.llvm.org/D55865



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

[clang-tools-extra] r349891 - Fix warning about unused variable [NFC]

[Bjorn Pettersson via cfe-commits]

Author: bjope
Date: Fri Dec 21 00:51:04 2018
New Revision: 349891

URL: http://llvm.org/viewvc/llvm-project?rev=349891&view=rev
Log:
Fix warning about unused variable [NFC]

Modified:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.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=349891&r1=349890&r2=349891&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
Fri Dec 21 00:51:04 2018
@@ -525,7 +525,7 @@ void SimplifyBooleanExprCheck::registerM
 }
 
 void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *TU = Result.Nodes.getNodeAs("top"))
+  if (Result.Nodes.getNodeAs("top"))
 Visitor(this, Result).TraverseAST(*Result.Context);
   else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
getBoolLiteral(Result, ConditionThenStmtId))


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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179248.
alexey.lapshin added a comment.

put a better link to doc


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp
=

[clang-tools-extra] r349893 - [clangd] Cleanup syntax errors in the test, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Dec 21 01:32:49 2018
New Revision: 349893

URL: http://llvm.org/viewvc/llvm-project?rev=349893&view=rev
Log:
[clangd] Cleanup syntax errors in the test, NFC.

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=349893&r1=349892&r2=349893&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Dec 21 
01:32:49 2018
@@ -195,7 +195,7 @@ struct ClassWithMembers {
   int AAA();
   int BBB();
   int CCC();
-}
+};
 int main() { ClassWithMembers().^ }
   )cpp",
  /*IndexSymbols=*/{}, Opts);
@@ -628,7 +628,7 @@ TEST(CompletionTest, NoIncludeInsertionW
  R"cpp(
   namespace ns {
 class X;
-class Y {}
+class Y {};
   }
   int main() { ns::^ }
   )cpp",
@@ -651,7 +651,7 @@ TEST(CompletionTest, IndexSuppressesPrea
   #include "bar.h"
   namespace ns { int local; }
   void f() { ns::^; }
-  void f() { ns::preamble().$2^; }
+  void f2() { ns::preamble().$2^; }
   )cpp");
   runAddDocument(Server, File, Test.code());
   clangd::CodeCompleteOptions Opts = {};
@@ -1927,7 +1927,7 @@ TEST(CompletionTest, EnableSpeculativeIn
   namespace ns1 { int abc; }
   namespace ns2 { int abc; }
   void f() { ns1::ab$1^; ns1::ab$2^; }
-  void f() { ns2::ab$3^; }
+  void f2() { ns2::ab$3^; }
   )cpp");
   runAddDocument(Server, File, Test.code());
   clangd::CodeCompleteOptions Opts = {};

Modified: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp?rev=349893&r1=349892&r2=349893&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp Fri Dec 21 
01:32:49 2018
@@ -439,7 +439,7 @@ TEST_F(DocumentSymbolsTest, DeclarationD
 TEST_F(DocumentSymbolsTest, ExternSymbol) {
   std::string FilePath = testPath("foo.cpp");
   addFile(testPath("foo.h"), R"cpp(
-  extern int var = 2;
+  extern int var;
   )cpp");
   addFile(FilePath, R"cpp(
   #include "foo.h"

Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=349893&r1=349892&r2=349893&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Fri Dec 21 
01:32:49 2018
@@ -429,7 +429,7 @@ TEST(QualityTests, Operator) {
   auto Header = TestTU::withHeaderCode(R"cpp(
 class Foo {
 public:
-  bool operator<(const Foo& f1, const Foo& f2);
+  bool operator<(const Foo& f1);
 };
   )cpp");
   auto AST = Header.build();

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=349893&r1=349892&r2=349893&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Fri Dec 
21 01:32:49 2018
@@ -131,7 +131,7 @@ TEST_F(ShouldCollectSymbolTest, ShouldCo
 namespace nx {
 class X{};
 auto f() { int Local; } // auto ensures function body is parsed.
-struct { int x } var;
+struct { int x; } var;
 namespace { class InAnonymous {}; }
 }
   )",


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

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 179249.
balazske added a comment.

Rebase.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53818

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -135,25 +135,6 @@
   To->setIsUsed();
   }
 
-  // FIXME: Temporary until every import returns Expected.
-  template <>
-  LLVM_NODISCARD Error
-  ASTImporter::importInto(SourceLocation &To, const SourceLocation &From) {
-To = Import(From);
-if (From.isValid() && To.isInvalid())
-return llvm::make_error();
-return Error::success();
-  }
-  // FIXME: Temporary until every import returns Expected.
-  template <>
-  LLVM_NODISCARD Error
-  ASTImporter::importInto(QualType &To, const QualType &From) {
-To = Import(From);
-if (!From.isNull() && To.isNull())
-return llvm::make_error();
-return Error::success();
-  }
-
   class ASTNodeImporter : public TypeVisitor,
   public DeclVisitor,
   public StmtVisitor {
@@ -7625,13 +7606,6 @@
 
 ASTImporter::~ASTImporter() = default;
 
-Expected ASTImporter::Import_New(QualType FromT) {
-  QualType ToT = Import(FromT);
-  if (ToT.isNull() && !FromT.isNull())
-return make_error();
-  return ToT;
-}
-
 Optional ASTImporter::getFieldIndex(Decl *F) {
   assert(F && (isa(*F) || isa(*F)) &&
   "Try to get field index for non-field.");
@@ -7683,9 +7657,9 @@
   LookupTable->add(ToND);
 }
 
-QualType ASTImporter::Import(QualType FromT) {
+Expected ASTImporter::Import_New(QualType FromT) {
   if (FromT.isNull())
-return {};
+return QualType{};
 
   const Type *FromTy = FromT.getTypePtr();
 
@@ -7698,46 +7672,64 @@
   // Import the type
   ASTNodeImporter Importer(*this);
   ExpectedType ToTOrErr = Importer.Visit(FromTy);
-  if (!ToTOrErr) {
-llvm::consumeError(ToTOrErr.takeError());
-return {};
-  }
+  if (!ToTOrErr)
+return ToTOrErr.takeError();
 
   // Record the imported type.
   ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
 
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
+QualType ASTImporter::Import(QualType From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return {};
+}
 
 Expected ASTImporter::Import_New(TypeSourceInfo *FromTSI) {
-  TypeSourceInfo *ToTSI = Import(FromTSI);
-  if (!ToTSI && FromTSI)
-return llvm::make_error();
-  return ToTSI;
-}
-TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *FromTSI) {
   if (!FromTSI)
 return FromTSI;
 
   // FIXME: For now we just create a "trivial" type source info based
   // on the type and a single location. Implement a real version of this.
-  QualType T = Import(FromTSI->getType());
-  if (T.isNull())
-return nullptr;
+  ExpectedType TOrErr = Import_New(FromTSI->getType());
+  if (!TOrErr)
+return TOrErr.takeError();
+  ExpectedSLoc BeginLocOrErr = Import_New(FromTSI->getTypeLoc().getBeginLoc());
+  if (!BeginLocOrErr)
+return BeginLocOrErr.takeError();
 
-  return ToContext.getTrivialTypeSourceInfo(
-  T, Import(FromTSI->getTypeLoc().getBeginLoc()));
+  return ToContext.getTrivialTypeSourceInfo(*TOrErr, *BeginLocOrErr);
+}
+TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return nullptr;
 }
 
 Expected ASTImporter::Import_New(const Attr *FromAttr) {
-  return Import(FromAttr);
-}
-Attr *ASTImporter::Import(const Attr *FromAttr) {
   Attr *ToAttr = FromAttr->clone(ToContext);
-  // NOTE: Import of SourceRange may fail.
-  ToAttr->setRange(Import(FromAttr->getRange()));
+  if (auto ToRangeOrErr = Import_New(FromAttr->getRange()))
+ToAttr->setRange(*ToRangeOrErr);
+  else
+return ToRangeOrErr.takeError();
+
   return ToAttr;
 }
+Attr *ASTImporter::Import(const Attr *From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return nullptr;
+}
 
 Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
   auto Pos = ImportedDecls.find(FromD);
@@ -7748,12 +7740,6 @@
 }
 
 Expected ASTImporter::Import_New(Decl *FromD) {
-  Decl *ToD = Import(FromD);
-  if (!ToD && FromD)
-return llvm::make_error();
-  return ToD;
-}
-Decl *ASTImporter::Import(Decl *FromD) {
   if (!FromD)
 return nullptr;
 
@@ -7767,12 +7753,10 @@
 return ToD;
   }
 
-  // Import the type.
+  // Import the declaration.
   ExpectedDecl ToDOrErr = Importer.Visit(FromD);
-  if (!ToDOrErr) {
-llvm::consumeError(ToDOrErr.takeError());
-return nullptr;
-  }
+  if (!ToDOrErr)
+return ToDOrErr;
   ToD = *ToDOrErr;
 
   // Once the decl is connec

[PATCH] D55933: [Sema] Do not print default template parameters.

[Clement Courbet via Phabricator via cfe-commits]

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



Comment at: lib/AST/TypePrinter.cpp:173
+  // information about whether default template parameters were actually
+  // written.
+  switch (QT.getTypePtr()->getTypeClass()) {

Quuxplusone wrote:
> FWIW, I don't understand this comment's terminology. "Whether default 
> template parameters were actually written": should that say something like 
> "whether a template parameter came from a default argument"?
> And why are the `Pointer`, `VariableArray`, etc. cases here? What goes wrong 
> if you remove them?
I've expanded the comments.



Comment at: test/SemaCXX/static-assert-cxx17.cpp:99
   static_assert((const X[]){} == nullptr);
-  // expected-error@-1{{static_assert failed due to requirement '(X 
const[0]){} == nullptr'}}
+  // expected-error@-1{{static_assert failed due to requirement '(const X 
[0]){} == nullptr'}}
   static_assert(sizeof(X().X::~X())>) 
== 0);

Quuxplusone wrote:
> (1) This cosmetic change is produced by the `case IncompleteArray:` that I 
> questioned above, right? I'm surprised that the pretty-printed type changes 
> from "east const" to "west const." But that's fine.
> 
> (2) Sorry I didn't notice before: surely where the message currently says 
> `[0]` it should say `[]` instead! That's an existing bug in the 
> pretty-printer, going back super far: https://godbolt.org/z/y9KzEq  So I 
> guess it's not your responsibility to fix. But I hope there's a bug open 
> about it somewhere?
> (1) This cosmetic change is produced by the case IncompleteArray: that I 
> questioned above, right? I'm surprised that the pretty-printed type changes 
> from "east const" to "west const." But that's fine.

Indeed. There's some logic in the printer to see where the const should go. 

> (2) Sorry I didn't notice before: surely where the message currently says [0] 
> it should say [] instead! That's an existing bug in the pretty-printer, going 
> back super far: https://godbolt.org/z/y9KzEq So I guess it's not your 
> responsibility to fix. But I hope there's a bug open about it somewhere?

Is it actually a bug ? In the godbolt example I actually thing putting the zero 
there actually clarifies the semantics of the expression for the reader, so I 
would'nt say it hurts.




Comment at: test/SemaCXX/static-assert-cxx17.cpp:103
+  static_assert(constexpr_return_false());
+  // expected-error@-1{{static_assert failed due to requirement 
'constexpr_return_false()'}}
 }

Quuxplusone wrote:
> Please keep the old test case 
> 
> static_assert(constexpr_return_false());
> // expected-error@-1{{static_assert failed due to requirement 
> 'constexpr_return_false()'}}
> 
> and then //add// this new test case. My understanding is that the old test 
> case should still pass, even after your change.
The old one is in foo7().



Comment at: test/SemaCXX/static-assert-cxx17.cpp:109
+template 
+void foo7() {
+  static_assert(X());

Quuxplusone wrote:
> None of these new test cases actually involve the default template argument.
> 
> I'd like to see two test cases explicitly mimicking `vector`. (Warning: I 
> didn't run this code!)
> ```
> template struct A {};
> template> struct V {};
> template void testx() {
> static_assert(std::is_same::value, "");
> // expected-error@-1{{due to requirement 'std::is_same*, 
> void>::value'}}
> }
> template testx>();
> template struct PMRA {};
> template using PMRV = V>;
> template void test() {
> static_assert(std::is_same::value, "");
> // expected-error@-1{{due to requirement 'std::is_same PMRA>*, void>::value'}}
> // expected-error@-2{{due to requirement 'std::is_same*, 
> void>::value'}}
> }
> template testy>();
> ```
> The `expected-error@-2` above is my fantasy world. I don't think it's 
> possible to achieve in real life. :)
> None of these new test cases actually involve the default template argument.

This one is to check that we actually do print when specified explicitly. foo6 
above tests  the default template arguments (notice the change from `template 
 struct X to `template  struct X` above). 
I've renamed `foo6` and `foo7` to make that clear.

Before this change, static_asserts in `foo6` and `foo7` printed the same thing. 
Now they don't.

> I'd like to see two test cases explicitly mimicking vector.

OK, I think I misunderstood what you wanted here. I don't think what you want 
is actually doable, because by the time you're in `test()`, C is just a type 
without any way of knowing whether the user explicitly provided the template 
parameter or relied on the default.

What we could do though is **always** erase template parameters that are the 
same as default ones. But that would mean printing `due to requirement 
'std::is_same*, void>::value'` even when the user wrote `template 
testx>>();`.
WDYT ?





Comment at: test/Sem

r349894 - Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Dec 21 02:11:23 2018
New Revision: 349894

URL: http://llvm.org/viewvc/llvm-project?rev=349894&view=rev
Log:
Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in 
RetainCountChecker on type mismatch even for inlined functions

The fix done in D55465 did not previously apply when the function was inlined.

rdar://46889541

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

Fixes broken buildbot: 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/14764

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=349894&r1=349893&r2=349894&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Fri Dec 21 02:11:23 2018
@@ -502,25 +502,6 @@ static Optional refValFromRetEff
   return None;
 }
 
-static bool isPointerToObject(QualType QT) {
-  QualType PT = QT->getPointeeType();
-  if (!PT.isNull())
-if (PT->getAsCXXRecordDecl())
-  return true;
-  return false;
-}
-
-/// Whether the tracked value should be escaped on a given call.
-/// OSObjects are escaped when passed to void * / etc.
-static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
-   const RefVal *TrackedValue) {
-  if (TrackedValue->getObjKind() != RetEffect::OS)
-return false;
-  if (ArgIdx >= CE.parameters().size())
-return false;
-  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
-}
-
 // We don't always get the exact modeling of the function with regards to the
 // retain count checker even when the function is inlined. For example, we need
 // to stop tracking the symbols which were marked with StopTrackingHard.
@@ -531,16 +512,11 @@ void RetainCountChecker::processSummaryO
 
   // Evaluate the effect of the arguments.
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
-SVal V = CallOrMsg.getArgSVal(idx);
-
-if (SymbolRef Sym = V.getAsLocSymbol()) {
-  bool ShouldRemoveBinding = Summ.getArg(idx) == StopTrackingHard;
-  if (const RefVal *T = getRefBinding(state, Sym))
-if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
-  ShouldRemoveBinding = true;
-
-  if (ShouldRemoveBinding)
+if (Summ.getArg(idx) == StopTrackingHard) {
+  SVal V = CallOrMsg.getArgSVal(idx);
+  if (SymbolRef Sym = V.getAsLocSymbol()) {
 state = removeRefBinding(state, Sym);
+  }
 }
   }
 
@@ -598,6 +574,25 @@ static ProgramStateRef updateOutParamete
   return State;
 }
 
+static bool isPointerToObject(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull())
+if (PT->getAsCXXRecordDecl())
+  return true;
+  return false;
+}
+
+/// Whether the tracked value should be escaped on a given call.
+/// OSObjects are escaped when passed to void * / etc.
+static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+   const RefVal *TrackedValue) {
+  if (TrackedValue->getObjKind() != RetEffect::OS)
+return false;
+  if (ArgIdx >= CE.parameters().size())
+return false;
+  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
+}
+
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   const CallEvent &CallOrMsg,
   CheckerContext &C) const {

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=349894&r1=349893&r2=349894&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Dec 21 02:11:23 2018
@@ -90,10 +90,7 @@ struct OSMetaClassBase {
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
-typedef unsigned long MYTYPE;
-
 void escape(void *);
-void escape_with_source(MYTYPE p) {}
 bool coin();
 
 bool os_consume_violation_two_args(OS_CONSUME OSObject *obj, bool extra) {
@@ -142,13 +139,6 @@ void test_escaping_into_voidstar() {
   escape(obj);
 }
 
-void test_escape_has_source() {
-  OSObject *obj = new OSObject;
-  if (obj)
-escape_with_source((MYTYPE)obj);
-  return;
-}
-
 void test_no_infinite_check_recursion(MyArray *arr) {
   OSObject *input = new OSObject;
   OSObject *o = arr->generateObject(input);

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.

Only run completion when we were trigerred on '->' and '::', otherwise
send an error code in return.
To avoid automatically invoking completions in cases like 'a >^' or
'a ? b :^'.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55994

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h

Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -775,6 +775,31 @@
 };
 bool fromJSON(const llvm::json::Value &, TextDocumentPositionParams &);
 
+enum class CompletionTriggerKind {
+  /// Completion was triggered by typing an identifier (24x7 code
+  /// complete), manual invocation (e.g Ctrl+Space) or via API.
+  Invoked = 1,
+  /// Completion was triggered by a trigger character specified by
+  /// the `triggerCharacters` properties of the `CompletionRegistrationOptions`.
+  TriggerCharacter = 2,
+  /// Completion was re-triggered as the current completion list is incomplete.
+  TriggerTriggerForIncompleteCompletions = 3
+};
+
+struct CompletionContext {
+  /// How the completion was triggered.
+  CompletionTriggerKind triggerKind = CompletionTriggerKind::Invoked;
+  /// The trigger character (a single character) that has trigger code complete.
+  /// Is undefined if `triggerKind !== CompletionTriggerKind.TriggerCharacter`
+  std::string triggerCharacter;
+};
+bool fromJSON(const llvm::json::Value &, CompletionContext &);
+
+struct CompletionParams : TextDocumentPositionParams {
+  CompletionContext context;
+};
+bool fromJSON(const llvm::json::Value &, CompletionParams &);
+
 enum class MarkupKind {
   PlainText,
   Markdown,
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -550,6 +550,29 @@
  O.map("position", R.position);
 }
 
+bool fromJSON(const llvm::json::Value &Params, CompletionContext &R) {
+  json::ObjectMapper O(Params);
+  if (!O)
+return false;
+
+  int triggerKind;
+  if (!O.map("triggerKind", triggerKind))
+return false;
+  R.triggerKind = static_cast(triggerKind);
+
+  if (auto *TC = Params.getAsObject()->get("triggerCharacter"))
+return fromJSON(*TC, R.triggerCharacter);
+  return true;
+}
+
+bool fromJSON(const llvm::json::Value &Params, CompletionParams &R) {
+  if (!fromJSON(Params, static_cast(R)))
+return false;
+  if (auto *Context = Params.getAsObject()->get("context"))
+return fromJSON(*Context, R.context);
+  return true;
+}
+
 static StringRef toTextKind(MarkupKind Kind) {
   switch (Kind) {
   case MarkupKind::PlainText:
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -74,8 +74,7 @@
   void onDocumentSymbol(const DocumentSymbolParams &,
 Callback);
   void onCodeAction(const CodeActionParams &, Callback);
-  void onCompletion(const TextDocumentPositionParams &,
-Callback);
+  void onCompletion(const CompletionParams &, Callback);
   void onSignatureHelp(const TextDocumentPositionParams &,
Callback);
   void onGoToDefinition(const TextDocumentPositionParams &,
@@ -98,6 +97,12 @@
 
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
+  /// Checks if completion request should be ignored. We need this due to the
+  /// limitation of the LSP. Per LSP, a client sends requests for all "trigger
+  /// character" we specify, but for '>' and ':' we need to check they actually
+  /// produce '->' and '::', respectively.
+  bool isSporadicCompletion(const CompletionParams &Params) const;
+
   /// Forces a reparse of all currently opened files.  As a result, this method
   /// may be very expensive.  This method is normally called when the
   /// compilation database is changed.
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -22,6 +22,15 @@
 namespace clang {
 namespace clangd {
 namespace {
+class CompletionSporadicError : public llvm::ErrorInfo {
+public:
+  void log(llvm::raw_ostream &OS) const override {
+OS << "ignored triggered completion, preceding char did not match";
+  }
+  std::error_code convertToErrorCode() const override {
+return std::make_error_code(std::errc::operation_canceled);
+  }
+};
 
 void adjustSymbolKinds(llvm::MutableArrayRef Syms,
SymbolKindBitset Kinds) {
@@ -310,6 +319,8 @@
 {"completionProvider",
  json::Object{
  {"resolveProvider", false},
+ // We do extra checks for '>' and ':' in completion to only
+ // trigger on '->' and '

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This still needs tests, will add them before committing the change. However, 
the implementation should be good for review.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 179272.
lebedev.ri added a comment.

Rebased, NFC.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54589

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp
  test/CodeGen/catch-alignment-assumption-attribute-align_value-on-paramvar.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp
  test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function-two-params.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function.cpp
  test/CodeGen/catch-alignment-assumption-blacklist.c
  
test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params-variable.cpp
  
test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params.cpp
  test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
  test/CodeGen/catch-alignment-assumption-openmp.cpp

Index: test/CodeGen/catch-alignment-assumption-openmp.cpp
===
--- /dev/null
+++ test/CodeGen/catch-alignment-assumption-openmp.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fopenmp-simd -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-NOSANITIZE
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 100, i32 30 }, {{.*}}* @[[CHAR]] }
+
+void func(char *data) {
+  // CHECK: define void @{{.*}}(i8* %[[DATA:.*]])
+  // CHECK-NEXT: [[ENTRY:.*]]:
+  // CHECK-NEXT:   %[[DATA_ADDR:.*]] = alloca i8*, align 8
+  // CHECK:   store i8* %[[DATA]], i8** %[[DATA_ADDR]], align 8
+  // CHECK:   %[[DATA_RELOADED:.*]] = load i8*, i8** %[[DATA_ADDR]], align 8
+  // CHECK-NEXT:   %[[PTRINT:.*]] = ptrtoint i8* %[[DATA_RELOADED]] to i64
+  // CHECK-NEXT:   %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 1073741823
+  // CHECK-NEXT:   %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[DATA_RELOADED]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:   br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[LINE_100_ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1073741824, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:   call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[LINE_100_ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1073741824, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-TRAP-NEXT:  call void @llvm.trap(){{.*}}, !nosanitize
+  // CHECK-SANITIZE-UNREACHABLE-NEXT:   unreachable, !nosanitize
+  // CHECK-SANITIZE:  [[CONT]]:
+  // CHECK-NEXT:call void @llvm.assume(i1 %[[MASKCOND]])
+
+#line 100
+#pragma omp for simd aligned(data : 0x4000)
+  for (int x = 0; x < 1; x++)
+data[x] = data[x];
+}
Index: test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
===
--- /dev/null
+++ test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-NOSANITIZE
+// RUN: %clang_cc1 -fsanitize=alignment -fno-sanitize-recover

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349899: [xray] [tests] Detect and handle missing 
LLVMTestingSupport gracefully (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D55891?vs=178896&id=179275#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55891

Files:
  compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
  compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt


Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -239,7 +239,7 @@
 # Detect if we have the LLVMXRay and TestingSupport library installed and
 # available from llvm-config.
 execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray" "testingsupport"
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray"
   RESULT_VARIABLE HAD_ERROR
   OUTPUT_VARIABLE CONFIG_OUTPUT)
 if (HAD_ERROR)
@@ -254,6 +254,26 @@
   set(COMPILER_RT_HAS_LLVMXRAY TRUE)
 endif()
 
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT FALSE)
+execute_process(
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "testingsupport"
+  RESULT_VARIABLE HAD_ERROR
+  OUTPUT_VARIABLE CONFIG_OUTPUT)
+if (HAD_ERROR)
+  message(WARNING "llvm-config finding testingsupport failed with status 
${HAD_ERROR}")
+else()
+  string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
+  list(GET CONFIG_OUTPUT 0 LDFLAGS)
+  list(GET CONFIG_OUTPUT 1 LIBLIST)
+  if (LIBLIST STREQUAL "")
+message(WARNING "testingsupport library not installed, some tests will 
be skipped")
+  else()
+set(LLVM_TESTINGSUPPORT_LDFLAGS ${LDFLAGS} CACHE STRING "Linker flags 
for LLVMTestingSupport library")
+set(LLVM_TESTINGSUPPORT_LIBLIST ${LIBLIST} CACHE STRING "Library list 
for LLVMTestingSupport")
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT TRUE)
+  endif()
+endif()
+
 # Make use of LLVM CMake modules.
 # --cmakedir is supported since llvm r291218 (4.0 release)
 execute_process(
Index: compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
===
--- compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
+++ compiler-rt/trunk/lib/xray/tests/unit/CMakeLists.txt
@@ -1,10 +1,16 @@
-add_xray_unittest(XRayTest SOURCES
+set(TEST_SOURCES
   allocator_test.cc
   buffer_queue_test.cc
-  fdr_controller_test.cc
-  fdr_log_writer_test.cc
   function_call_trie_test.cc
   profile_collector_test.cc
   segmented_array_test.cc
   test_helpers.cc
   xray_unit_test_main.cc)
+
+if (NOT COMPILER_RT_STANDALONE_BUILD OR COMPILER_RT_HAS_LLVMTESTINGSUPPORT)
+  list(APPEND TEST_SOURCES
+fdr_controller_test.cc
+fdr_log_writer_test.cc)
+endif()
+
+add_xray_unittest(XRayTest SOURCES ${TEST_SOURCES})
Index: compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
===
--- compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
+++ compiler-rt/trunk/lib/xray/tests/CMakeLists.txt
@@ -60,6 +60,10 @@
   if (COMPILER_RT_STANDALONE_BUILD)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LDFLAGS} 
XRAY_UNITTEST_LINK_FLAGS)
 append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LIBLIST} 
XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
+  ${LLVM_TESTINGSUPPORT_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
   else()
 # We add the library directories one at a time in our CFLAGS.
 foreach (DIR ${LLVM_LIBRARY_DIR})


Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -239,7 +239,7 @@
 # Detect if we have the LLVMXRay and TestingSupport library installed and
 # available from llvm-config.
 execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray" "testingsupport"
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "xray"
   RESULT_VARIABLE HAD_ERROR
   OUTPUT_VARIABLE CONFIG_OUTPUT)
 if (HAD_ERROR)
@@ -254,6 +254,26 @@
   set(COMPILER_RT_HAS_LLVMXRAY TRUE)
 endif()
 
+set(COMPILER_RT_HAS_LLVMTESTINGSUPPORT FALSE)
+execute_process(
+  COMMAND ${LLVM_CONFIG_PATH} "--ldflags" "--libs" "testingsupport"
+  RESULT_VARIABLE HAD_ERROR
+  OUTPUT_VARIABLE CONFIG_OUTPUT)
+if (HAD_ERROR)
+  message(WARNING "llvm-config 

[PATCH] D55916: [clang] Replace getOS() == llvm::Triple::*BSD with isOS*BSD() [NFCI]

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

No problem. I'm sorry about my initial mistake. Apparently it slipped me even 
though I've double checked it originally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55916



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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 179277.
dkrupp marked an inline comment as done.
dkrupp added a comment.

All comments fixed.

I also added the handling of redundancy comparison of sizeof(..), alignof() 
operators.


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

https://reviews.llvm.org/D55125

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -106,6 +106,7 @@
 
 #define COND_OP_MACRO 9
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {
   int k = 0;
   k += (y < 0) ? x : x;
@@ -114,16 +115,33 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent
   k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO;
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? COND_OP_MACRO + COND_OP_OTHER_MACRO : COND_OP_MACRO + COND_OP_OTHER_MACRO;
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'true' and 'false' expressions are equivalent
 
   // Do not match for conditional operators with a macro and a const.
   k += (y < 0) ? COND_OP_MACRO : 9;
   // Do not match for conditional operators with expressions from different macros.
   k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO;
+  // Do not match for conditional operators when a macro is defined to another macro
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+#define   COND_OP_THIRD_MACRO 8
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+
+  k += (y < 0) ? sizeof(I64) : sizeof(I64);
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(TestConditional(k,y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'true' and 'false' expressions are equivalent
+  // No warning if the expression arguments are different.
+  k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(Valid(k,y));
+
   return k;
 }
 #undef COND_OP_MACRO
 #undef COND_OP_OTHER_MACRO
 
+
 // Overloaded operators that compare two instances of a struct.
 struct MyStruct {
   int x;  
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -132,6 +132,18 @@
   case Stmt::BinaryOperatorClass:
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
+  case Stmt::UnaryExprOrTypeTraitExprClass:
+if (cast(Left)->isArgumentType() &&
+cast(Right)->isArgumentType())
+  return cast(Left)->getArgumentType() ==
+ cast(Right)->getArgumentType();
+else if (!cast(Left)->isArgumentType() &&
+ !cast(Right)->isArgumentType())
+  return areEquivalentExpr(
+  cast(Left)->getArgumentExpr(),
+  cast(Right)->getArgumentExpr());
+
+return false;
   }
 }
 
@@ -598,23 +610,66 @@
   return true;
 }
 
+static bool isSameToken(Token &T1, Token &T2, const SourceManager &SM) {
+  if (T1.getKind()!=T2.getKind())
+return false;
+  else if (T1.isNot(tok::raw_identifier))
+return true;
+  if (T1.getLength() != T2.getLength())
+return false;
+  return strncmp(SM.getCharacterData(T1.getLocation()),
+ SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0;
+}
+
+unsigned getCharCountUntilEndOfExpr(SourceRange ExprSR, Token T,
+const SourceManager &SM) {
+  assert((T.getLocation() < ExprSR.getBegin()  ||
+  ExprSR.getEnd() < T.getLocation()) &&
+ "Token is not within the expression range!");
+
+  return SM.getCharacterData(SM.getExpansionLoc(ExprSR.getEnd())) -
+ SM.getCharacterData(T.getLocation());
+}
+
+/// Returns true if both LhsEpxr and RhsExpr are
+/// macro expressions and they are expanded
+/// from different macros.
 static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const Expr *RhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
-
-  if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+  SourceRange Lsr = LhsExpr->getSourceRange();
+  SourceRange Rsr = RhsExpr->getSourceRange();
+  if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
-
   const SourceManager &SM = AstCtx->getSourceManager();
   const LangOptions &LO = AstCtx->getLangOpts();
 
-  return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
-  Lexer::getImmediateMacroNa

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, dberris, vitalybuka.
Herald added subscribers: Sanitizers, llvm-commits.

Disable enforcing alignas() for structs that are used as thread_local
data on NetBSD.  The NetBSD ld.so implementation is buggy and does
not enforce correct alignment; however, clang seems to take it for
granted and generates instructions that segv on wrongly aligned objects.
Therefore, disable those alignas() statements on NetBSD until we can
establish a better fix.

Apparently, std::aligned_storage<> does not have any real effect
at the moment, so we can leave it as-is.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D56000

Files:
  lib/xray/xray_basic_logging.cc
  lib/xray/xray_fdr_logging.cc


Index: lib/xray/xray_fdr_logging.cc
===
--- lib/xray/xray_fdr_logging.cc
+++ lib/xray/xray_fdr_logging.cc
@@ -51,7 +51,12 @@
 // call so that it can be initialized on first use instead of as a global. We
 // force the alignment to 64-bytes for x86 cache line alignment, as this
 // structure is used in the hot path of implementation.
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */
+#if !SANITIZER_NETBSD
+alignas(64)
+#endif
+ThreadLocalData {
   BufferQueue::Buffer Buffer{};
   BufferQueue *BQ = nullptr;
 
@@ -124,8 +129,10 @@
 // critical section, calling a function that might be XRay instrumented (and
 // thus in turn calling into malloc by virtue of registration of the
 // thread_local's destructor).
+#if !SANITIZER_NETBSD
 static_assert(alignof(ThreadLocalData) >= 64,
   "ThreadLocalData must be cache line aligned.");
+#endif
 static ThreadLocalData &getThreadLocalData() {
   thread_local typename std::aligned_storage<
   sizeof(ThreadLocalData), alignof(ThreadLocalData)>::type TLDStorage{};
Index: lib/xray/xray_basic_logging.cc
===
--- lib/xray/xray_basic_logging.cc
+++ lib/xray/xray_basic_logging.cc
@@ -55,7 +55,12 @@
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */
+#if !SANITIZER_NETBSD
+alignas(64)
+#endif
+ThreadLocalData {
   void *InMemoryBuffer = nullptr;
   size_t BufferSize = 0;
   size_t BufferOffset = 0;


Index: lib/xray/xray_fdr_logging.cc
===
--- lib/xray/xray_fdr_logging.cc
+++ lib/xray/xray_fdr_logging.cc
@@ -51,7 +51,12 @@
 // call so that it can be initialized on first use instead of as a global. We
 // force the alignment to 64-bytes for x86 cache line alignment, as this
 // structure is used in the hot path of implementation.
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */
+#if !SANITIZER_NETBSD
+alignas(64)
+#endif
+ThreadLocalData {
   BufferQueue::Buffer Buffer{};
   BufferQueue *BQ = nullptr;
 
@@ -124,8 +129,10 @@
 // critical section, calling a function that might be XRay instrumented (and
 // thus in turn calling into malloc by virtue of registration of the
 // thread_local's destructor).
+#if !SANITIZER_NETBSD
 static_assert(alignof(ThreadLocalData) >= 64,
   "ThreadLocalData must be cache line aligned.");
+#endif
 static ThreadLocalData &getThreadLocalData() {
   thread_local typename std::aligned_storage<
   sizeof(ThreadLocalData), alignof(ThreadLocalData)>::type TLDStorage{};
Index: lib/xray/xray_basic_logging.cc
===
--- lib/xray/xray_basic_logging.cc
+++ lib/xray/xray_basic_logging.cc
@@ -55,7 +55,12 @@
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */
+#if !SANITIZER_NETBSD
+alignas(64)
+#endif
+ThreadLocalData {
   void *InMemoryBuffer = nullptr;
   size_t BufferSize = 0;
   size_t BufferOffset = 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp marked 13 inline comments as done.
dkrupp added a comment.

Thanks for your comments. I fixed them all. I also added the handling of 
redundant sizeof() and alignof() operators on the way. Please check if OK now...




Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:602
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM) {
+  if (T1.getLength() != T2.getLength())
+return false;

Szelethus wrote:
> alexfh wrote:
> > `Token::getLength()` will assert-fail for annotation tokens.
> I guess if `T1.isNot(tok::raw_identifier)` (or `T2`) we could get away with 
> simply comparing token kinds. If they are, it wouldn't assert. :)
I changed to code as per the suggestion from Szelethus.
So we will compare only raw_identifiers char-by-char, the rest by kind only. 
According to my tests, macros and types, typedefs for example are such 
raw_identifiers. 




Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:605
+  return strncmp(SM.getCharacterData(T1.getLocation()),
+ SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0;
+}

alexfh wrote:
> JonasToth wrote:
> > This operation could overflow if `len(T1) > len(T2)` and `T2` is the last 
> > token of the file. I think `std::min(T1.getLength(), T2.getLength())` would 
> > be better.
> This code is only executed when they have the same length.
exactly, overflow should not happen.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:638
+  SourceLocation LLoc, RLoc;
+  int LRem, RRem;
+  do {//we compare the expressions token-by-token

Szelethus wrote:
> It seems like we're only checking whether these values are `0` or not, maybe 
> make them `bool`? Also, I find `Rem` a little cryptic, what is it referring 
> to? Maybe `RemainingCharCount`?
> 
> Maybe we should just make a simple function, so we could both boost 
> readability, and get rid of some these variables (`LLoc`, `RLoc`) entirely:
> 
> ```
> unsinged getCharCountUntilEndOfExpr(SourceRange ExprSR, Token T,
> const SourceManager &SM) {
> 
>   assert((ExprSR.getBegin() > T.getLocation() ||
>   ExprSR.getEnd() < T.getLocation()) &&
>  "Token is not within the expression range!");
> 
>   return SM.getCharacterData(SM.getExpansionLoc(ExprSR.getEnd())) -
>  SM.getCharacterData(T.getLocation());
> }
> ```
> 
> 
good point. Refactored ass suggested.


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

https://reviews.llvm.org/D55125



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

r349901 - [AST][NFC] Pass the AST context to one of the ctor of DeclRefExpr.

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 06:10:18 2018
New Revision: 349901

URL: http://llvm.org/viewvc/llvm-project?rev=349901&view=rev
Log:
[AST][NFC] Pass the AST context to one of the ctor of DeclRefExpr.

All of the other constructors already take a reference to the AST context.
This avoids calling Decl::getASTContext in most cases. Additionally move
the definition of the constructor from Expr.h to Expr.cpp since it is calling
DeclRefExpr::computeDependence. NFC.


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaCUDA.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaObjCProperty.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/TreeTransform.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=349901&r1=349900&r2=349901&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Dec 21 06:10:18 2018
@@ -1086,20 +1086,10 @@ class DeclRefExpr final
   void computeDependence(const ASTContext &Ctx);
 
 public:
-  DeclRefExpr(ValueDecl *D, bool RefersToEnclosingVariableOrCapture, QualType 
T,
+  DeclRefExpr(const ASTContext &Ctx, ValueDecl *D,
+  bool RefersToEnclosingVariableOrCapture, QualType T,
   ExprValueKind VK, SourceLocation L,
-  const DeclarationNameLoc &LocInfo = DeclarationNameLoc())
-  : Expr(DeclRefExprClass, T, VK, OK_Ordinary, false, false, false, false),
-D(D), DNLoc(LocInfo) {
-DeclRefExprBits.HasQualifier = false;
-DeclRefExprBits.HasTemplateKWAndArgsInfo = false;
-DeclRefExprBits.HasFoundDecl = false;
-DeclRefExprBits.HadMultipleCandidates = false;
-DeclRefExprBits.RefersToEnclosingVariableOrCapture =
-RefersToEnclosingVariableOrCapture;
-DeclRefExprBits.Loc = L;
-computeDependence(D->getASTContext());
-  }
+  const DeclarationNameLoc &LocInfo = DeclarationNameLoc());
 
   static DeclRefExpr *
   Create(const ASTContext &Context, NestedNameSpecifierLoc QualifierLoc,

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=349901&r1=349900&r2=349901&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Dec 21 06:10:18 2018
@@ -4320,7 +4320,7 @@ TemplateArgument ASTContext::getInjected
 Arg = TemplateArgument(ArgType);
   } else if (auto *NTTP = dyn_cast(Param)) {
 Expr *E = new (*this) DeclRefExpr(
-NTTP, /*enclosing*/false,
+*this, NTTP, /*enclosing*/ false,
 NTTP->getType().getNonLValueExprType(*this),
 Expr::getValueKindForType(NTTP->getType()), NTTP->getLocation());
 

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=349901&r1=349900&r2=349901&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Fri Dec 21 06:10:18 2018
@@ -341,16 +341,31 @@ void DeclRefExpr::computeDependence(cons
 ExprBits.ContainsUnexpandedParameterPack = true;
 }
 
+DeclRefExpr::DeclRefExpr(const ASTContext &Ctx, ValueDecl *D,
+ bool RefersToEnclosingVariableOrCapture, QualType T,
+ ExprValueKind VK, SourceLocation L,
+ const DeclarationNameLoc &LocInfo)
+: Expr(DeclRefExprClass, T, VK, OK_Ordinary, false, false, false, false),
+  D(D), DNLoc(LocInfo) {
+  DeclRefExprBits.HasQualifier = false;
+  DeclRefExprBits.HasTemplateKWAndArgsInfo = false;
+  DeclRefExprBits.HasFoundDecl = false;
+  DeclRefExprBits.HadMultipleCandidates = false;
+  DeclRefExprBits.RefersToEnclosingVariableOrCapture =
+  RefersToEnclosingVariableOrCapture;
+  DeclRefExprBits.Loc = L;
+  computeDependence(Ctx);
+}
+
 DeclRefExpr::DeclRefExpr(const ASTContext &Ctx,
  NestedNameSpecifierLoc QualifierLoc,
- SourceLocation TemplateKWLoc,
- ValueDecl *D, bool RefersToEnclosingVariableOrCapture,
- const DeclarationNameInfo &NameInfo,
- NamedDecl *FoundD,
+ SourceLocation TemplateKWLoc, Va

[PATCH] D55982: [OPENMP] Add support for explicit mapping of classes using 'this' pointer

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D55982



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

r349904 - [Sema][NFC] Remove some unnecessary calls to getASTContext.

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 06:35:24 2018
New Revision: 349904

URL: http://llvm.org/viewvc/llvm-project?rev=349904&view=rev
Log:
[Sema][NFC] Remove some unnecessary calls to getASTContext.

The AST context is already easily available. NFC.


Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=349904&r1=349903&r2=349904&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec 21 06:35:24 2018
@@ -5148,8 +5148,6 @@ Sema::SemaBuiltinAtomicOverloaded(ExprRe
 TheCall->setArg(i+1, Arg.get());
   }
 
-  ASTContext& Context = this->getASTContext();
-
   // Create a new DeclRefExpr to refer to the new decl.
   DeclRefExpr* NewDRE = DeclRefExpr::Create(
   Context,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=349904&r1=349903&r2=349904&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Dec 21 06:35:24 2018
@@ -5875,10 +5875,10 @@ isOutOfScopePreviousDeclaration(NamedDec
   return true;
 }
 
-static void SetNestedNameSpecifier(DeclaratorDecl *DD, Declarator &D) {
+static void SetNestedNameSpecifier(Sema &S, DeclaratorDecl *DD, Declarator &D) 
{
   CXXScopeSpec &SS = D.getCXXScopeSpec();
   if (!SS.isSet()) return;
-  DD->setQualifierInfo(SS.getWithLocInContext(DD->getASTContext()));
+  DD->setQualifierInfo(SS.getWithLocInContext(S.Context));
 }
 
 bool Sema::inferObjCARCLifetime(ValueDecl *decl) {
@@ -6583,7 +6583,7 @@ NamedDecl *Sema::ActOnVariableDeclarator
 NewTemplate->setInvalidDecl();
 }
 
-SetNestedNameSpecifier(NewVD, D);
+SetNestedNameSpecifier(*this, NewVD, D);
 
 // If we have any template parameter lists that don't directly belong to
 // the variable (matching the scope specifier), store them.
@@ -8384,7 +8384,7 @@ Sema::ActOnFunctionDeclarator(Scope *S,
 Diag(D.getDeclSpec().getVirtualSpecLoc(), diag::err_virtual_in_union);
 }
 
-SetNestedNameSpecifier(NewFD, D);
+SetNestedNameSpecifier(*this, NewFD, D);
 isMemberSpecialization = false;
 isFunctionTemplateSpecialization = false;
 if (D.isInvalidType())

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=349904&r1=349903&r2=349904&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Dec 21 06:35:24 2018
@@ -16547,7 +16547,7 @@ ExprResult RebuildUnknownAnyExpr::resolv
   DeclRefExpr *DRE = dyn_cast(E);
   if (DRE && Proto && Proto->getParamTypes().empty() && 
Proto->isVariadic()) {
 SourceLocation Loc = FD->getLocation();
-FunctionDecl *NewFD = FunctionDecl::Create(FD->getASTContext(),
+FunctionDecl *NewFD = FunctionDecl::Create(S.Context,
   FD->getDeclContext(),
   Loc, Loc, FD->getNameInfo().getName(),
   DestType, FD->getTypeSourceInfo(),

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=349904&r1=349903&r2=349904&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Dec 21 06:35:24 2018
@@ -1255,9 +1255,10 @@ Sema::ActOnTemplateParameterList(unsigne
   RAngleLoc, RequiresClause);
 }
 
-static void SetNestedNameSpecifier(TagDecl *T, const CXXScopeSpec &SS) {
+static void SetNestedNameSpecifier(Sema &S, TagDecl *T,
+   const CXXScopeSpec &SS) {
   if (SS.isSet())
-T->setQualifierInfo(SS.getWithLocInContext(T->getASTContext()));
+T->setQualifierInfo(SS.getWithLocInContext(S.Context));
 }
 
 DeclResult Sema::CheckClassTemplate(
@@ -1554,7 +1555,7 @@ DeclResult Sema::CheckClassTemplate(
   PrevClassTemplate && ShouldAddRedecl ?
 PrevClassTemplate->getTemplatedDecl() : nullptr,
   /*DelayTypeCreation=*/true);
-  SetNestedNameSpecifier(NewClass, SS);
+  SetNestedNameSpecifier(*this, NewClass, SS);
   if (NumOuterTemplateParamLists > 0)
 NewClass->setTemplateParameterListsInfo(
 Context, llvm::makeArrayRef(OuterTemplateParamLists,
@@ -7650,7 +7651,7 @@ DeclResult Sema::ActOnClassTemplateSpeci
TemplateArgs,
  

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 3 inline comments as done.
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaStmt.cpp:2846
 diag::warn_empty_range_based_for_body);
+  DiagnoseUnusedExprResult(B);
 

rsmith wrote:
> rsmith wrote:
> > While this looks correct per the current approach to this function, we 
> > really shouldn't be duplicating calls to this everywhere. Can we move all 
> > the calls to a single call in `ActOnFinishFullStmt`?
> Looks like that's not actually called from almost anywhere, but checking from 
> `ActOnFinishFullExpr` in the case where `DiscardedValue` is `true` seems 
> appropriate.
That seems sensible, but how will that work with GNU statement expressions? If 
we check for unused expression results, then we will trigger on code like this:
```
int a = ({blah(); yada(); 0;});
// or
int b = ({blah(); yada(); some_no_discard_call();});
```
I don't know of a way to handle that case -- we call `ActOnFinishFullExpr()` 
while parsing the compound statement for the `StmtExpr`, so there's no way to 
know whether there's another statement coming (without lookahead) to determine 
whether to suppress the diagnostic for the last expression statement. Do you 
have ideas on how to handle that?


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

https://reviews.llvm.org/D55955



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

[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

I think the arch-change (switching from a whitelist to a MIPS blacklist) is 
reasonable. What is the motivation for dropping `DT_HASH`, just binary size 
reduction?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55878



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

r349910 - [AST] Store the callee and argument expressions of CallExpr in a trailing array.

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 07:20:32 2018
New Revision: 349910

URL: http://llvm.org/viewvc/llvm-project?rev=349910&view=rev
Log:
[AST] Store the callee and argument expressions of CallExpr in a trailing array.

Since CallExpr::setNumArgs has been removed, it is now possible to store the
callee expression and the argument expressions of CallExpr in a trailing array.
This saves one pointer per CallExpr, CXXOperatorCallExpr, CXXMemberCallExpr,
CUDAKernelCallExpr and UserDefinedLiteral.

Given that CallExpr is used as a base of the above classes we cannot use
llvm::TrailingObjects. Instead we store the offset in bytes from the this 
pointer
to the start of the trailing objects and manually do the casts + arithmetic.

Some notes:

1.) I did not try to fit the number of arguments in the bit-fields of Stmt.
This leaves some space for future additions and avoid the discussion about
whether x bits are sufficient to hold the number of arguments.

2.) It would be perfectly possible to recompute the offset to the trailing
objects before accessing the trailing objects. However the trailing objects
are frequently accessed and benchmarks show that it is slightly faster to
just load the offset from the bit-fields. Additionally, because of 1),
we have plenty of space in the bit-fields of Stmt.

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

Reviewed By: rjmccall


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=349910&r1=349909&r2=349910&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Dec 21 07:20:32 2018
@@ -2395,59 +2395,126 @@ public:
 /// a subclass for overloaded operator calls that use operator syntax, e.g.,
 /// "str1 + str2" to resolve to a function call.
 class CallExpr : public Expr {
-  enum { FN=0, PREARGS_START=1 };
-  Stmt **SubExprs;
+  enum { FN = 0, PREARGS_START = 1 };
+
+  /// The number of arguments in the call expression.
   unsigned NumArgs;
+
+  /// The location of the right parenthese. This has a different meaning for
+  /// the derived classes of CallExpr.
   SourceLocation RParenLoc;
 
   void updateDependenciesFromArg(Expr *Arg);
 
+  // CallExpr store some data in trailing objects. However since CallExpr
+  // is used a base of other expression classes we cannot use
+  // llvm::TrailingObjects. Instead we manually perform the pointer arithmetic
+  // and casts.
+  //
+  // The trailing objects are in order:
+  //
+  // * A single "Stmt *" for the callee expression.
+  //
+  // * An array of getNumPreArgs() "Stmt *" for the pre-argument expressions.
+  //
+  // * An array of getNumArgs() "Stmt *" for the argument expressions.
+  //
+  // Note that we store the offset in bytes from the this pointer to the start
+  // of the trailing objects. It would be perfectly possible to compute it
+  // based on the dynamic kind of the CallExpr. However 1.) we have plenty of
+  // space in the bit-fields of Stmt. 2.) It was benchmarked to be faster to
+  // compute this once and then load the offset from the bit-fields of Stmt,
+  // instead of re-computing the offset each time the trailing objects are
+  // accessed.
+
+  /// Return a pointer to the start of the trailing array of "Stmt *".
+  Stmt **getTrailingStmts() {
+return reinterpret_cast(reinterpret_cast(this) +
+ CallExprBits.OffsetToTrailingObjects);
+  }
+  Stmt *const *getTrailingStmts() const {
+return const_cast(this)->getTrailingStmts();
+  }
+
+  /// Map a statement class to the appropriate offset in bytes from the
+  /// this pointer to the trailing objects.
+  static unsigned offsetToTrailingObjects(StmtClass SC);
+
 public:
   enum class ADLCallKind : bool { NotADL, UsesADL };
   static constexpr ADLCallKind NotADL = ADLCallKind::NotADL;
   static constexpr ADLCallKind UsesADL = ADLCallKind::UsesADL;
 
 protected:
-  // These versions of the constructor are for derived classes.
-  CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,
-   ArrayRef preargs, ArrayRef args, QualType t,
-   ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0,
-   ADLCallKind UsesADL

[PATCH] D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349910: [AST] Store the callee and argument expressions of 
CallExpr in a trailing array. (authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55771?vs=178641&id=179286#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55771

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/AST/ExprCXX.cpp
  cfe/trunk/lib/Analysis/BodyFarm.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -6990,9 +6990,8 @@
   if (Error Err = ImportContainerChecked(E->arguments(), ToArgs))
 return std::move(Err);
 
-  return new (Importer.getToContext()) CXXMemberCallExpr(
-  Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
-  ToRParenLoc);
+  return CXXMemberCallExpr::Create(Importer.getToContext(), ToCallee, ToArgs,
+   ToType, E->getValueKind(), ToRParenLoc);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXThisExpr(CXXThisExpr *E) {
@@ -7317,15 +7316,15 @@
  return std::move(Err);
 
   if (const auto *OCE = dyn_cast(E)) {
-return new (Importer.getToContext()) CXXOperatorCallExpr(
+return CXXOperatorCallExpr::Create(
 Importer.getToContext(), OCE->getOperator(), ToCallee, ToArgs, ToType,
 OCE->getValueKind(), ToRParenLoc, OCE->getFPFeatures(),
 OCE->getADLCallKind());
   }
 
-  return new (Importer.getToContext()) CallExpr(
-  Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
-  ToRParenLoc, /*MinNumArgs=*/0, E->getADLCallKind());
+  return CallExpr::Create(Importer.getToContext(), ToCallee, ToArgs, ToType,
+  E->getValueKind(), ToRParenLoc, /*MinNumArgs=*/0,
+  E->getADLCallKind());
 }
 
 ExpectedStmt ASTNodeImporter::VisitLambdaExpr(LambdaExpr *E) {
Index: cfe/trunk/lib/AST/ExprCXX.cpp
===
--- cfe/trunk/lib/AST/ExprCXX.cpp
+++ cfe/trunk/lib/AST/ExprCXX.cpp
@@ -478,6 +478,46 @@
   return End;
 }
 
+CXXOperatorCallExpr::CXXOperatorCallExpr(OverloadedOperatorKind OpKind,
+ Expr *Fn, ArrayRef Args,
+ QualType Ty, ExprValueKind VK,
+ SourceLocation OperatorLoc,
+ FPOptions FPFeatures,
+ ADLCallKind UsesADL)
+: CallExpr(CXXOperatorCallExprClass, Fn, /*PreArgs=*/{}, Args, Ty, VK,
+   OperatorLoc, /*MinNumArgs=*/0, UsesADL),
+  Operator(OpKind), FPFeatures(FPFeatures) {
+  Range = getSourceRangeImpl();
+}
+
+CXXOperatorCallExpr::CXXOperatorCallExpr(unsigned NumArgs, EmptyShell Empty)
+: CallExpr(CXXOperatorCallExprClass, /*NumPreArgs=*/0, NumArgs, Empty) {}
+
+CXXOperatorCallExpr *CXXOperatorCallExpr::Create(
+const ASTContext &Ctx, OverloadedOperatorKind OpKind, Expr *Fn,
+ArrayRef Args, QualType Ty, ExprValueKind VK,
+SourceLocation OperatorLoc, FPOptions FPFeatures, ADLCallKind UsesADL) {
+  // Allocate storage for the trailing objects of CallExpr.
+  unsigned NumArgs = Args.size();
+  unsigned SizeOfTrailingObjects =
+  CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/0, NumArgs);
+  void *Mem = Ctx.Allocate(sizeof(CXXOperatorCallExpr) + SizeOfTrailingObjects,
+   alignof(CXXOperatorCallExpr));
+  return new (Mem) CXXOperatorCallExpr(OpKind, Fn, Args, Ty, VK, OperatorLoc,
+   FPFeatures, UsesADL);
+}
+
+CXXOperatorCallExpr *CXXOperatorCallExpr::CreateEmpty(const ASTContext &Ctx,
+  unsigned NumArgs,
+  EmptyShell Empty) {
+  // Allocate storage for the trailing objects of CallExpr.
+  unsigned SizeOfTrailingObjects =
+  CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/0, NumArgs);
+  void *Mem = Ctx.Allocate(sizeof(CXXOperatorCallExpr) + SizeOfTrailingObjects,
+   alignof(CXXOperatorCallExpr));
+  return new (Mem) CXXOperatorCallExpr(NumArgs, Empty);
+}
+
 SourceRange CXXOperatorCallExpr::getSo

[PATCH] D56006: [AST] Fix a -Wimplicit-fallthrough warning in ScanfFormatString.cpp

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added a reviewer: hans.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

This is showing up with some bots
(eg 
http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/19061/steps/build%20stage%201/logs/warnings%20%284%29)

I am putting this for review since I am not familiar with this code and am not
sure whether adding `llvm_unreachable` is better than adding a default case
covering the invalid cases, as is done a few lines below.


Repository:
  rC Clang

https://reviews.llvm.org/D56006

Files:
  lib/AST/ScanfFormatString.cpp


Index: lib/AST/ScanfFormatString.cpp
===
--- lib/AST/ScanfFormatString.cpp
+++ lib/AST/ScanfFormatString.cpp
@@ -264,6 +264,7 @@
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unexpected length modifier!");
 
 // Unsigned int.
 case ConversionSpecifier::oArg:
@@ -303,6 +304,7 @@
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unexpected length modifier!");
 
 // Float.
 case ConversionSpecifier::aArg:


Index: lib/AST/ScanfFormatString.cpp
===
--- lib/AST/ScanfFormatString.cpp
+++ lib/AST/ScanfFormatString.cpp
@@ -264,6 +264,7 @@
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unexpected length modifier!");
 
 // Unsigned int.
 case ConversionSpecifier::oArg:
@@ -303,6 +304,7 @@
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unexpected length modifier!");
 
 // Float.
 case ConversionSpecifier::aArg:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/xray/xray_basic_logging.cc:58
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */

Can we introduce a macro like:

```
#if SANITIZER_NETBSD
#define XRAY_TLS_ALIGNAS64 /* Nor supported */
#else
#define XRAY_TLS_ALIGNED_SUPPORTED
#define XRAY_TLS_ALIGNAS64 alignas(64)
#endif
```

And later:

```
struct XRAY_TLS_ALIGNAS64 ThreadLocalData {
```

and

```
#ifdef XRAY_TLS_ALIGNED_SUPPORTED
static_assert(alignof(ThreadLocalData) >= 64,
  "ThreadLocalData must be cache line aligned.");
#endif
```


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56000



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

Ok, I found the fix for the first crash that landed in 8.0 trunk. It works fine 
for me if backported to 7.0.1:
https://reviews.llvm.org/D50461


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Michał Górny via Phabricator via cfe-commits]

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



Comment at: lib/xray/xray_basic_logging.cc:58
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */

krytarowski wrote:
> Can we introduce a macro like:
> 
> ```
> #if SANITIZER_NETBSD
> #define XRAY_TLS_ALIGNAS64 /* Nor supported */
> #else
> #define XRAY_TLS_ALIGNED_SUPPORTED
> #define XRAY_TLS_ALIGNAS64 alignas(64)
> #endif
> ```
> 
> And later:
> 
> ```
> struct XRAY_TLS_ALIGNAS64 ThreadLocalData {
> ```
> 
> and
> 
> ```
> #ifdef XRAY_TLS_ALIGNED_SUPPORTED
> static_assert(alignof(ThreadLocalData) >= 64,
>   "ThreadLocalData must be cache line aligned.");
> #endif
> ```
Maybe. Though i suppose it'd make more sense to make `64` an argument to the 
macro.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56000



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D49754#1339161 , @vit9696 wrote:

> Ok, I found the fix for the first crash that landed in 8.0 trunk. It works 
> fine for me if backported to 7.0.1:
>  https://reviews.llvm.org/D50461


That's awesome, thanks so much for fixing that issue!


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/xray/xray_basic_logging.cc:58
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */

mgorny wrote:
> krytarowski wrote:
> > Can we introduce a macro like:
> > 
> > ```
> > #if SANITIZER_NETBSD
> > #define XRAY_TLS_ALIGNAS64 /* Nor supported */
> > #else
> > #define XRAY_TLS_ALIGNED_SUPPORTED
> > #define XRAY_TLS_ALIGNAS64 alignas(64)
> > #endif
> > ```
> > 
> > And later:
> > 
> > ```
> > struct XRAY_TLS_ALIGNAS64 ThreadLocalData {
> > ```
> > 
> > and
> > 
> > ```
> > #ifdef XRAY_TLS_ALIGNED_SUPPORTED
> > static_assert(alignof(ThreadLocalData) >= 64,
> >   "ThreadLocalData must be cache line aligned.");
> > #endif
> > ```
> Maybe. Though i suppose it'd make more sense to make `64` an argument to the 
> macro.
```
#if !SANITIZER_NETBSD
#define XRAY_TLS_ALIGNAS(x) alignas(x)
#endif
```

```
struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
```

and

```
#ifdef XRAY_TLS_ALIGNAS
static_assert(alignof(ThreadLocalData) >= 64,
  "ThreadLocalData must be cache line aligned.");
#endif
```


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56000



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

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/xray/xray_basic_logging.cc:58
 
-struct alignas(64) ThreadLocalData {
+struct
+/* TLD is not aligned properly on NetBSD, resulting in segfault */

krytarowski wrote:
> mgorny wrote:
> > krytarowski wrote:
> > > Can we introduce a macro like:
> > > 
> > > ```
> > > #if SANITIZER_NETBSD
> > > #define XRAY_TLS_ALIGNAS64 /* Nor supported */
> > > #else
> > > #define XRAY_TLS_ALIGNED_SUPPORTED
> > > #define XRAY_TLS_ALIGNAS64 alignas(64)
> > > #endif
> > > ```
> > > 
> > > And later:
> > > 
> > > ```
> > > struct XRAY_TLS_ALIGNAS64 ThreadLocalData {
> > > ```
> > > 
> > > and
> > > 
> > > ```
> > > #ifdef XRAY_TLS_ALIGNED_SUPPORTED
> > > static_assert(alignof(ThreadLocalData) >= 64,
> > >   "ThreadLocalData must be cache line aligned.");
> > > #endif
> > > ```
> > Maybe. Though i suppose it'd make more sense to make `64` an argument to 
> > the macro.
> ```
> #if !SANITIZER_NETBSD
> #define XRAY_TLS_ALIGNAS(x) alignas(x)
> #endif
> ```
> 
> ```
> struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
> ```
> 
> and
> 
> ```
> #ifdef XRAY_TLS_ALIGNAS
> static_assert(alignof(ThreadLocalData) >= 64,
>   "ThreadLocalData must be cache line aligned.");
> #endif
> ```
Oops, actually not fully correct, but some variation of the above.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56000



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

r349924 - [AST][NFC] Pack CXXOperatorCallExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 08:51:57 2018
New Revision: 349924

URL: http://llvm.org/viewvc/llvm-project?rev=349924&view=rev
Log:
[AST][NFC] Pack CXXOperatorCallExpr

Use the space available in the bit-fields of Stmt.
This saves 8 bytes per CXXOperatorCallExpr. NFC.


Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=349924&r1=349923&r2=349924&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Fri Dec 21 08:51:57 2018
@@ -79,14 +79,8 @@ class CXXOperatorCallExpr final : public
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
-  /// The overloaded operator.
-  OverloadedOperatorKind Operator;
-
   SourceRange Range;
 
-  // Only meaningful for floating point types.
-  FPOptions FPFeatures;
-
   // CXXOperatorCallExpr has some trailing objects belonging
   // to CallExpr. See CallExpr for the details.
 
@@ -110,15 +104,17 @@ public:
   unsigned NumArgs, EmptyShell Empty);
 
   /// Returns the kind of overloaded operator that this expression refers to.
-  OverloadedOperatorKind getOperator() const { return Operator; }
+  OverloadedOperatorKind getOperator() const {
+return static_cast(
+CXXOperatorCallExprBits.OperatorKind);
+  }
 
   static bool isAssignmentOp(OverloadedOperatorKind Opc) {
-return Opc == OO_Equal || Opc == OO_StarEqual ||
-   Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
-   Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
-   Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
-   Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
-   Opc == OO_PipeEqual;
+return Opc == OO_Equal || Opc == OO_StarEqual || Opc == OO_SlashEqual ||
+   Opc == OO_PercentEqual || Opc == OO_PlusEqual ||
+   Opc == OO_MinusEqual || Opc == OO_LessLessEqual ||
+   Opc == OO_GreaterGreaterEqual || Opc == OO_AmpEqual ||
+   Opc == OO_CaretEqual || Opc == OO_PipeEqual;
   }
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
@@ -133,14 +129,15 @@ public:
   SourceLocation getOperatorLoc() const { return getRParenLoc(); }
 
   SourceLocation getExprLoc() const LLVM_READONLY {
+OverloadedOperatorKind Operator = getOperator();
 return (Operator < OO_Plus || Operator >= OO_Arrow ||
 Operator == OO_PlusPlus || Operator == OO_MinusMinus)
? getBeginLoc()
: getOperatorLoc();
   }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return Range.getBegin(); }
-  SourceLocation getEndLoc() const LLVM_READONLY { return Range.getEnd(); }
+  SourceLocation getBeginLoc() const { return Range.getBegin(); }
+  SourceLocation getEndLoc() const { return Range.getEnd(); }
   SourceRange getSourceRange() const { return Range; }
 
   static bool classof(const Stmt *T) {
@@ -149,14 +146,17 @@ public:
 
   // Set the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  void setFPFeatures(FPOptions F) { FPFeatures = F; }
-
-  FPOptions getFPFeatures() const { return FPFeatures; }
+  void setFPFeatures(FPOptions F) {
+CXXOperatorCallExprBits.FPFeatures = F.getInt();
+  }
+  FPOptions getFPFeatures() const {
+return FPOptions(CXXOperatorCallExprBits.FPFeatures);
+  }
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
   bool isFPContractableWithinStatement() const {
-return FPFeatures.allowFPContractWithinStatement();
+return getFPFeatures().allowFPContractWithinStatement();
   }
 };
 

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=349924&r1=349923&r2=349924&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Fri Dec 21 08:51:57 2018
@@ -532,6 +532,20 @@ protected:
 
   //===--- C++ Expression bitfields classes ---===//
 
+  class CXXOperatorCallExprBitfields {
+friend class ASTStmtReader;
+friend class CXXOperatorCallExpr;
+
+unsigned : NumCallExprBits;
+
+/// The kind of this overloaded operator. One of the enumerator
+/// value of OverloadedOperatorKind.
+unsigned OperatorKind : 6;
+
+// Only meaningful for floating point types.
+unsigned FPFeatures : 3;
+  };
+
   class CXXBoolLiteralExprBitfields {
 friend class CXXBoolLiteralExpr;
 
@@ -723,6 +737,7 @@ protected:
 PseudoObjectExprBitfields Ps

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 179297.

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

https://reviews.llvm.org/D56000

Files:
  lib/xray/xray_basic_logging.cc
  lib/xray/xray_defs.h
  lib/xray/xray_fdr_logging.cc


Index: lib/xray/xray_fdr_logging.cc
===
--- lib/xray/xray_fdr_logging.cc
+++ lib/xray/xray_fdr_logging.cc
@@ -51,7 +51,7 @@
 // call so that it can be initialized on first use instead of as a global. We
 // force the alignment to 64-bytes for x86 cache line alignment, as this
 // structure is used in the hot path of implementation.
-struct alignas(64) ThreadLocalData {
+struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
   BufferQueue::Buffer Buffer{};
   BufferQueue *BQ = nullptr;
 
@@ -124,8 +124,10 @@
 // critical section, calling a function that might be XRay instrumented (and
 // thus in turn calling into malloc by virtue of registration of the
 // thread_local's destructor).
+#if XRAY_HAS_TLS_ALIGNAS
 static_assert(alignof(ThreadLocalData) >= 64,
   "ThreadLocalData must be cache line aligned.");
+#endif
 static ThreadLocalData &getThreadLocalData() {
   thread_local typename std::aligned_storage<
   sizeof(ThreadLocalData), alignof(ThreadLocalData)>::type TLDStorage{};
Index: lib/xray/xray_defs.h
===
--- lib/xray/xray_defs.h
+++ lib/xray/xray_defs.h
@@ -19,4 +19,14 @@
 #define XRAY_NEVER_INSTRUMENT
 #endif
 
+#if !SANITIZER_NETBSD
+#define XRAY_TLS_ALIGNAS(x) alignas(x)
+#define XRAY_HAS_TLS_ALIGNAS 1
+#else
+// NetBSD: thread_local is not aligned properly, and the code relying
+// on it segfaults
+#define XRAY_TLS_ALIGNAS(x)
+#define XRAY_HAS_TLS_ALIGNAS 0
+#endif
+
 #endif  // XRAY_XRAY_DEFS_H
Index: lib/xray/xray_basic_logging.cc
===
--- lib/xray/xray_basic_logging.cc
+++ lib/xray/xray_basic_logging.cc
@@ -55,7 +55,7 @@
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
 
-struct alignas(64) ThreadLocalData {
+struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
   void *InMemoryBuffer = nullptr;
   size_t BufferSize = 0;
   size_t BufferOffset = 0;


Index: lib/xray/xray_fdr_logging.cc
===
--- lib/xray/xray_fdr_logging.cc
+++ lib/xray/xray_fdr_logging.cc
@@ -51,7 +51,7 @@
 // call so that it can be initialized on first use instead of as a global. We
 // force the alignment to 64-bytes for x86 cache line alignment, as this
 // structure is used in the hot path of implementation.
-struct alignas(64) ThreadLocalData {
+struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
   BufferQueue::Buffer Buffer{};
   BufferQueue *BQ = nullptr;
 
@@ -124,8 +124,10 @@
 // critical section, calling a function that might be XRay instrumented (and
 // thus in turn calling into malloc by virtue of registration of the
 // thread_local's destructor).
+#if XRAY_HAS_TLS_ALIGNAS
 static_assert(alignof(ThreadLocalData) >= 64,
   "ThreadLocalData must be cache line aligned.");
+#endif
 static ThreadLocalData &getThreadLocalData() {
   thread_local typename std::aligned_storage<
   sizeof(ThreadLocalData), alignof(ThreadLocalData)>::type TLDStorage{};
Index: lib/xray/xray_defs.h
===
--- lib/xray/xray_defs.h
+++ lib/xray/xray_defs.h
@@ -19,4 +19,14 @@
 #define XRAY_NEVER_INSTRUMENT
 #endif
 
+#if !SANITIZER_NETBSD
+#define XRAY_TLS_ALIGNAS(x) alignas(x)
+#define XRAY_HAS_TLS_ALIGNAS 1
+#else
+// NetBSD: thread_local is not aligned properly, and the code relying
+// on it segfaults
+#define XRAY_TLS_ALIGNAS(x)
+#define XRAY_HAS_TLS_ALIGNAS 0
+#endif
+
 #endif  // XRAY_XRAY_DEFS_H
Index: lib/xray/xray_basic_logging.cc
===
--- lib/xray/xray_basic_logging.cc
+++ lib/xray/xray_basic_logging.cc
@@ -55,7 +55,7 @@
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
 
-struct alignas(64) ThreadLocalData {
+struct XRAY_TLS_ALIGNAS(64) ThreadLocalData {
   void *InMemoryBuffer = nullptr;
   size_t BufferSize = 0;
   size_t BufferOffset = 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Michał Górny via Phabricator via cfe-commits]

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

Updated as requested.


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

https://reviews.llvm.org/D56000



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

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete added a comment.

Thanks for all the feedback so far.  Is there anything else you'd like me to 
change before I can land this?


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

https://reviews.llvm.org/D55869



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

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/xray/xray_defs.h:22
 
+#if !SANITIZER_NETBSD
+#define XRAY_TLS_ALIGNAS(x) alignas(x)

I would switch the order, in order to remove unneeded negation.

```
#if SANITIZER_NETBSD
...
#else
...
#endif
```

`#define XRAY_HAS_TLS_ALIGNAS 0` is not needed as `#if XRAY_HAS_TLS_ALIGNAS` 
will evaluate to false anyway and `#define XRAY_HAS_TLS_ALIGNAS 1` is 
equivalent to `#define XRAY_HAS_TLS_ALIGNAS`. But it's just a matter of style.


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

https://reviews.llvm.org/D56000



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added a comment.

I think Aaron and John have covered any comments I would make! I'm glad this 
came out so simply, though. I was afraid it'd be a much larger change than just 
expanding what was already there.


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

https://reviews.llvm.org/D55865



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

[clang-tools-extra] r349930 - [clang-tidy] Add export-fixes flag to clang-tidy-diff

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri Dec 21 09:25:27 2018
New Revision: 349930

URL: http://llvm.org/viewvc/llvm-project?rev=349930&view=rev
Log:
[clang-tidy] Add export-fixes flag to clang-tidy-diff

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=349930&r1=349929&r2=349930&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Fri Dec 21 
09:25:27 2018
@@ -57,6 +57,9 @@ def main():
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@ def main():
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:


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

[PATCH] D55848: [clang-tidy] Add export-fixes flag to clang-tidy-diff

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349930: [clang-tidy] Add export-fixes flag to 
clang-tidy-diff (authored by juliehockett, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55848?vs=178757&id=179301#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55848

Files:
  clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py


Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:


Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -57,6 +57,9 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
+  parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-extra-arg', dest='extra_arg',
   action='append', default=[],
   help='Additional argument to append to the compiler '
@@ -122,6 +125,8 @@
   command.append('-line-filter=' + quote + line_filter_json + quote)
   if args.fix:
 command.append('-fix')
+  if args.export_fixes:
+command.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

[Phil Camp via Phabricator via cfe-commits]

FlameTop added a comment.

I'm afraid we are seeing a build failure here on our local Windows checking 
MSVC build. Unfortunately I cannot find a public buildbot that uses the exact 
configuration that causes the assertion. The assertion we are seeing is

Assertion failed: isa(Val) && "cast() argument of incompatible type!", 
file F:\merge3\o\llvm\include\llvm/Support/Casting.h, line 255

It would appear it will not allow a cast from 'Init' to 'BitInit'. There is a 
very similar routine in the file X86EX2VEXTTablesEmitter.cpp as follows

static inline uint64_t getValueFromBitsInit(const BitsInit *B) {
uint64_t Value = 0;
for (unsigned i = 0, e = B->getNumBits(); i != e; ++i) {

  if (BitInit *Bit = dyn_cast(B->getBit(i)))
Value |= uint64_t(Bit->getValue()) << i;
  else
PrintFatalError("Invalid VectSize bit");

}
return Value;
}

Which appears to serve the same purpose but uses a dynamic cast. If I replace 
your routine with this technique our build succeeds.

regards

Phil Camp, SIE


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

https://reviews.llvm.org/D55792



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

r349933 - [Sema][NFC] Fix Wimplicit-fallthrough warning in getCursorKindForDecl

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 09:52:13 2018
New Revision: 349933

URL: http://llvm.org/viewvc/llvm-project?rev=349933&view=rev
Log:
[Sema][NFC] Fix Wimplicit-fallthrough warning in getCursorKindForDecl

All cases are covered so add an llvm_unreachable. NFC.


Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=349933&r1=349932&r2=349933&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Fri Dec 21 09:52:13 2018
@@ -3447,6 +3447,7 @@ CXCursorKind clang::getCursorKindForDecl
 case ObjCPropertyImplDecl::Synthesize:
   return CXCursor_ObjCSynthesizeDecl;
 }
+llvm_unreachable("Unexpected Kind!");
 
   case Decl::Import:
 return CXCursor_ModuleImportDecl;


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

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 179305.
tejohnson added a comment.

Implement comment suggestions. Added test for new error. Also test changes for 
LLVM side changes in D53890 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D53891

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-distributed-cfi-devirt.ll
  test/CodeGen/thinlto-distributed-cfi.ll
  test/CodeGenCXX/no-lto-unit.cpp
  test/CodeGenCXX/type-metadata-thinlto.cpp
  test/Driver/split-lto-unit.c

Index: test/Driver/split-lto-unit.c
===
--- /dev/null
+++ test/Driver/split-lto-unit.c
@@ -0,0 +1,10 @@
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fsplit-lto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=ERROR1 %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fsanitize=cfi 2>&1 | FileCheck --check-prefix=ERROR2 %s
+
+// UNIT: "-fsplit-lto-unit"
+// NOUNIT-NOT: "-fsplit-lto-unit"
+// ERROR1: error: invalid argument '-fno-split-lto-unit' not allowed with '-fwhole-program-vtables'
+// ERROR2: error: invalid argument '-fno-split-lto-unit' not allowed with '-fsanitize=cfi'
Index: test/CodeGenCXX/type-metadata-thinlto.cpp
===
--- test/CodeGenCXX/type-metadata-thinlto.cpp
+++ test/CodeGenCXX/type-metadata-thinlto.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
+// RUN: %clang_cc1 -flto=thin -flto-unit -fsplit-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+// RUN: llvm-modextract -b -o - -n 1 %t | llvm-bcanalyzer -dump | FileCheck %s --check-prefix=LTOUNIT
+// LTOUNIT: 
 
 // CHECK: @_ZTV1A = linkonce_odr
 class A {
Index: test/CodeGenCXX/no-lto-unit.cpp
===
--- test/CodeGenCXX/no-lto-unit.cpp
+++ test/CodeGenCXX/no-lto-unit.cpp
@@ -2,6 +2,8 @@
 // RUN: llvm-dis -o - %t | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -flto-unit -fno-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-dis -o - %t | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s --check-prefix=NOLTOUNIT
+// NOLTOUNIT: 
 
 // CHECK-NOT: !type
 class A {
Index: test/CodeGen/thinlto-distributed-cfi.ll
===
--- test/CodeGen/thinlto-distributed-cfi.ll
+++ test/CodeGen/thinlto-distributed-cfi.ll
@@ -2,7 +2,7 @@
 
 ; Backend test for distribute ThinLTO with CFI.
 
-; RUN: opt -thinlto-bc -o %t.o %s
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
 ; RUN:   -o %t2.index \
Index: test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -4,7 +4,7 @@
 ; It additionally enables -fwhole-program-vtables to get more information in
 ; TYPE_IDs of GLOBALVAL_SUMMARY_BLOCK.
 
-; RUN: opt -thinlto-bc -o %t.o %s
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436.
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -906,6 +906,7 @@
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S;
   }
   Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false);
+  Opts.EnableSplitLTOUnit = Args.hasArg(OPT_fsplit_lto_unit);
   if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) {
 if (IK.getLanguage() != InputKind::LLVM_IR)
   Diags.Report(diag::err_drv_argument_only_allowed_with)
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5206,6 +5206,21 @@
 CmdArgs.push_back("-fwhole-program-vtables");
  

r349934 - [AST][NFC] Fix Wsign-compare warning introduced in CXXOperatorCallExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 09:54:51 2018
New Revision: 349934

URL: http://llvm.org/viewvc/llvm-project?rev=349934&view=rev
Log:
[AST][NFC] Fix Wsign-compare warning introduced in CXXOperatorCallExpr


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

Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=349934&r1=349933&r2=349934&view=diff
==
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Fri Dec 21 09:54:51 2018
@@ -488,8 +488,9 @@ CXXOperatorCallExpr::CXXOperatorCallExpr
OperatorLoc, /*MinNumArgs=*/0, UsesADL) {
   CXXOperatorCallExprBits.OperatorKind = OpKind;
   CXXOperatorCallExprBits.FPFeatures = FPFeatures.getInt();
-  assert((CXXOperatorCallExprBits.OperatorKind == OpKind) &&
- "OperatorKind overflow!");
+  assert(
+  (CXXOperatorCallExprBits.OperatorKind == static_cast(OpKind)) 
&&
+  "OperatorKind overflow!");
   assert((CXXOperatorCallExprBits.FPFeatures == FPFeatures.getInt()) &&
  "FPFeatures overflow!");
   Range = getSourceRangeImpl();


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

[PATCH] D56012: Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via Phabricator via cfe-commits]

hwright created this revision.
hwright added reviewers: aaron.ballman, JonasToth, alexfh, hokein.
hwright added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Previously, we'd only match on literal floating or integral zeroes, but I've 
now also learned that some users spell that value as `int{0}` or `float{0}`, 
which also need to be matched.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56012

Files:
  clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  test/clang-tidy/abseil-duration-factory-scale.cpp


Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -1,5 +1,6 @@
 // RUN: %check_clang_tidy %s abseil-duration-factory-scale %t -- -- -I%S/Inputs
 
+#include 
 #include "absl/time/time.h"
 
 void ScaleTest() {
@@ -30,6 +31,15 @@
   d = absl::Seconds(0x0.01p-126f);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
   // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int64_t{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(float{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
 
   // Fold seconds into minutes
   d = absl::Seconds(30 * 60);
@@ -83,6 +93,8 @@
 
   // None of these should trigger the check
   d = absl::Seconds(60);
+  d = absl::Seconds(int{60});
+  d = absl::Seconds(float{60});
   d = absl::Seconds(60 + 30);
   d = absl::Seconds(60 - 30);
   d = absl::Seconds(50 * 30);
Index: clang-tidy/abseil/DurationRewriter.cpp
===
--- clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tidy/abseil/DurationRewriter.cpp
@@ -105,14 +105,37 @@
   llvm_unreachable("unknown scaling factor");
 }
 
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return (N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder));
+}
+
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.
 bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
-  return selectFirst(
- "val",
- match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)),
-   floatLiteral(equals(0.0)
-   .bind("val"),
-   Node, *Result.Context)) != nullptr;
+  auto ZeroMatcher =
+  anyOf(integerLiteral(equals(0)), floatLiteral(equals(0.0)));
+
+  // Check to see if we're using a zero directly.
+  if (selectFirst(
+  "val", match(expr(ignoringImpCasts(ZeroMatcher)).bind("val"), Node,
+   *Result.Context)) != nullptr)
+return true;
+
+  // Now check to see if we're using a functional cast with a scalar
+  // initializer expression, e.g. `int{0}`.
+  if (selectFirst(
+  "val",
+  match(cxxFunctionalCastExpr(
+hasDestinationType(
+anyOf(isInteger(), realFloatingPointType())),
+hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher
+.bind("val"),
+Node, *Result.Context)) != nullptr)
+return true;
+
+  return false;
 }
 
 llvm::Optional
Index: clang-tidy/abseil/DurationFactoryScaleCheck.cpp
===
--- clang-tidy/abseil/DurationFactoryScaleCheck.cpp
+++ clang-tidy/abseil/DurationFactoryScaleCheck.cpp
@@ -123,6 +123,10 @@
   hasArgument(
   0,
   ignoringImpCasts(anyOf(
+  cxxFunctionalCastExpr(
+  hasDestinationType(
+  anyOf(isInteger(), realFloatingPointType())),
+  hasSourceExpression(initListExpr())),
   integerLiteral(equals(0)), floatLiteral(equals(0.0)),
   binaryOperator(hasOperatorName("*"),
  hasEitherOperand(ignoringImpCasts(


Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -1,5 +1,6 @@
 // RUN: %c

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: test/clang-tidy/abseil-duration-factory-scale.cpp:3
 
+#include 
 #include "absl/time/time.h"

cstdint, please.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56012



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

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 179318.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Updated based on review feedback.

Moved `FunctionDecl::hasUnusedResultAttr()` and 
`FunctionDecl::getUnusedResultAttr()` to `CallExpr` to reduce logic duplication 
on checking.


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

https://reviews.llvm.org/D55949

Files:
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/Sema/SemaStmt.cpp
  test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

Index: test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
===
--- test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
+++ test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
@@ -32,6 +32,35 @@
   // OK, warning suppressed.
   (void)fp();
 }
+
+namespace PR31526 {
+typedef E (*fp1)();
+typedef S (*fp2)();
+
+typedef S S_alias;
+typedef S_alias (*fp3)();
+
+typedef fp2 fp2_alias;
+
+void f() {
+  fp1 one;
+  fp2 two;
+  fp3 three;
+  fp2_alias four;
+
+  one(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  two(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  three(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  four(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  // These are all okay because of the explicit cast to void.
+  (void)one();
+  (void)two();
+  (void)three();
+  (void)four();
+}
+} // namespace PR31526
+
 #ifdef EXT
 // expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -259,17 +259,16 @@
 if (E->getType()->isVoidType())
   return;
 
+if (const Attr *A = CE->getUnusedResultAttr(Context)) {
+  Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+  return;
+}
+
 // If the callee has attribute pure, const, or warn_unused_result, warn with
 // a more specific message to make it clear what is happening. If the call
 // is written in a macro body, only warn if it has the warn_unused_result
 // attribute.
 if (const Decl *FD = CE->getCalleeDecl()) {
-  if (const Attr *A = isa(FD)
-  ? cast(FD)->getUnusedResultAttr()
-  : FD->getAttr()) {
-Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
-return;
-  }
   if (ShouldSuppress)
 return;
   if (FD->hasAttr()) {
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1363,6 +1363,19 @@
   return FnType->getReturnType();
 }
 
+const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the return type is a struct, union, or enum that is marked nodiscard,
+  // then return the return type attribute.
+  if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl())
+if (const auto *A = TD->getAttr())
+  return A;
+
+  // Otherwise, see if the callee is marked nodiscard and return that attribute
+  // instead.
+  const Decl *D = getCalleeDecl();
+  return D ? D->getAttr() : nullptr;
+}
+
 SourceLocation CallExpr::getBeginLoc() const {
   if (isa(this))
 return cast(this)->getBeginLoc();
@@ -2274,16 +2287,12 @@
 // If this is a direct call, get the callee.
 const CallExpr *CE = cast(this);
 if (const Decl *FD = CE->getCalleeDecl()) {
-  const FunctionDecl *Func = dyn_cast(FD);
-  bool HasWarnUnusedResultAttr = Func ? Func->hasUnusedResultAttr()
-  : FD->hasAttr();
-
   // If the callee has attribute pure, const, or warn_unused_result, warn
   // about it. void foo() { strlen("bar"); } should warn.
   //
   // Note: If new cases are added here, DiagnoseUnusedExprResult should be
   // updated to match for QoI.
-  if (HasWarnUnusedResultAttr ||
+  if (CE->hasUnusedResultAttr(Ctx) ||
   FD->hasAttr() || FD->hasAttr()) {
 WarnE = this;
 Loc = CE->getCallee()->getBeginLoc();
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3231,20 +3231,6 @@
   return FTL.getExceptionSpecRange();
 }
 
-const Attr *FunctionDecl::getUnusedResultAttr() const {
-  QualType RetType = getReturnType();
-  if (const auto *Ret = RetType->getAsRecordDecl()) {
-if (const auto *R = Ret->getAttr())
-  return R;
-  } else if (const auto *ET = RetType->getAs()) {
-if (const EnumDecl *ED = ET->getDecl()) {
-  if (const auto *R = ED

r349938 - [analyzer] Correct the summary violation diagnostics for the retain count checker

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec 21 11:13:28 2018
New Revision: 349938

URL: http://llvm.org/viewvc/llvm-project?rev=349938&view=rev
Log:
[analyzer] Correct the summary violation diagnostics for the retain count 
checker

It should be in the past tense.

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=349938&r1=349937&r2=349938&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 Fri Dec 21 11:13:28 2018
@@ -285,7 +285,7 @@ annotateConsumedSummaryMismatch(const Ex
 os << "Parameter '";
 PVD->getNameForDiagnostic(os, PVD->getASTContext().getPrintingPolicy(),
   /*Qualified=*/false);
-os << "' is marked as consuming, but the function does not consume "
+os << "' is marked as consuming, but the function did not consume "
<< "the reference\n";
   }
 }

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=349938&r1=349937&r2=349938&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Dec 21 11:13:28 2018
@@ -99,7 +99,7 @@ bool os_consume_violation_two_args(OS_CO
 escape(obj);
 return true;
   }
-  return false; // expected-note{{Parameter 'obj' is marked as consuming, but 
the function does not consume the reference}}
+  return false; // expected-note{{Parameter 'obj' is marked as consuming, but 
the function did not consume the reference}}
 }
 
 bool os_consume_violation(OS_CONSUME OSObject *obj) {
@@ -108,7 +108,7 @@ bool os_consume_violation(OS_CONSUME OSO
 escape(obj);
 return true;
   }
-  return false; // expected-note{{Parameter 'obj' is marked as consuming, but 
the function does not consume the reference}}
+  return false; // expected-note{{Parameter 'obj' is marked as consuming, but 
the function did not consume the reference}}
 }
 
 void os_consume_ok(OS_CONSUME OSObject *obj) {


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

r349939 - Revert "Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions"

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec 21 11:13:40 2018
New Revision: 349939

URL: http://llvm.org/viewvc/llvm-project?rev=349939&view=rev
Log:
Revert "Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in 
RetainCountChecker on type mismatch even for inlined functions"

This reverts commit b44b33f6e020a2c369da2b0c1d53cd52975f2526.

Revert the revert with the fix.

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=349939&r1=349938&r2=349939&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Fri Dec 21 11:13:40 2018
@@ -502,6 +502,25 @@ static Optional refValFromRetEff
   return None;
 }
 
+static bool isPointerToObject(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull())
+if (PT->getAsCXXRecordDecl())
+  return true;
+  return false;
+}
+
+/// Whether the tracked value should be escaped on a given call.
+/// OSObjects are escaped when passed to void * / etc.
+static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+   const RefVal *TrackedValue) {
+  if (TrackedValue->getObjKind() != RetEffect::OS)
+return false;
+  if (ArgIdx >= CE.parameters().size())
+return false;
+  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
+}
+
 // We don't always get the exact modeling of the function with regards to the
 // retain count checker even when the function is inlined. For example, we need
 // to stop tracking the symbols which were marked with StopTrackingHard.
@@ -512,11 +531,16 @@ void RetainCountChecker::processSummaryO
 
   // Evaluate the effect of the arguments.
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
-if (Summ.getArg(idx) == StopTrackingHard) {
-  SVal V = CallOrMsg.getArgSVal(idx);
-  if (SymbolRef Sym = V.getAsLocSymbol()) {
+SVal V = CallOrMsg.getArgSVal(idx);
+
+if (SymbolRef Sym = V.getAsLocSymbol()) {
+  bool ShouldRemoveBinding = Summ.getArg(idx) == StopTrackingHard;
+  if (const RefVal *T = getRefBinding(state, Sym))
+if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
+  ShouldRemoveBinding = true;
+
+  if (ShouldRemoveBinding)
 state = removeRefBinding(state, Sym);
-  }
 }
   }
 
@@ -574,25 +598,6 @@ static ProgramStateRef updateOutParamete
   return State;
 }
 
-static bool isPointerToObject(QualType QT) {
-  QualType PT = QT->getPointeeType();
-  if (!PT.isNull())
-if (PT->getAsCXXRecordDecl())
-  return true;
-  return false;
-}
-
-/// Whether the tracked value should be escaped on a given call.
-/// OSObjects are escaped when passed to void * / etc.
-static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
-   const RefVal *TrackedValue) {
-  if (TrackedValue->getObjKind() != RetEffect::OS)
-return false;
-  if (ArgIdx >= CE.parameters().size())
-return false;
-  return !isPointerToObject(CE.parameters()[ArgIdx]->getType());
-}
-
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   const CallEvent &CallOrMsg,
   CheckerContext &C) const {

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=349939&r1=349938&r2=349939&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Dec 21 11:13:40 2018
@@ -91,6 +91,7 @@ struct OSMetaClassBase {
 };
 
 void escape(void *);
+void escape_with_source(void *p) {}
 bool coin();
 
 bool os_consume_violation_two_args(OS_CONSUME OSObject *obj, bool extra) {
@@ -139,6 +140,13 @@ void test_escaping_into_voidstar() {
   escape(obj);
 }
 
+void test_escape_has_source() {
+  OSObject *obj = new OSObject;
+  if (obj)
+escape_with_source((MYTYPE)obj);
+  return;
+}
+
 void test_no_infinite_check_recursion(MyArray *arr) {
   OSObject *input = new OSObject;
   OSObject *o = arr->generateObject(input);


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

r349940 - Switch from cast<> to dyn_cast<>.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Dec 21 11:16:38 2018
New Revision: 349940

URL: http://llvm.org/viewvc/llvm-project?rev=349940&view=rev
Log:
Switch from cast<> to dyn_cast<>.

This avoids a potential failed assertion that is happening on someone's 
out-of-tree build.

Modified:
cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp?rev=349940&r1=349939&r2=349940&view=diff
==
--- cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp Fri Dec 21 11:16:38 2018
@@ -63,8 +63,11 @@ static inline uint64_t getValueFromBitsI
 
   uint64_t Value = 0;
   for (unsigned i = 0, e = B->getNumBits(); i != e; ++i) {
-const auto *Bit = cast(B->getBit(i));
-Value |= uint64_t(Bit->getValue()) << i;
+const auto *Bit = dyn_cast(B->getBit(i));
+if (Bit)
+  Value |= uint64_t(Bit->getValue()) << i;
+else
+  PrintFatalError("Invalid bits");
   }
   return Value;
 }


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

Re: [PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

[Aaron Ballman via cfe-commits]

On Fri, Dec 21, 2018 at 12:35 PM Phil Camp via Phabricator
 wrote:
>
> FlameTop added a comment.
>
> I'm afraid we are seeing a build failure here on our local Windows checking 
> MSVC build. Unfortunately I cannot find a public buildbot that uses the exact 
> configuration that causes the assertion. The assertion we are seeing is
>
> Assertion failed: isa(Val) && "cast() argument of incompatible type!", 
> file F:\merge3\o\llvm\include\llvm/Support/Casting.h, line 255
>
> It would appear it will not allow a cast from 'Init' to 'BitInit'. There is a 
> very similar routine in the file X86EX2VEXTTablesEmitter.cpp as follows
>
> static inline uint64_t getValueFromBitsInit(const BitsInit *B) {
> uint64_t Value = 0;
> for (unsigned i = 0, e = B->getNumBits(); i != e; ++i) {
>
>   if (BitInit *Bit = dyn_cast(B->getBit(i)))
> Value |= uint64_t(Bit->getValue()) << i;
>   else
> PrintFatalError("Invalid VectSize bit");
>
> }
> return Value;
> }
>
> Which appears to serve the same purpose but uses a dynamic cast. If I replace 
> your routine with this technique our build succeeds.

Thank you for the note, I've updated this code in r349940 to use a
dyn_cast instead.

~Aaron

>
> regards
>
> Phil Camp, SIE
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55792/new/
>
> https://reviews.llvm.org/D55792
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same()), 
int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
 static_assert(std::is_const::value, "message");

Any idea why the `std::` was dropped here?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

r349942 - Remove stat cache chaining as it's no longer needed after PTH support has been

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Dec 21 11:33:09 2018
New Revision: 349942

URL: http://llvm.org/viewvc/llvm-project?rev=349942&view=rev
Log:
Remove stat cache chaining as it's no longer needed after PTH support has been
removed

Stat cache chaining was implemented for a StatListener in the PTH writer so that
it could write out the stat information to PTH. r348266 removed support for PTH,
and it doesn't seem like there are other uses of stat cache chaining. We can
remove the chaining support.

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

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/FileSystemStatCache.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=349942&r1=349941&r2=349942&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Dec 21 11:33:09 2018
@@ -192,18 +192,10 @@ public:
   ///
   /// \param statCache the new stat cache to install. Ownership of this
   /// object is transferred to the FileManager.
-  ///
-  /// \param AtBeginning whether this new stat cache must be installed at the
-  /// beginning of the chain of stat caches. Otherwise, it will be added to
-  /// the end of the chain.
-  void addStatCache(std::unique_ptr statCache,
-bool AtBeginning = false);
-
-  /// Removes the specified FileSystemStatCache object from the manager.
-  void removeStatCache(FileSystemStatCache *statCache);
+  void setStatCache(std::unique_ptr statCache);
 
-  /// Removes all FileSystemStatCache objects from the manager.
-  void clearStatCaches();
+  /// Removes the FileSystemStatCache object from the manager.
+  void clearStatCache();
 
   /// Lookup, cache, and verify the specified directory (real or
   /// virtual).

Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=349942&r1=349941&r2=349942&view=diff
==
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Fri Dec 21 11:33:09 2018
@@ -60,9 +60,6 @@ struct FileData {
 class FileSystemStatCache {
   virtual void anchor();
 
-protected:
-  std::unique_ptr NextStatCache;
-
 public:
   virtual ~FileSystemStatCache() = default;
 
@@ -88,22 +85,6 @@ public:
   std::unique_ptr *F,
   FileSystemStatCache *Cache, llvm::vfs::FileSystem &FS);
 
-  /// Sets the next stat call cache in the chain of stat caches.
-  /// Takes ownership of the given stat cache.
-  void setNextStatCache(std::unique_ptr Cache) {
-NextStatCache = std::move(Cache);
-  }
-
-  /// Retrieve the next stat call cache in the chain.
-  FileSystemStatCache *getNextStatCache() { return NextStatCache.get(); }
-
-  /// Retrieve the next stat call cache in the chain, transferring
-  /// ownership of this cache (and, transitively, all of the remaining caches)
-  /// to the caller.
-  std::unique_ptr takeNextStatCache() {
-return std::move(NextStatCache);
-  }
-
 protected:
   // FIXME: The pointer here is a non-owning/optional reference to the
   // unique_ptr. Optional&> might be nicer, but
@@ -111,17 +92,6 @@ protected:
   virtual LookupResult getStat(StringRef Path, FileData &Data, bool isFile,
std::unique_ptr *F,
llvm::vfs::FileSystem &FS) = 0;
-
-  LookupResult statChained(StringRef Path, FileData &Data, bool isFile,
-   std::unique_ptr *F,
-   llvm::vfs::FileSystem &FS) {
-if (FileSystemStatCache *Next = getNextStatCache())
-  return Next->getStat(Path, Data, isFile, F, FS);
-
-// If we hit the end of the list of stat caches to try, just compute and
-// return it without a cache.
-return get(Path, Data, isFile, F, nullptr, FS) ? CacheMissing : 
CacheExists;
-  }
 };
 
 /// A stat "cache" that can be used by FileManager to keep

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=349942&r1=349941&r2=349942&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Dec 21 11:33:09 2018
@@ -63,44 +63,12 @@ FileManager::FileManager(const FileSyste
 
 FileManager::~FileManager() = default;
 
-void FileManager::addStatCache(std::unique_ptr statCache,
-   bool AtBeginning) {
+void File

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349942: Remove stat cache chaining as it's no longer 
needed after PTH support has been (authored by arphaman, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55455?vs=177288&id=179323#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55455

Files:
  cfe/trunk/include/clang/Basic/FileManager.h
  cfe/trunk/include/clang/Basic/FileSystemStatCache.h
  cfe/trunk/lib/Basic/FileManager.cpp
  cfe/trunk/lib/Basic/FileSystemStatCache.cpp
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/unittests/Basic/FileManagerTest.cpp

Index: cfe/trunk/lib/Basic/FileManager.cpp
===
--- cfe/trunk/lib/Basic/FileManager.cpp
+++ cfe/trunk/lib/Basic/FileManager.cpp
@@ -63,44 +63,12 @@
 
 FileManager::~FileManager() = default;
 
-void FileManager::addStatCache(std::unique_ptr statCache,
-   bool AtBeginning) {
+void FileManager::setStatCache(std::unique_ptr statCache) {
   assert(statCache && "No stat cache provided?");
-  if (AtBeginning || !StatCache.get()) {
-statCache->setNextStatCache(std::move(StatCache));
-StatCache = std::move(statCache);
-return;
-  }
-
-  FileSystemStatCache *LastCache = StatCache.get();
-  while (LastCache->getNextStatCache())
-LastCache = LastCache->getNextStatCache();
-
-  LastCache->setNextStatCache(std::move(statCache));
+  StatCache = std::move(statCache);
 }
 
-void FileManager::removeStatCache(FileSystemStatCache *statCache) {
-  if (!statCache)
-return;
-
-  if (StatCache.get() == statCache) {
-// This is the first stat cache.
-StatCache = StatCache->takeNextStatCache();
-return;
-  }
-
-  // Find the stat cache in the list.
-  FileSystemStatCache *PrevCache = StatCache.get();
-  while (PrevCache && PrevCache->getNextStatCache() != statCache)
-PrevCache = PrevCache->getNextStatCache();
-
-  assert(PrevCache && "Stat cache not found for removal");
-  PrevCache->setNextStatCache(statCache->takeNextStatCache());
-}
-
-void FileManager::clearStatCaches() {
-  StatCache.reset();
-}
+void FileManager::clearStatCache() { StatCache.reset(); }
 
 /// Retrieve the directory that the given file name resides in.
 /// Filename can point to either a real file or a virtual file.
Index: cfe/trunk/lib/Basic/FileSystemStatCache.cpp
===
--- cfe/trunk/lib/Basic/FileSystemStatCache.cpp
+++ cfe/trunk/lib/Basic/FileSystemStatCache.cpp
@@ -114,18 +114,17 @@
 MemorizeStatCalls::getStat(StringRef Path, FileData &Data, bool isFile,
std::unique_ptr *F,
llvm::vfs::FileSystem &FS) {
-  LookupResult Result = statChained(Path, Data, isFile, F, FS);
-
-  // Do not cache failed stats, it is easy to construct common inconsistent
-  // situations if we do, and they are not important for PCH performance (which
-  // currently only needs the stats to construct the initial FileManager
-  // entries).
-  if (Result == CacheMissing)
-return Result;
+  if (get(Path, Data, isFile, F, nullptr, FS)) {
+// Do not cache failed stats, it is easy to construct common inconsistent
+// situations if we do, and they are not important for PCH performance
+// (which currently only needs the stats to construct the initial
+// FileManager entries).
+return CacheMissing;
+  }
 
   // Cache file 'stat' results and directories with absolutely paths.
   if (!Data.IsDirectory || llvm::sys::path::is_absolute(Path))
 StatCalls[Path] = Data;
 
-  return Result;
+  return CacheExists;
 }
Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -369,7 +369,7 @@
 
   const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
 
-  Files->clearStatCaches();
+  Files->clearStatCache();
   return Success;
 }
 
Index: cfe/trunk/unittests/Basic/FileManagerTest.cpp
===
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp
@@ -111,7 +111,7 @@
   // FileManager to report "file/directory doesn't exist".  This
   // avoids the possibility of the result of this test being affected
   // by what's in the real file system.
-  manager.addStatCache(llvm::make_unique());
+  manager.setStatCache(llvm::make_unique());
 
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
   EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir"));
@@ -121,7 +121,7 @@
 // When a virtual file is added, all of its ancestors should be created.
 TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   // Fake an empty real file system.
-  m

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2650
+Software Pipelining optimization is a technique used to optimize loops by
+  utilizing instructions level parallelism. It reorders loop instructions to
+  overlap iterations. As the result next iteration started before previous

instructions level -> instruction-level



Comment at: include/clang/Basic/AttrDocs.td:2651
+  utilizing instructions level parallelism. It reorders loop instructions to
+  overlap iterations. As the result next iteration started before previous
+  have finished. The Modulo Scheduling technique creates schedule for one

As a result, the next iteration starts before the previous iteration has 
finished.



Comment at: include/clang/Basic/AttrDocs.td:2652-2654
+  have finished. The Modulo Scheduling technique creates schedule for one
+  iteration such that when repeating at regular interval no inter-iteration
+  dependence violated. This constant interval(in cycles) between the start

The module scheduling technique creates a schedule for one iteration such that 
when repeating at regular intervals, no inter-iteration dependencies are 
violated.



Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

is called the initiation interval.

I can't quite parse the second sentence, though. Is it trying to say that the 
initiation interval is the number of cycles for a single iteration of the 
optimized loop?



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

I'm can't quite parse this sentence either. Is it trying to say that the 
scheduler will try to find a loop iteration cycle count that most-closely 
matches the specified initiation interval?



Comment at: include/clang/Basic/AttrDocs.td:2677
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:
+

The initiation interval must be a positive number greater than zero.


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

https://reviews.llvm.org/D55710



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

[PATCH] D54565: Introduce `-Wctad` as a subgroup of `-Wc++14-compat`

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D54565#1331293 , @Quuxplusone wrote:

> @rsmith ping!
>
> @thakis: You said, //"From a user point of view, I have no idea what "CTAD" 
> means, and I sometimes work on a C++ compiler."// Did you mean "I think the 
> command-line option should be named something more verbose than 
> `-W[c++14-compat-]ctad`"? Or "I don't know what CTAD is and I would indeed 
> like a diagnostic if I use it accidentally"? Or something else?


The former. Since I don't know what it means, I don't know if I want a 
diagnostic for it :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54565



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

r349943 - [analyzer] Tests quickfix.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Dec 21 11:40:44 2018
New Revision: 349943

URL: http://llvm.org/viewvc/llvm-project?rev=349943&view=rev
Log:
[analyzer] Tests quickfix.

Modified:
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=349943&r1=349942&r2=349943&view=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Fri Dec 21 11:40:44 2018
@@ -143,7 +143,7 @@ void test_escaping_into_voidstar() {
 void test_escape_has_source() {
   OSObject *obj = new OSObject;
   if (obj)
-escape_with_source((MYTYPE)obj);
+escape_with_source(obj);
   return;
 }
 


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

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit updated this revision to Diff 179325.
tabloid.adroit added a comment.

- [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer
- add test
- update


Repository:
  rC Clang

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

https://reviews.llvm.org/D55915

Files:
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/cl-options.c
  test/Driver/frame-pointer-elim.c


Index: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -69,5 +69,30 @@
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=OMIT_LEAF %s
 
+// RUN: %clang -### -S -Os -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO-OMIT-ALL %s
+// NO-OMIT-ALL: "-mdisable-fp-elim"
+// NO-OMIT-ALL-NOT: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -S -Os -fno-omit-frame-pointer -momit-leaf-frame-pointer 
%s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO-OMIT-NONLEAF %s
+// NO-OMIT-NONLEAF: "-mdisable-fp-elim"
+// NO-OMIT-NONLEAF: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -S -Os -fomit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=OMIT-ALL %s
+// OMIT-ALL-NOT: "-mdisable-fp-elim"
+// OMIT-ALL: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -S -Os -fomit-frame-pointer -momit-leaf-frame-pointer %s 
2>&1 | \
+// RUN:   FileCheck --check-prefix=OMIT-ALL2 %s
+// OMIT-ALL2-NOT: "-mdisable-fp-elim"
+// OMIT-ALL2: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -S -Os -fno-omit-frame-pointer 
-mno-omit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO-OMIT-ALL2 %s
+// NO-OMIT-ALL2: "-mdisable-fp-elim"
+// NO-OMIT-ALL2-NOT: "-momit-leaf-frame-pointer"
+
 void f0() {}
 void f1() { f0(); }
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -171,11 +171,10 @@
 
 // RUN: %clang_cl --target=i686-pc-win32 /O2sy- -### -- %s 2>&1 | FileCheck 
-check-prefix=PR24003 %s
 // PR24003: -mdisable-fp-elim
-// PR24003: -momit-leaf-frame-pointer
 // PR24003: -Os
 
 // RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- /O2 -### -- %s 2>&1 | 
FileCheck -check-prefix=Oy_2 %s
-// Oy_2: -momit-leaf-frame-pointer
+// Oy_2: -mdisable-fp-elim
 // Oy_2: -O2
 
 // RUN: %clang_cl --target=i686-pc-win32 -Werror /O2 /O2 -### -- %s 2>&1 | 
FileCheck -check-prefix=O2O2 %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -498,7 +498,7 @@
   return codegenoptions::LimitedDebugInfo;
 }
 
-static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
+static bool mustUseFramePointerForTarget(const llvm::Triple &Triple) {
   switch (Triple.getArch()){
   default:
 return false;
@@ -575,7 +575,7 @@
   if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
options::OPT_fomit_frame_pointer))
 return A->getOption().matches(options::OPT_fno_omit_frame_pointer) ||
-   mustUseNonLeafFramePointerForTarget(Triple);
+   mustUseFramePointerForTarget(Triple);
 
   if (Args.hasArg(options::OPT_pg))
 return true;
@@ -589,6 +589,11 @@
options::OPT_momit_leaf_frame_pointer))
 return A->getOption().matches(options::OPT_mno_omit_leaf_frame_pointer);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
+   options::OPT_fomit_frame_pointer))
+return A->getOption().matches(options::OPT_fno_omit_frame_pointer) ||
+   mustUseFramePointerForTarget(Triple);
+
   if (Args.hasArg(options::OPT_pg))
 return true;
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1736,7 +1736,6 @@
   FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
 } else {
   FuncAttrs.addAttribute("no-frame-pointer-elim", "true");
-  FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
 }
 
 FuncAttrs.addAttribute("less-precise-fpmad",


Index: test/Driver/frame-pointer-elim.c
===
--- test/Driver/frame-pointer-elim.c
+++ test/Driver/frame-pointer-elim.c
@@ -69,5 +69,30 @@
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=OMIT_LEAF %s
 
+// RUN: %clang -### -S -Os -fno-omit-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO-OMIT-ALL %s
+// NO-OMIT-ALL: "-mdisable-fp-elim"
+// NO-OMIT-ALL-NOT: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -S -Os -fno-omit-frame-pointer -momit-leaf-frame-pointer %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO-OMIT-NONLEA

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaObjC/externally-retained.m:14
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;

erik.pilkington wrote:
> aaron.ballman wrote:
> > Should this be useful for function pointer parameters as well? e.g.,
> > ```
> > typedef void (*fp)(EXT_RET __strong ObjCTy *);
> > 
> > void f(__strong ObjCTy *);
> > 
> > void g(EXT_RET ObjCTy *Ptr) {
> >   fp Fn = f; // Good idea? Bad idea?
> >   Fn(Ptr); // Which behavior "wins" in this call?
> > }
> > ```
> The attribute doesn't have any effect on the caller side, so when used with a 
> function pointer type the attribute doesn't really do anything (the function 
> definition always "wins").
Perhaps we should warn (and drop the attribute) if you try to write it on a 
parameter in a function pointer type rather than a function declaration? That 
way users won't think code like the above does anything useful. Alternatively, 
we could warn if there was a mismatch between markings (so the attribute is 
still a noop with function pointer types, but the consistent markings make that 
benign).


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

https://reviews.llvm.org/D55865



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:51
+  diag(ASDecl->getLocation(), "duplicated access specifier")
+  << MatchedDecl
+  << FixItHint::CreateRemoval(ASDecl->getSourceRange());

m4tx wrote:
> aaron.ballman wrote:
> > There is no %0 in the diagnostic string, so I'm not certain what this 
> > argument is trying to print out. Did you mean `ASDecl->getSourceRange()` 
> > for the underlining?
> Sorry, this is another line I forgot to remove. Thanks for pointing this out!
> 
> By the way, does the underlining work in clang-tidy's `diag()` function? I 
> see it is used outside the project, but here only `FixItHint`s seem to 
> generate underline in the generated messages.
That's a good question that I don't actually know the answer to. :-D I believe 
it still works -- you can try it out by passing a `SourceRange` to the `diag()` 
call, like:
```
diag(LastASDecl->getLocation(), "duplicated access specifier") << 
SomeSourceRange;
```
You should see the range from `SomeSourceRange` underlined.


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

https://reviews.llvm.org/D55793



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

[PATCH] D55823: [analyzer] Fix backward compatibility issue after D53280 'Emit an error for invalid -analyzer-config inputs'

[Artem Dergachev via Phabricator via cfe-commits]

NoQ closed this revision.
NoQ added a comment.

Should be sorted our in rC349866 .

Ugh, what a mess :) Thanks everyone :)


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

https://reviews.llvm.org/D55823



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

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/abseil/DurationRewriter.cpp:110
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return (N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,

Spurious parens can be removed.



Comment at: test/clang-tidy/abseil-duration-factory-scale.cpp:3
 
+#include 
 #include "absl/time/time.h"

Eugene.Zelenko wrote:
> cstdint, please.
Rather than write this as an include, I'd rather just do: `namespace std { 
typedef long long int64_t; } using int64_t = std::int64_t;` Then there's no 
question about whether this is using the host stdint.h or the 
current-clang-stdint.h.



Comment at: test/clang-tidy/abseil-duration-factory-scale.cpp:34
   // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]

Do you also have users doing something like: `absl::Seconds(int{});`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56012



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

r349948 - [AST][NFC] Remove stale comment in CXXRecordDecl::is(Virtually)DerivedFrom.

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 12:23:07 2018
New Revision: 349948

URL: http://llvm.org/viewvc/llvm-project?rev=349948&view=rev
Log:
[AST][NFC] Remove stale comment in CXXRecordDecl::is(Virtually)DerivedFrom.

The "this" capture was removed in r291939.


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

Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=349948&r1=349947&r2=349948&view=diff
==
--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
+++ cfe/trunk/lib/AST/CXXInheritance.cpp Fri Dec 21 12:23:07 2018
@@ -103,7 +103,6 @@ bool CXXRecordDecl::isDerivedFrom(const
   Paths.setOrigin(const_cast(this));
 
   const CXXRecordDecl *BaseDecl = Base->getCanonicalDecl();
-  // FIXME: Capturing 'this' is a workaround for name lookup bugs in GCC 4.7.
   return lookupInBases(
   [BaseDecl](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) {
 return FindBaseClass(Specifier, Path, BaseDecl);
@@ -124,7 +123,6 @@ bool CXXRecordDecl::isVirtuallyDerivedFr
   Paths.setOrigin(const_cast(this));
 
   const CXXRecordDecl *BaseDecl = Base->getCanonicalDecl();
-  // FIXME: Capturing 'this' is a workaround for name lookup bugs in GCC 4.7.
   return lookupInBases(
   [BaseDecl](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) {
 return FindVirtualBaseClass(Specifier, Path, BaseDecl);


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

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

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

It sounds like it's fine.


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

https://reviews.llvm.org/D55869



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

r349949 - [Sema][NFC] Fix a Wimplicit-fallthrough warning in CheckSpecializationInstantiationRedecl

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Dec 21 12:38:06 2018
New Revision: 349949

URL: http://llvm.org/viewvc/llvm-project?rev=349949&view=rev
Log:
[Sema][NFC] Fix a Wimplicit-fallthrough warning in 
CheckSpecializationInstantiationRedecl

All cases are covered so add an llvm_unreachable. NFC.


Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=349949&r1=349948&r2=349949&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Dec 21 12:38:06 2018
@@ -7966,6 +7966,7 @@ Sema::CheckSpecializationInstantiationRe
   HasNoEffect = true;
   return false;
 }
+llvm_unreachable("Unexpected TemplateSpecializationKind!");
 
   case TSK_ExplicitInstantiationDefinition:
 switch (PrevTSK) {


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

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested review of this revision.
aaron.ballman added a comment.

There's enough churn based on the review feedback that this should probably 
have a second round of review just to be sure.




Comment at: lib/AST/Expr.cpp:2281-2286
+  // If there is no FunctionDecl for the call, check the return type of the
+  // callee to see if it was declared with the WarnUnusedResult attribute.
+  if (!Func && !HasWarnUnusedResultAttr) {
+if (const TagDecl *TD = CE->getCallReturnType(Ctx)->getAsTagDecl())
+  HasWarnUnusedResultAttr = TD->hasAttr();
+  }

erik.pilkington wrote:
> This duplicates some logic from FunctionDecl::hasUnusedResultAttr(), maybe we 
> should move that member function to the CallExpr?
Good suggestion -- I think that cleans up the interface nicely.



Comment at: test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:51
+
+  one(); // expected-warning {{ignoring return value of function declared with 
'nodiscard' attribute}}
+  two(); // expected-warning {{ignoring return value of function declared with 
'nodiscard' attribute}}

erik.pilkington wrote:
> This diagnostic should also probably be improved at some point, the function 
> wasn't declared 'nodiscard', the type was.
Yeah, I noticed that as well.


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

https://reviews.llvm.org/D55949



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

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 179335.
hwright marked 6 inline comments as done.
hwright added a comment.

Add documentation, adjust test.


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

https://reviews.llvm.org/D56012

Files:
  clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  test/clang-tidy/abseil-duration-factory-scale.cpp

Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -2,6 +2,9 @@
 
 #include "absl/time/time.h"
 
+namespace std { typedef long long int64_t; }
+using int64_t = std::int64_t;
+
 void ScaleTest() {
   absl::Duration d;
 
@@ -30,6 +33,15 @@
   d = absl::Seconds(0x0.01p-126f);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
   // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int64_t{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(float{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
 
   // Fold seconds into minutes
   d = absl::Seconds(30 * 60);
@@ -83,6 +95,8 @@
 
   // None of these should trigger the check
   d = absl::Seconds(60);
+  d = absl::Seconds(int{60});
+  d = absl::Seconds(float{60});
   d = absl::Seconds(60 + 30);
   d = absl::Seconds(60 - 30);
   d = absl::Seconds(50 * 30);
Index: clang-tidy/abseil/DurationRewriter.cpp
===
--- clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tidy/abseil/DurationRewriter.cpp
@@ -105,14 +105,44 @@
   llvm_unreachable("unknown scaling factor");
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);
+}
+
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.
 bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
-  return selectFirst(
- "val",
- match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)),
-   floatLiteral(equals(0.0)
-   .bind("val"),
-   Node, *Result.Context)) != nullptr;
+  auto ZeroMatcher =
+  anyOf(integerLiteral(equals(0)), floatLiteral(equals(0.0)));
+
+  // Check to see if we're using a zero directly.
+  if (selectFirst(
+  "val", match(expr(ignoringImpCasts(ZeroMatcher)).bind("val"), Node,
+   *Result.Context)) != nullptr)
+return true;
+
+  // Now check to see if we're using a functional cast with a scalar
+  // initializer expression, e.g. `int{0}`.
+  if (selectFirst(
+  "val",
+  match(cxxFunctionalCastExpr(
+hasDestinationType(
+anyOf(isInteger(), realFloatingPointType())),
+hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher
+.bind("val"),
+Node, *Result.Context)) != nullptr)
+return true;
+
+  return false;
 }
 
 llvm::Optional
Index: clang-tidy/abseil/DurationFactoryScaleCheck.cpp
===
--- clang-tidy/abseil/DurationFactoryScaleCheck.cpp
+++ clang-tidy/abseil/DurationFactoryScaleCheck.cpp
@@ -123,6 +123,10 @@
   hasArgument(
   0,
   ignoringImpCasts(anyOf(
+  cxxFunctionalCastExpr(
+  hasDestinationType(
+  anyOf(isInteger(), realFloatingPointType())),
+  hasSourceExpression(initListExpr())),
   integerLiteral(equals(0)), floatLiteral(equals(0.0)),
   binaryOperator(hasOperatorName("*"),
  hasEitherOperand(ignoringImpCasts(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via Phabricator via cfe-commits]

hwright added a comment.

btw, I think `hasInit` should probably be moved into the core set of matchers 
at some point.




Comment at: clang-tidy/abseil/DurationRewriter.cpp:110
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return (N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,

aaron.ballman wrote:
> Spurious parens can be removed.
Done.  (Though there are similar parens in the implementation of `hasArgument`. 
:)



Comment at: test/clang-tidy/abseil-duration-factory-scale.cpp:34
   // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]

aaron.ballman wrote:
> Do you also have users doing something like: `absl::Seconds(int{});`?
I have not yet seen that.


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

https://reviews.llvm.org/D56012



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

r349952 - Convert some ObjC retain/release msgSends to runtime calls.

[Pete Cooper via cfe-commits]

Author: pete
Date: Fri Dec 21 13:00:32 2018
New Revision: 349952

URL: http://llvm.org/viewvc/llvm-project?rev=349952&view=rev
Log:
Convert some ObjC retain/release msgSends to runtime calls.

It is faster to directly call the ObjC runtime for methods such as 
retain/release instead of sending a message to those functions.

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

Reviewed By: rjmccall

Modified:
cfe/trunk/include/clang/Basic/ObjCRuntime.h
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m

Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ObjCRuntime.h?rev=349952&r1=349951&r2=349952&view=diff
==
--- cfe/trunk/include/clang/Basic/ObjCRuntime.h (original)
+++ cfe/trunk/include/clang/Basic/ObjCRuntime.h Fri Dec 21 13:00:32 2018
@@ -173,6 +173,43 @@ public:
 llvm_unreachable("bad kind");
   }
 
+  /// Does this runtime provide ARC entrypoints that are likely to be faster
+  /// than an ordinary message send of the appropriate selector?
+  ///
+  /// The ARC entrypoints are guaranteed to be equivalent to just sending the
+  /// corresponding message.  If the entrypoint is implemented naively as just 
a
+  /// message send, using it is a trade-off: it sacrifices a few cycles of
+  /// overhead to save a small amount of code.  However, it's possible for
+  /// runtimes to detect and special-case classes that use "standard"
+  /// retain/release behavior; if that's dynamically a large proportion of all
+  /// retained objects, using the entrypoint will also be faster than using a
+  /// message send.
+  ///
+  /// When this method returns true, Clang will turn non-super message sends of
+  /// certain selectors into calls to the correspond entrypoint:
+  ///   retain => objc_retain
+  ///   release => objc_release
+  ///   autorelease => objc_autorelease
+  bool shouldUseARCFunctionsForRetainRelease() const {
+switch (getKind()) {
+case FragileMacOSX:
+  return false;
+case MacOSX:
+  return getVersion() >= VersionTuple(10, 10);
+case iOS:
+  return getVersion() >= VersionTuple(8);
+case WatchOS:
+  return true;
+case GCC:
+  return false;
+case GNUstep:
+  return false;
+case ObjFW:
+  return false;
+}
+llvm_unreachable("bad kind");
+  }
+
   /// Does this runtime provide entrypoints that are likely to be faster
   /// than an ordinary message send of the "alloc" selector?
   ///

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=349952&r1=349951&r2=349952&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Fri Dec 21 13:00:32 2018
@@ -396,6 +396,29 @@ tryGenerateSpecializedMessageSend(CodeGe
 }
 break;
 
+  case OMF_autorelease:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCAutorelease(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_retain:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCRetainNonBlock(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_release:
+if (ResultType->isVoidType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease()) {
+  CGF.EmitObjCRelease(Receiver, ARCPreciseLifetime);
+  return nullptr;
+}
+break;
+
   default:
 break;
   }
@@ -2006,6 +2029,11 @@ static llvm::Value *emitObjCValueOperati
 llvm::FunctionType *fnType =
   llvm::FunctionType::get(CGF.Int8PtrTy, CGF.Int8PtrTy, false);
 fn = CGF.CGM.CreateRuntimeFunction(fnType, fnName);
+
+// We have Native ARC, so set nonlazybind attribute for performance
+if (llvm::Function *f = dyn_cast(fn))
+  if (fnName == "objc_retain")
+f->addFnAttr(llvm::Attribute::NonLazyBind);
   }
 
   // Cast the argument to 'id'.
@@ -2510,6 +2538,55 @@ void CodeGenFunction::emitARCIntrinsicUs
   CGF.EmitARCIntrinsicUse(value);
 }
 
+/// Autorelease the given object.
+///   call i8* \@objc_autorelease(i8* %value)
+llvm::Value *CodeGenFunction::EmitObjCAutorelease(llvm::Value *value,
+  llvm::Type *returnType) {
+  return emitObjCValueOperation(*this, value, returnType,
+  CGM.getObjCEntrypoints().objc_autoreleaseRuntimeFunction,
+"objc_autorelea

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349952: Convert some ObjC retain/release msgSends to runtime 
calls. (authored by pete, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D55869

Files:
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  test/CodeGenObjC/convert-messages-to-runtime-calls.m

Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -396,6 +396,29 @@
 }
 break;
 
+  case OMF_autorelease:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCAutorelease(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_retain:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCRetainNonBlock(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_release:
+if (ResultType->isVoidType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease()) {
+  CGF.EmitObjCRelease(Receiver, ARCPreciseLifetime);
+  return nullptr;
+}
+break;
+
   default:
 break;
   }
@@ -2006,6 +2029,11 @@
 llvm::FunctionType *fnType =
   llvm::FunctionType::get(CGF.Int8PtrTy, CGF.Int8PtrTy, false);
 fn = CGF.CGM.CreateRuntimeFunction(fnType, fnName);
+
+// We have Native ARC, so set nonlazybind attribute for performance
+if (llvm::Function *f = dyn_cast(fn))
+  if (fnName == "objc_retain")
+f->addFnAttr(llvm::Attribute::NonLazyBind);
   }
 
   // Cast the argument to 'id'.
@@ -2510,6 +2538,55 @@
   CGF.EmitARCIntrinsicUse(value);
 }
 
+/// Autorelease the given object.
+///   call i8* \@objc_autorelease(i8* %value)
+llvm::Value *CodeGenFunction::EmitObjCAutorelease(llvm::Value *value,
+  llvm::Type *returnType) {
+  return emitObjCValueOperation(*this, value, returnType,
+  CGM.getObjCEntrypoints().objc_autoreleaseRuntimeFunction,
+"objc_autorelease");
+}
+
+/// Retain the given object, with normal retain semantics.
+///   call i8* \@objc_retain(i8* %value)
+llvm::Value *CodeGenFunction::EmitObjCRetainNonBlock(llvm::Value *value,
+ llvm::Type *returnType) {
+  return emitObjCValueOperation(*this, value, returnType,
+  CGM.getObjCEntrypoints().objc_retainRuntimeFunction,
+"objc_retain");
+}
+
+/// Release the given object.
+///   call void \@objc_release(i8* %value)
+void CodeGenFunction::EmitObjCRelease(llvm::Value *value,
+  ARCPreciseLifetime_t precise) {
+  if (isa(value)) return;
+
+  llvm::Constant *&fn = CGM.getObjCEntrypoints().objc_release;
+  if (!fn) {
+if (!fn) {
+  llvm::FunctionType *fnType =
+llvm::FunctionType::get(Builder.getVoidTy(), Int8PtrTy, false);
+  fn = CGM.CreateRuntimeFunction(fnType, "objc_release");
+  setARCRuntimeFunctionLinkage(CGM, fn);
+  // We have Native ARC, so set nonlazybind attribute for performance
+  if (llvm::Function *f = dyn_cast(fn))
+f->addFnAttr(llvm::Attribute::NonLazyBind);
+}
+  }
+
+  // Cast the argument to 'id'.
+  value = Builder.CreateBitCast(value, Int8PtrTy);
+
+  // Call objc_release.
+  llvm::CallInst *call = EmitNounwindRuntimeCall(fn, value);
+
+  if (precise == ARCImpreciseLifetime) {
+call->setMetadata("clang.imprecise_release",
+  llvm::MDNode::get(Builder.getContext(), None));
+  }
+}
+
 namespace {
   struct CallObjCAutoreleasePoolObject final : EHScopeStack::Cleanup {
 llvm::Value *Token;
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -138,6 +138,10 @@
   /// id objc_autorelease(id);
   llvm::Constant *objc_autorelease;
 
+  /// id objc_autorelease(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_autoreleaseRuntimeFunction;
+
   /// id objc_autoreleaseReturnValue(id);
   llvm::Constant *objc_autoreleaseReturnValue;
 
@@ -162,6 +166,10 @@
   /// id objc_retain(id);
   llvm::Constant *objc_retain;
 
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_retainRuntimeFunction;
+
   /// id objc_retainAutorelease(id);
   llvm::Constant *objc_retainAutorelease;
 
@@ -177,6 +185,10 @@
   /// void objc_release(id);
   llvm::Con

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete added a comment.

In D55869#1339537 , @rjmccall wrote:

> It sounds like it's fine.


Thanks for the review!  Just pushed it as r349952.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55869



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

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[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.

LGTM!


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

https://reviews.llvm.org/D56012



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

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349953: [clang-tidy] Be more liberal about literal zeroes in 
abseil checks (authored by hwright, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56012?vs=179335&id=179341#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56012

Files:
  clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
  clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp

Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
===
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
@@ -105,14 +105,44 @@
   llvm_unreachable("unknown scaling factor");
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);
+}
+
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.
 bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
-  return selectFirst(
- "val",
- match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)),
-   floatLiteral(equals(0.0)
-   .bind("val"),
-   Node, *Result.Context)) != nullptr;
+  auto ZeroMatcher =
+  anyOf(integerLiteral(equals(0)), floatLiteral(equals(0.0)));
+
+  // Check to see if we're using a zero directly.
+  if (selectFirst(
+  "val", match(expr(ignoringImpCasts(ZeroMatcher)).bind("val"), Node,
+   *Result.Context)) != nullptr)
+return true;
+
+  // Now check to see if we're using a functional cast with a scalar
+  // initializer expression, e.g. `int{0}`.
+  if (selectFirst(
+  "val",
+  match(cxxFunctionalCastExpr(
+hasDestinationType(
+anyOf(isInteger(), realFloatingPointType())),
+hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher
+.bind("val"),
+Node, *Result.Context)) != nullptr)
+return true;
+
+  return false;
 }
 
 llvm::Optional
Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
@@ -123,6 +123,10 @@
   hasArgument(
   0,
   ignoringImpCasts(anyOf(
+  cxxFunctionalCastExpr(
+  hasDestinationType(
+  anyOf(isInteger(), realFloatingPointType())),
+  hasSourceExpression(initListExpr())),
   integerLiteral(equals(0)), floatLiteral(equals(0.0)),
   binaryOperator(hasOperatorName("*"),
  hasEitherOperand(ignoringImpCasts(
Index: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -2,6 +2,9 @@
 
 #include "absl/time/time.h"
 
+namespace std { typedef long long int64_t; }
+using int64_t = std::int64_t;
+
 void ScaleTest() {
   absl::Duration d;
 
@@ -30,6 +33,15 @@
   d = absl::Seconds(0x0.01p-126f);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
   // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int64_t{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(float{0});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
 
   // Fold seconds into minutes
   d = absl::Seconds(30 * 60);
@@ -83,6 +95,8 @@
 

[clang-tools-extra] r349953 - [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Fri Dec 21 13:07:11 2018
New Revision: 349953

URL: http://llvm.org/viewvc/llvm-project?rev=349953&view=rev
Log:
[clang-tidy] Be more liberal about literal zeroes in abseil checks

Summary:
Previously, we'd only match on literal floating or integral zeroes, but I've 
now also learned that some users spell that value as int{0} or float{0}, which 
also need to be matched.

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

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp?rev=349953&r1=349952&r2=349953&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp Fri 
Dec 21 13:07:11 2018
@@ -123,6 +123,10 @@ void DurationFactoryScaleCheck::register
   hasArgument(
   0,
   ignoringImpCasts(anyOf(
+  cxxFunctionalCastExpr(
+  hasDestinationType(
+  anyOf(isInteger(), realFloatingPointType())),
+  hasSourceExpression(initListExpr())),
   integerLiteral(equals(0)), floatLiteral(equals(0.0)),
   binaryOperator(hasOperatorName("*"),
  hasEitherOperand(ignoringImpCasts(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp?rev=349953&r1=349952&r2=349953&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp Fri Dec 21 
13:07:11 2018
@@ -105,14 +105,44 @@ llvm::StringRef getFactoryForScale(Durat
   llvm_unreachable("unknown scaling factor");
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);
+}
+
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.
 bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
-  return selectFirst(
- "val",
- match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)),
-   floatLiteral(equals(0.0)
-   .bind("val"),
-   Node, *Result.Context)) != nullptr;
+  auto ZeroMatcher =
+  anyOf(integerLiteral(equals(0)), floatLiteral(equals(0.0)));
+
+  // Check to see if we're using a zero directly.
+  if (selectFirst(
+  "val", match(expr(ignoringImpCasts(ZeroMatcher)).bind("val"), Node,
+   *Result.Context)) != nullptr)
+return true;
+
+  // Now check to see if we're using a functional cast with a scalar
+  // initializer expression, e.g. `int{0}`.
+  if (selectFirst(
+  "val",
+  match(cxxFunctionalCastExpr(
+hasDestinationType(
+anyOf(isInteger(), realFloatingPointType())),
+hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher
+.bind("val"),
+Node, *Result.Context)) != nullptr)
+return true;
+
+  return false;
 }
 
 llvm::Optional

Modified: 
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp?rev=349953&r1=349952&r2=349953&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp 
Fri Dec 21 13:07:11 2018
@@ -2,6 +2,9 @@
 
 #include "absl/time/time.h"
 
+namespace std { typedef long long int64_t; }
+using int64_t = std::int64_t;
+
 void ScaleTest() {
   absl::Duration d;
 
@@ -30,6 +33,15 @@ void ScaleTest() {
   d = absl::Seconds(0x0.01p-126f);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
   // C

r349955 - Switch from static_cast<> to cast<>, update identifier for coding conventions; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Dec 21 13:11:36 2018
New Revision: 349955

URL: http://llvm.org/viewvc/llvm-project?rev=349955&view=rev
Log:
Switch from static_cast<> to cast<>, update identifier for coding conventions; 
NFC.

Modified:
cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=349955&r1=349954&r2=349955&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Fri Dec 21 13:11:36 2018
@@ -469,11 +469,9 @@ Sema::ActOnCaseStmt(SourceLocation CaseL
 }
 
 /// ActOnCaseStmtBody - This installs a statement as the body of a case.
-void Sema::ActOnCaseStmtBody(Stmt *caseStmt, Stmt *SubStmt) {
+void Sema::ActOnCaseStmtBody(Stmt *S, Stmt *SubStmt) {
   DiagnoseUnusedExprResult(SubStmt);
-
-  auto *CS = static_cast(caseStmt);
-  CS->setSubStmt(SubStmt);
+  cast(S)->setSubStmt(SubStmt);
 }
 
 StmtResult


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

[PATCH] D56022: [AST] Store the arguments of CXXConstructExpr in a trailing array

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added a reviewer: rjmccall.
riccibruno added a project: clang.
Herald added a reviewer: shafik.
Herald added a subscriber: cfe-commits.

Store the arguments of `CXXConstructExpr` in a trailing array. This is very 
similar to the
`CallExpr` case in D55771 , with the exception 
that there is only one derived class
(`CXXTemporaryObjectExpr`) and that we compute the offset to the trailing array 
instead
of storing it.

This saves one pointer per `CXXConstructExpr` and `CXXTemporaryObjectExpr`.

(with some comments inline)


Repository:
  rC Clang

https://reviews.llvm.org/D56022

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h
  lib/AST/ASTImporter.cpp
  lib/AST/ExprCXX.cpp
  lib/Sema/SemaInit.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp

Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -1315,18 +1315,21 @@
 
 void ASTStmtWriter::VisitCXXConstructExpr(CXXConstructExpr *E) {
   VisitExpr(E);
+
   Record.push_back(E->getNumArgs());
-  for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I)
-Record.AddStmt(E->getArg(I));
-  Record.AddDeclRef(E->getConstructor());
-  Record.AddSourceLocation(E->getLocation());
   Record.push_back(E->isElidable());
   Record.push_back(E->hadMultipleCandidates());
   Record.push_back(E->isListInitialization());
   Record.push_back(E->isStdInitListInitialization());
   Record.push_back(E->requiresZeroInitialization());
   Record.push_back(E->getConstructionKind()); // FIXME: stable encoding
+  Record.AddSourceLocation(E->getLocation());
+  Record.AddDeclRef(E->getConstructor());
   Record.AddSourceRange(E->getParenOrBraceRange());
+
+  for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I)
+Record.AddStmt(E->getArg(I));
+
   Code = serialization::EXPR_CXX_CONSTRUCT;
 }
 
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -1353,20 +1353,22 @@
 
 void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) {
   VisitExpr(E);
-  E->NumArgs = Record.readInt();
-  if (E->NumArgs)
-E->Args = new (Record.getContext()) Stmt*[E->NumArgs];
-  for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I)
-E->setArg(I, Record.readSubExpr());
-  E->setConstructor(ReadDeclAs());
-  E->setLocation(ReadSourceLocation());
-  E->setElidable(Record.readInt());
-  E->setHadMultipleCandidates(Record.readInt());
-  E->setListInitialization(Record.readInt());
-  E->setStdInitListInitialization(Record.readInt());
-  E->setRequiresZeroInitialization(Record.readInt());
-  E->setConstructionKind((CXXConstructExpr::ConstructionKind)Record.readInt());
+
+  unsigned NumArgs = Record.readInt();
+  assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
+
+  E->CXXConstructExprBits.Elidable = Record.readInt();
+  E->CXXConstructExprBits.HadMultipleCandidates = Record.readInt();
+  E->CXXConstructExprBits.ListInitialization = Record.readInt();
+  E->CXXConstructExprBits.StdInitListInitialization = Record.readInt();
+  E->CXXConstructExprBits.ZeroInitialization = Record.readInt();
+  E->CXXConstructExprBits.ConstructionKind = Record.readInt();
+  E->CXXConstructExprBits.Loc = ReadSourceLocation();
+  E->Constructor = ReadDeclAs();
   E->ParenOrBraceRange = ReadSourceRange();
+
+  for (unsigned I = 0; I != NumArgs; ++I)
+E->setArg(I, Record.readSubExpr());
 }
 
 void ASTStmtReader::VisitCXXInheritedCtorInitExpr(CXXInheritedCtorInitExpr *E) {
@@ -1379,7 +1381,7 @@
 
 void ASTStmtReader::VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *E) {
   VisitCXXConstructExpr(E);
-  E->Type = GetTypeSourceInfo();
+  E->TSI = GetTypeSourceInfo();
 }
 
 void ASTStmtReader::VisitLambdaExpr(LambdaExpr *E) {
@@ -3082,7 +3084,9 @@
   break;
 
 case EXPR_CXX_CONSTRUCT:
-  S = new (Context) CXXConstructExpr(Empty);
+  S = CXXConstructExpr::CreateEmpty(
+  Context,
+  /* NumArgs=*/Record[ASTStmtReader::NumExprFields]);
   break;
 
 case EXPR_CXX_INHERITED_CTOR_INIT:
@@ -3090,7 +3094,9 @@
   break;
 
 case EXPR_CXX_TEMPORARY_OBJECT:
-  S = new (Context) CXXTemporaryObjectExpr(Empty);
+  S = CXXTemporaryObjectExpr::CreateEmpty(
+  Context,
+  /* NumArgs=*/Record[ASTStmtReader::NumExprFields]);
   break;
 
 case EXPR_CXX_STATIC_CAST:
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6199,7 +6199,7 @@
 }
 S.MarkFunctionReferenced(Loc, Constructor);
 
-CurInit = new (S.Context) CXXTemporaryObjectExpr(
+CurInit = CXXTemporaryObjectExpr::Create(
 S.Context, Constructor,
 Entity.getType().getNonLValueExprType

[PATCH] D56022: [AST] Store the arguments of CXXConstructExpr in a trailing array

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/ExprCXX.h:1295
+  /// A pointer to the constructor which will be ultimately called.
+  CXXConstructorDecl *Constructor;
+

Removed the default member initializer as it is initialized unconditionally
in the constructor.



Comment at: include/clang/AST/ExprCXX.h:1613
+
+  TypeSourceInfo *TSI = nullptr;
+

Changed the name from `Type` since `Type` was imho an unfortunate choice.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56022



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added a comment.

will correct all mistakes. please check explanations for the questions.




Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

aaron.ballman wrote:
> is called the initiation interval.
> 
> I can't quite parse the second sentence, though. Is it trying to say that the 
> initiation interval is the number of cycles for a single iteration of the 
> optimized loop?
Not quite right : initiation interval is not a number of cycles for a single 
iteration of the optimized loop. But initiation interval matches with number of 
cycles for a single iteration of the optimized loop. Initiation interval is the 
number of cycles between next and previous iteration of original loop. New loop 
created so that single iteration of the optimized loop has the same number 
cycles as initiation interval( thus every new iteration of original loop 
started each time when new iteration of optimized loop started - difference 
between iterations is initiation interval). 

trying to rephrase : number of cycles for a single iteration of the optimized 
loop matches with number of cycles of initiation interval.



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

aaron.ballman wrote:
> I'm can't quite parse this sentence either. Is it trying to say that the 
> scheduler will try to find a loop iteration cycle count that most-closely 
> matches the specified initiation interval?
I wanted to say that pipeliner will try to find a schedule that exactly matches 
the specified initiation interval. And if schedule would be found then single 
iteration of the optimized loop would have exactly the same amount of cycles as 
initiation interval. trying to rephrase :

If schedule would be found then single iteration of the optimized loop would 
have exactly the same amount of cycles as initiation interval has. 


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

https://reviews.llvm.org/D55710



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

[PATCH] D54565: Introduce `-Wctad` as a subgroup of `-Wc++14-compat`

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In D54565#1339454 , @thakis wrote:

> Since I don't know what it means, I don't know if I want a diagnostic for it 
> :-)


Fair point. ;) But the full text of the diagnostic message does say "class 
template argument deduction is...," which does hint at the meaning of "CTAD."
I just checked and the text of the diagnostic for `-Wvla` is "variable length 
array used"; maybe I should follow its lead and just say "class template 
argument deduction used," thus even further divorcing this option from 
`-Wc++14-compat`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54565



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

[PATCH] D56022: [AST] Store the arguments of CXXConstructExpr in a trailing array

[John McCall via Phabricator via cfe-commits]

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

LGTM.




Comment at: include/clang/AST/ExprCXX.h:1613
+
+  TypeSourceInfo *TSI = nullptr;
+

riccibruno wrote:
> Changed the name from `Type` since `Type` was imho an unfortunate choice.
Agreed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56022



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

[PATCH] D55933: [Sema] Do not print default template parameters.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: test/SemaCXX/static-assert-cxx17.cpp:109
+template 
+void foo7() {
+  static_assert(X());

courbet wrote:
> Quuxplusone wrote:
> > None of these new test cases actually involve the default template argument.
> > 
> > I'd like to see two test cases explicitly mimicking `vector`. (Warning: I 
> > didn't run this code!)
> > ```
> > template struct A {};
> > template> struct V {};
> > template void testx() {
> > static_assert(std::is_same::value, "");
> > // expected-error@-1{{due to requirement 'std::is_same*, 
> > void>::value'}}
> > }
> > template testx>();
> > template struct PMRA {};
> > template using PMRV = V>;
> > template void test() {
> > static_assert(std::is_same::value, "");
> > // expected-error@-1{{due to requirement 'std::is_same > PMRA>*, void>::value'}}
> > // expected-error@-2{{due to requirement 'std::is_same*, 
> > void>::value'}}
> > }
> > template testy>();
> > ```
> > The `expected-error@-2` above is my fantasy world. I don't think it's 
> > possible to achieve in real life. :)
> > None of these new test cases actually involve the default template argument.
> 
> This one is to check that we actually do print when specified explicitly. 
> foo6 above tests  the default template arguments (notice the change from 
> `template  struct X to `template  struct 
> X` above). I've renamed `foo6` and `foo7` to make that clear.
> 
> Before this change, static_asserts in `foo6` and `foo7` printed the same 
> thing. Now they don't.
> 
> > I'd like to see two test cases explicitly mimicking vector.
> 
> OK, I think I misunderstood what you wanted here. I don't think what you want 
> is actually doable, because by the time you're in `test()`, C is just a type 
> without any way of knowing whether the user explicitly provided the template 
> parameter or relied on the default.
> 
> What we could do though is **always** erase template parameters that are the 
> same as default ones. But that would mean printing `due to requirement 
> 'std::is_same*, void>::value'` even when the user wrote `template 
> testx>>();`.
> WDYT ?
> 
> 
Here's what I wrote over on D55270.

>>! In D55270#1327704, @Quuxplusone wrote:
> @courbet: On the cpplang Slack, Peter Feichtinger mentioned that defaulted 
> template arguments should perhaps be treated differently. Is there any way to 
> suppress the `, std::allocator` part of this diagnostic? 
> https://godbolt.org/z/TM0UHc
> 
> Before your patches:
> ```
> :11:5: error: static_assert failed due to requirement 
> 'std::is_integral_v'
> static_assert(std::is_integral_v);
> ^ ~
> ```
> After your patches:
> ```
> :11:5: error: static_assert failed due to requirement 
> 'std::is_integral_v > >'
> static_assert(std::is_integral_v);
> ^ ~ 
> ```
> I don't think the new behavior is //worse//; but I don't think it's optimal, 
> either.
> 
> I think Clang already has a feature to suppress printing these parameters; 
> you would just have to figure out where it is and try to hook it into your 
> thing. (And if you do, I'm going to ask for a test case where `T::t` is 
> `std::pmr::vector`!)

So does this patch (D55933) actually change the message printed by Peter's 
https://godbolt.org/z/TM0UHc ? (I think it does not.)
Does it change the message printed by https://godbolt.org/z/Q0AD70 ? (I think 
it does.)

I think most of your new test cases in `foo7` are redundant and don't really 
need to be there. Contrariwise, I would like to see one new test case 
isomorphic to https://godbolt.org/z/Q0AD70 , because that strikes me as a very 
realistic case that we want to protect against regressions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55933



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

[PATCH] D55878: [Driver] Use --hash-style=gnu instead of both on FreeBSD

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D55878#1339098 , @emaste wrote:

> I think the arch-change (switching from a whitelist to a MIPS blacklist) is 
> reasonable. What is the motivation for dropping `DT_HASH`, just binary size 
> reduction?


Yes. It saves a few hundreds bytes to a few kilobytes for each EXE/DSO.

See `Linux.cpp`, some Linux distributions default to `--hash-style=gnu` too:

  if (!IsMips && !IsHexagon) {
if (Distro.IsRedhat() || Distro.IsOpenSUSE() || Distro.IsAlpineLinux() ||
(Distro.IsUbuntu() && Distro >= Distro::UbuntuMaverick) ||
(IsAndroid && !Triple.isAndroidVersionLT(23)))
  ExtraOpts.push_back("--hash-style=gnu");

This section is only consumed by dynamic loader. It is unfortunate that OpenBSD 
only ported this a month ago and illumos does not support it at all. The GNU 
section is superior to the SYSV counterpart, even in term of section sizes 
(`.hash` does not skip local symbols).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55878



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

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 179352.
NoQ added a comment.

Add test case `bool_to_nullptr` in `casts.cpp` on which my second attempt 
 crashes but the current code 
 does not.


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

https://reviews.llvm.org/D55875

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c
  test/Analysis/casts.cpp


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 3; ++i)
+(void)(s.a[i] != nullptr);
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,12 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  if (castTy->isFloatingType()) {
+SymbolRef Sym = V.getAsSymbol();
+if (Sym && !Sym->getType()->isFloatingType())
+  return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 3; ++i)
+(void)(s.a[i] != nullptr);
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,12 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  if (castTy->isFloatingType()) {
+SymbolRef Sym = V.getAsSymbol();
+if (Sym && !Sym->getType()->isFloatingType())
+  return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56024: [clang] [Distro] Support detecting Gentoo

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: sylvestre.ledru, bruno, bkramer, phosek.

Add support for distinguishing plain Gentoo distribution, and a unit
test for it.  This is going to be used to introduce distro-specific
customizations in the driver code; most notably, it is going to be used
to disable -faddrsig.


Repository:
  rC Clang

https://reviews.llvm.org/D56024

Files:
  include/clang/Driver/Distro.h
  lib/Driver/Distro.cpp
  unittests/Driver/DistroTest.cpp


Index: unittests/Driver/DistroTest.cpp
===
--- unittests/Driver/DistroTest.cpp
+++ unittests/Driver/DistroTest.cpp
@@ -302,4 +302,28 @@
   ASSERT_FALSE(ArchLinux.IsDebian());
 }
 
+TEST(DistroTest, DetectGentoo) {
+  llvm::vfs::InMemoryFileSystem GentooFileSystem;
+  GentooFileSystem.addFile(
+  "/etc/gentoo-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("Gentoo Base System release 2.6"));
+  GentooFileSystem.addFile(
+  "/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer(
+  "NAME=Gentoo\n"
+  "ID=gentoo\n"
+  "PRETTY_NAME=\"Gentoo/Linux\"\n"
+  "ANSI_COLOR=\"1;32\"\n"
+  "HOME_URL=\"https://www.gentoo.org/\"\n";
+  "SUPPORT_URL=\"https://www.gentoo.org/support/\"\n";
+  "BUG_REPORT_URL=\"https://bugs.gentoo.org/\"\n";));
+
+  Distro Gentoo{GentooFileSystem};
+  ASSERT_EQ(Distro(Distro::Gentoo), Gentoo);
+  ASSERT_FALSE(Gentoo.IsUbuntu());
+  ASSERT_FALSE(Gentoo.IsRedhat());
+  ASSERT_FALSE(Gentoo.IsOpenSUSE());
+  ASSERT_FALSE(Gentoo.IsDebian());
+}
+
 } // end anonymous namespace
Index: lib/Driver/Distro.cpp
===
--- lib/Driver/Distro.cpp
+++ lib/Driver/Distro.cpp
@@ -138,6 +138,9 @@
   if (VFS.exists("/etc/arch-release"))
 return Distro::ArchLinux;
 
+  if (VFS.exists("/etc/gentoo-release"))
+return Distro::Gentoo;
+
   return Distro::UnknownDistro;
 }
 
Index: include/clang/Driver/Distro.h
===
--- include/clang/Driver/Distro.h
+++ include/clang/Driver/Distro.h
@@ -39,6 +39,7 @@
 RHEL6,
 RHEL7,
 Fedora,
+Gentoo,
 OpenSUSE,
 UbuntuHardy,
 UbuntuIntrepid,


Index: unittests/Driver/DistroTest.cpp
===
--- unittests/Driver/DistroTest.cpp
+++ unittests/Driver/DistroTest.cpp
@@ -302,4 +302,28 @@
   ASSERT_FALSE(ArchLinux.IsDebian());
 }
 
+TEST(DistroTest, DetectGentoo) {
+  llvm::vfs::InMemoryFileSystem GentooFileSystem;
+  GentooFileSystem.addFile(
+  "/etc/gentoo-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("Gentoo Base System release 2.6"));
+  GentooFileSystem.addFile(
+  "/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer(
+  "NAME=Gentoo\n"
+  "ID=gentoo\n"
+  "PRETTY_NAME=\"Gentoo/Linux\"\n"
+  "ANSI_COLOR=\"1;32\"\n"
+  "HOME_URL=\"https://www.gentoo.org/\"\n";
+  "SUPPORT_URL=\"https://www.gentoo.org/support/\"\n";
+  "BUG_REPORT_URL=\"https://bugs.gentoo.org/\"\n";));
+
+  Distro Gentoo{GentooFileSystem};
+  ASSERT_EQ(Distro(Distro::Gentoo), Gentoo);
+  ASSERT_FALSE(Gentoo.IsUbuntu());
+  ASSERT_FALSE(Gentoo.IsRedhat());
+  ASSERT_FALSE(Gentoo.IsOpenSUSE());
+  ASSERT_FALSE(Gentoo.IsDebian());
+}
+
 } // end anonymous namespace
Index: lib/Driver/Distro.cpp
===
--- lib/Driver/Distro.cpp
+++ lib/Driver/Distro.cpp
@@ -138,6 +138,9 @@
   if (VFS.exists("/etc/arch-release"))
 return Distro::ArchLinux;
 
+  if (VFS.exists("/etc/gentoo-release"))
+return Distro::Gentoo;
+
   return Distro::UnknownDistro;
 }
 
Index: include/clang/Driver/Distro.h
===
--- include/clang/Driver/Distro.h
+++ include/clang/Driver/Distro.h
@@ -39,6 +39,7 @@
 RHEL6,
 RHEL7,
 Fedora,
+Gentoo,
 OpenSUSE,
 UbuntuHardy,
 UbuntuIntrepid,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56024: [clang] [Distro] Support detecting Gentoo

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM




Comment at: include/clang/Driver/Distro.h:118
   }
 
   bool IsUbuntu() const {

Shall we also introduce the `IsGentoo()` predicate for convenience?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56024



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

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: JonasToth, alexfh, lebedev.ri.
Herald added subscribers: cfe-commits, xazax.hun.

And also enable it by default to be consistent with e.g. modernize-use-using.

This helps e.g. when running this check on client code where the macro is 
provided by the system, so there is no easy way to modify it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56025

Files:
  clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I 
%S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -49,3 +49,8 @@
 * ``uL`` will be kept as is.
 * ``ull`` will be kept as is, since it is not in the list
 * and so on.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about literal suffixes inside macros.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -236,6 +236,10 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.h
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -35,6 +35,7 @@
   bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
 
   const std::vector NewSuffixes;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,12 +183,14 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
-  utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+  utils::options::parseStringList(Options.get("NewSuffixes", ""))),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NewSuffixes",
 utils::options::serializeStringList(NewSuffixes));
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
@@ -216,6 +218,8 @@
   // We might have a suffix that is already uppercase.
   if (auto Details = shouldReplaceLiteralSuffix(
   *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros)
+  return true;
 auto Complaint = diag(Details->LiteralLocation.getBegin(),
   "%0 literal has suffix '%1', which is not uppercase")
  << LiteralType::Name << Details->OldSuffix;


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
==

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp:2-3
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 

And the positive test?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56025



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

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 179355.
NoQ added a comment.

`// no-crash` and a slightly cleaner test.


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

https://reviews.llvm.org/D55875

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c
  test/Analysis/casts.cpp


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 2; ++i)
+(void)(s.a[i] != nullptr); // no-crash
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,12 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  if (castTy->isFloatingType()) {
+SymbolRef Sym = V.getAsSymbol();
+if (Sym && !Sym->getType()->isFloatingType())
+  return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 2; ++i)
+(void)(s.a[i] != nullptr); // no-crash
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,12 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  if (castTy->isFloatingType()) {
+SymbolRef Sym = V.getAsSymbol();
+if (Sym && !Sym->getType()->isFloatingType())
+  return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55489: Implement dumpFunctionDeclParameters in NodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

steveire abandoned this revision.
steveire added a comment.

Abandoning this because it's not really the right approach as it appears in the 
interface between the generic traverser and the generic node dumper, which it 
really shouldn't.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55489



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

[PATCH] D55490: Add dumpMethodDeclOverrides to NodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

steveire abandoned this revision.
steveire added a comment.

Abandoning this because it's not really the right approach as it appears in the 
interface between the generic traverser and the generic node dumper, which it 
really shouldn't.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55490



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

[PATCH] D55980: [gn build] Add build file for clang/lib/FrontendTool

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55980



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

[PATCH] D55979: [gn build] Add build file for clang/lib/ARCMigrate

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55979



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

[PATCH] D55981: [gn build] Add build files for clang, clang-offload-bundler, and clang/lib/Headers

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn:6
+
+files = [
+  "adxintrin.h",

Why not just inline this? This variable doesn't seem to be used from anywhere 
else?


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

https://reviews.llvm.org/D55981



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

[PATCH] D55984: [gn build] Embed __TEXT __info_plist section into clang binary on macOS

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55984



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

[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55978



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

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

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

rsmith added inline comments.



Comment at: lib/Sema/SemaStmt.cpp:2846
 diag::warn_empty_range_based_for_body);
+  DiagnoseUnusedExprResult(B);
 

aaron.ballman wrote:
> rsmith wrote:
> > rsmith wrote:
> > > While this looks correct per the current approach to this function, we 
> > > really shouldn't be duplicating calls to this everywhere. Can we move all 
> > > the calls to a single call in `ActOnFinishFullStmt`?
> > Looks like that's not actually called from almost anywhere, but checking 
> > from `ActOnFinishFullExpr` in the case where `DiscardedValue` is `true` 
> > seems appropriate.
> That seems sensible, but how will that work with GNU statement expressions? 
> If we check for unused expression results, then we will trigger on code like 
> this:
> ```
> int a = ({blah(); yada(); 0;});
> // or
> int b = ({blah(); yada(); some_no_discard_call();});
> ```
> I don't know of a way to handle that case -- we call `ActOnFinishFullExpr()` 
> while parsing the compound statement for the `StmtExpr`, so there's no way to 
> know whether there's another statement coming (without lookahead) to 
> determine whether to suppress the diagnostic for the last expression 
> statement. Do you have ideas on how to handle that?
If we're calling `ActOnFinishFullExpr` there with `DiscardedValue == true`, 
that would be a bug that we should be fixing regardless. I don't think the 
lookahead is so bad itself -- it should just be a one-token lookahead for a `}` 
after the `;` -- but it might be awkward to wire it into our compound-statement 
/ expression-statement parsing. Still, it seems necessary for correctness.


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

https://reviews.llvm.org/D55955



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

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 179371.

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

https://reviews.llvm.org/D56025

Files:
  clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -235,6 +235,10 @@
   // CHECK-FIXES: static constexpr auto m0 = PASSTHROUGH(1U);
   static_assert(is_same::value, "");
   static_assert(m0 == 1, "");
+
+  // This location is inside a macro, no warning on that by default.
+#define MACRO 1u
+  int foo = MACRO;
 }
 
 // Check that user-defined literals do not cause any diags.
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I 
%S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -49,3 +49,8 @@
 * ``uL`` will be kept as is.
 * ``ull`` will be kept as is, since it is not in the list
 * and so on.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about literal suffixes inside macros.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -236,6 +236,10 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.h
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -35,6 +35,7 @@
   bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
 
   const std::vector NewSuffixes;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,12 +183,14 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
-  utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+  utils::options::parseStringList(Options.get("NewSuffixes", ""))),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NewSuffixes",
 utils::options::serializeStringList(NewSuffixes));
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
@@ -216,6 +218,8 @@
   // We might have a suffix that is already uppercase.
   if (auto Details = shouldReplaceLiteralSuffix(
   *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros)
+  return true;
 auto Complaint = diag(Details->LiteralLocation.getBegin(),
   "%0 literal has suffix '%1', which is not uppercase")
  << LiteralType::Name << Details->OldSuffix;


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -235,6 +235,10 @@
   // CHECK-FIXES: static constex

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

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



Comment at: 
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp:2-3
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 

lebedev.ri wrote:
> And the positive test?
Added a positive test now. :-)


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

https://reviews.llvm.org/D56025



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

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov accepted this revision.
george.karpenkov added a comment.

Seems good, but a comment explaining why this is necessary and why the crash 
follows otherwise would be great.


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

https://reviews.llvm.org/D55875



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

[PATCH] D56033: [CUDA] Treat extern global variable shadows same as regular extern vars.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: jlebar.
Herald added subscribers: bixia, sanjoy.

This fixes compiler crash when we attempted to compile this code:

  extern __device__ int data;
  __device__ int data = 1;


https://reviews.llvm.org/D56033

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/device-stub.cu


Index: clang/test/CodeGenCUDA/device-stub.cu
===
--- clang/test/CodeGenCUDA/device-stub.cu
+++ clang/test/CodeGenCUDA/device-stub.cu
@@ -42,13 +42,20 @@
 // ALL-DAG: @ext_host_var = external global i32
 extern int ext_host_var;
 
-// Shadows for external device-side variables are *definitions* of
-// those variables.
-// ALL-DAG: @ext_device_var = internal global i32
+// external device-side variables -> extern references to their shadows.
+// ALL-DAG: @ext_device_var = external global i32
 extern __device__ int ext_device_var;
-// ALL-DAG: @ext_device_var = internal global i32
+// ALL-DAG: @ext_device_var = external global i32
 extern __constant__ int ext_constant_var;
 
+// external device-side variables with definitiions should generate
+// definitions for the shadows.
+// ALL-DAG: @ext_device_var_def = internal global i32 undef,
+extern __device__ int ext_device_var_def;
+__device__ int ext_device_var_def = 1;
+// ALL-DAG: @ext_device_var_def = internal global i32 undef,
+__constant__ int ext_constant_var_def = 2;
+
 void use_pointers() {
   int *p;
   p = &device_var;
@@ -114,8 +121,8 @@
 // ALL: call{{.*}}[[PREFIX]]RegisterFunction(i8** %0, {{.*}}kernelfunc
 // ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, {{.*}}device_var{{.*}}i32 
0, i32 4, i32 0, i32 0
 // ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, 
{{.*}}constant_var{{.*}}i32 0, i32 4, i32 1, i32 0
-// ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, 
{{.*}}ext_device_var{{.*}}i32 1, i32 4, i32 0, i32 0
-// ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, 
{{.*}}ext_constant_var{{.*}}i32 1, i32 4, i32 1, i32 0
+// ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, 
{{.*}}ext_device_var_def{{.*}}i32 0, i32 4, i32 0, i32 0
+// ALL-DAG: call{{.*}}[[PREFIX]]RegisterVar(i8** %0, 
{{.*}}ext_constant_var_def{{.*}}i32 0, i32 4, i32 1, i32 0
 // ALL: ret void
 
 // Test that we've built a constructor.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2188,15 +2188,7 @@
   } else {
 const auto *VD = cast(Global);
 assert(VD->isFileVarDecl() && "Cannot emit local var decl as global.");
-// We need to emit device-side global CUDA variables even if a
-// variable does not have a definition -- we still need to define
-// host-side shadow for it.
-bool MustEmitForCuda = LangOpts.CUDA && !LangOpts.CUDAIsDevice &&
-   !VD->hasDefinition() &&
-   (VD->hasAttr() ||
-VD->hasAttr());
-if (!MustEmitForCuda &&
-VD->isThisDeclarationADefinition() != VarDecl::Definition &&
+if (VD->isThisDeclarationADefinition() != VarDecl::Definition &&
 !Context.isMSStaticDataMemberInlineDefinition(VD)) {
   if (LangOpts.OpenMP) {
 // Emit declaration of the must-be-emitted declare target variable.
@@ -3616,7 +3608,10 @@
   Flags |= CGCUDARuntime::ExternDeviceVar;
 if (D->hasAttr())
   Flags |= CGCUDARuntime::ConstantDeviceVar;
-getCUDARuntime().registerDeviceVar(*GV, Flags);
+// Extern global variables will be registered in the TU where they are
+// defined.
+if (!D->hasExternalStorage())
+  getCUDARuntime().registerDeviceVar(*GV, Flags);
   } else if (D->hasAttr())
 // __shared__ variables are odd. Shadows do get created, but
 // they are not registered with the CUDA runtime, so they


Index: clang/test/CodeGenCUDA/device-stub.cu
===
--- clang/test/CodeGenCUDA/device-stub.cu
+++ clang/test/CodeGenCUDA/device-stub.cu
@@ -42,13 +42,20 @@
 // ALL-DAG: @ext_host_var = external global i32
 extern int ext_host_var;
 
-// Shadows for external device-side variables are *definitions* of
-// those variables.
-// ALL-DAG: @ext_device_var = internal global i32
+// external device-side variables -> extern references to their shadows.
+// ALL-DAG: @ext_device_var = external global i32
 extern __device__ int ext_device_var;
-// ALL-DAG: @ext_device_var = internal global i32
+// ALL-DAG: @ext_device_var = external global i32
 extern __constant__ int ext_constant_var;
 
+// external device-side variables with definitiions should generate
+// definitions for the shadows.
+// ALL-DAG: @ext_device_var_def = internal global i32 undef,
+extern __device__ int ext_device_var_def;
+__device__ int ext_device_var_def = 1;
+// ALL-DAG: @ext_device_var_def = internal global i32 

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 179379.
NoQ added a comment.

Add a comment.

I guess it doesn't really matter why does this lead to a crash. The symbol 
itself is well-formed, but we probably don't support it yet in some place, and 
hopefully (but not necessarily) `CastRetrievedVal` is the only place we produce 
it. The point here is that the old behavior is clearly incorrect (i.e., the 
code doesn't do the right thing, and due to that the well-formed symbol is in 
fact not the symbol that we're looking for) and the new behavior is clearly 
conservative (i.e., returning `UnknownVal` should be pretty safe).

What we really need is a more direct test. Probably unit tests for 
`SValBuilder`, or some of those "denote - express" tests.


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

https://reviews.llvm.org/D55875

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c
  test/Analysis/casts.cpp


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 2; ++i)
+(void)(s.a[i] != nullptr); // no-crash
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,16 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // The dispatchCast() call below would round the int to a float. What we 
want,
+  // however, is a bit-by-bit reinterpretation of the int as a float, which
+  // usually yields nothing garbage. For now skip casts from ints to floats.
+  // TODO: What other combinations of types are affected?
+  if (castTy->isFloatingType()) {
+SymbolRef Sym = V.getAsSymbol();
+if (Sym && !Sym->getType()->isFloatingType())
+  return UnknownVal();
+  }
+
   // When retrieving symbolic pointer and expecting a non-void pointer,
   // wrap them into element regions of the expected type if necessary.
   // SValBuilder::dispatchCast() doesn't do that, but it is necessary to


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -102,3 +102,15 @@
   castToDerived(reinterpret_cast(ORef))->getNotInt();
 }
 } // namespace base_to_derived_opaque_class
+
+namespace bool_to_nullptr {
+struct S {
+  int *a[1];
+  bool b;
+};
+void foo(S s) {
+  s.b = true;
+  for (int i = 0; i < 2; ++i)
+(void)(s.a[i] != nullptr); // no-crash
+}
+} // namespace bool_to_nullptr
Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -213,3 +213,35 @@
 }
 
 #endif
+
+char no_crash_SymbolCast_of_float_type_aux(int *p) {
+  *p += 1;
+  return *p;
+}
+
+void no_crash_SymbolCast_of_float_type() {
+  extern float x;
+  char (*f)() = no_crash_SymbolCast_of_float_type_aux;
+  f(&x);
+}
+
+double no_crash_reinterpret_double_as_int(double a) {
+  *(int *)&a = 1;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_ptr(double a) {
+  *(void **)&a = 0;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_int(double a, int b) {
+  *(int *)&a = b;
+  return a * a;
+}
+
+double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
+  *(void **)&a = b;
+  return a * a;
+}
+
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -402,6 +402,16 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // The dispatchCast() call below would round the int to a float. What we want,
+  // however, is a bit-by-bit reinterpretation of the int as a float, which
+  // usually yields nothing garbage. For now skip casts from

[PATCH] D56033: [CUDA] Treat extern global variable shadows same as regular extern vars.

[Justin Lebar via Phabricator via cfe-commits]

jlebar accepted this revision.
jlebar added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CodeGenCUDA/device-stub.cu:51
 
+// external device-side variables with definitiions should generate
+// definitions for the shadows.

definiitions


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

https://reviews.llvm.org/D56033



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