[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

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

balazske added inline comments.



Comment at: lib/AST/ASTStructuralEquivalence.cpp:913
 
-  if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) {
+  if (!D1->getDeclName() && !D2->getDeclName()) {
 // If both anonymous structs/unions are in a record context, make sure

The problem was that value of `isAnonymousStructOrUnion` is not (or not yet) 
set correctly during the import? But `getDeclName` is always correct (if it is 
a `false` the record is anonymous struct)?



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:486
 struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+  RecordDecl* getRecordDecl(FieldDecl *FD) {
+auto *ET = cast(FD->getType().getTypePtr());

`RecordDecl *`



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:578
+
+  ASSERT_TRUE(R0);
+  ASSERT_TRUE(R1);

Is it possible at all that getRecordDecl returns nullptr? It uses cast (nor 
cast_or_null and not dyn_cast).



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:581
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));

a_sidorin wrote:
> Do we really want to test the equivalence of decl to itself, not to its 
> imported version?
I think this is correct: `R0` should be equivalent with itself but not with 
`R1`.


Repository:
  rC Clang

https://reviews.llvm.org/D49296



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

r337148 - [Sema] Add fixit for unused lambda captures

[Alexander Shaposhnikov via cfe-commits]

Author: alexshap
Date: Mon Jul 16 00:23:47 2018
New Revision: 337148

URL: http://llvm.org/viewvc/llvm-project?rev=337148&view=rev
Log:
[Sema] Add fixit for unused lambda captures

This diff adds a fixit to suggest removing unused lambda captures 
in the appropriate diagnostic.

Patch by Andrew Comminos!

Test plan: make check-all

Differential revision: https://reviews.llvm.org/D48845

Added:
cfe/trunk/test/FixIt/fixit-unused-lambda-capture.cpp
Modified:
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=337148&r1=337147&r2=337148&view=diff
==
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Mon Jul 16 00:23:47 2018
@@ -2540,12 +2540,16 @@ struct LambdaIntroducer {
 LambdaCaptureInitKind InitKind;
 ExprResult Init;
 ParsedType InitCaptureType;
+SourceRange ExplicitRange;
+
 LambdaCapture(LambdaCaptureKind Kind, SourceLocation Loc,
   IdentifierInfo *Id, SourceLocation EllipsisLoc,
   LambdaCaptureInitKind InitKind, ExprResult Init,
-  ParsedType InitCaptureType)
+  ParsedType InitCaptureType,
+  SourceRange ExplicitRange)
 : Kind(Kind), Loc(Loc), Id(Id), EllipsisLoc(EllipsisLoc),
-  InitKind(InitKind), Init(Init), InitCaptureType(InitCaptureType) {}
+  InitKind(InitKind), Init(Init), InitCaptureType(InitCaptureType),
+  ExplicitRange(ExplicitRange) {}
   };
 
   SourceRange Range;
@@ -2563,9 +2567,10 @@ struct LambdaIntroducer {
   SourceLocation EllipsisLoc,
   LambdaCaptureInitKind InitKind,
   ExprResult Init, 
-  ParsedType InitCaptureType) {
+  ParsedType InitCaptureType,
+  SourceRange ExplicitRange) {
 Captures.push_back(LambdaCapture(Kind, Loc, Id, EllipsisLoc, InitKind, 
Init,
- InitCaptureType));
+ InitCaptureType, ExplicitRange));
   }
 };
 

Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=337148&r1=337147&r2=337148&view=diff
==
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Mon Jul 16 00:23:47 2018
@@ -829,6 +829,9 @@ public:
   ///  if the enclosing full-expression is instantiation dependent).
   llvm::SmallSet NonODRUsedCapturingExprs;
 
+  /// A map of explicit capture indices to their introducer source ranges.
+  llvm::DenseMap ExplicitCaptureRanges;
+
   /// Contains all of the variables defined in this lambda that shadow 
variables
   /// that were defined in parent contexts. Used to avoid warnings when the
   /// shadowed variables are uncaptured by this lambda.

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=337148&r1=337147&r2=337148&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Jul 16 00:23:47 2018
@@ -5602,8 +5602,10 @@ public:
   /// Does copying/destroying the captured variable have side effects?
   bool CaptureHasSideEffects(const sema::Capture &From);
 
-  /// Diagnose if an explicit lambda capture is unused.
-  void DiagnoseUnusedLambdaCapture(const sema::Capture &From);
+  /// Diagnose if an explicit lambda capture is unused. Returns true if a
+  /// diagnostic is emitted.
+  bool DiagnoseUnusedLambdaCapture(SourceRange CaptureRange,
+   const sema::Capture &From);
 
   /// Complete a lambda-expression having processed and attached the
   /// lambda body.

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=337148&r1=337147&r2=337148&view=diff
==
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Mon Jul 16 00:23:47 2018
@@ -808,6 +808,7 @@ Optional Parser::ParseLambdaIn
 IdentifierInfo *Id = nullptr;
 SourceLocation EllipsisLoc;
 ExprResult Init;
+SourceLocation LocStart = Tok.getLocation();
 
 if (Tok.is(tok::star)) {
   Loc = ConsumeToken(); 
@@ -981,8 +982,11 @@ Optional Parser::ParseLambdaIn
   Loc, Kind == LCK_ByRef, Id, InitKind, InitExpr);
   Init = InitExpr;
 }

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Alexander Shaposhnikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337148: [Sema] Add fixit for unused lambda captures 
(authored by alexshap, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48845?vs=155616&id=155624#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,8 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  if (!LSI->Captures.empty())
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   continue;
 }
 
@@ -1139,6 +1141,8 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+if (!LSI->Captures.empty())
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,19 +1482,22 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+bool Sema::DiagnoseUnusedLambdaCapture(SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
-return;
+return false;
 
   if (From.isVLATypeCapture())
-return;
+return false;
 
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
+  return true;
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,18 +1539,49 @@
 
 // Translate captures.
 auto CurField = Class->field_begin();
+// True if the current capture has a used capture or default before it.
+bool CurHasPreviousCapture = CaptureDefault != LCD_None;
+SourceLocation PrevCaptureLoc = CurHasPreviousCapture ?
+CaptureDefaultLoc : IntroducerRange.getBegin();
+
 for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) {
   const Capture &From = LSI->Captures[I];
+
   assert(!From.isBlockCapture() && "Cannot capture __block variables");
   bool IsImplicit = I >= LSI->NumExplicitCaptures;
 
+  // Use source ranges of explicit captures for fixits where available.
+  SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I];
+
   // Warn about unused explicit captures.
+  bool IsCaptureUsed = true;
   if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) {
 // Initialized captures that are non-ODR used may not be eliminated.
 bool NonODRUsedInitCapture =
 IsGenericLambda && From.isNonODRUsed() && From.getInitExpr();
-if (!NonODRUsedInitCapture)
-  DiagnoseUnusedLambdaCapture(From);
+if (!NonODRUsedInitCapture) {
+  bool IsLast = (I + 1) == LSI->NumExplicitCaptures;
+	SourceRange FixItRange;
+  if (CaptureRange.isValid()) {
+if (!CurHasPreviousCapture && !IsLast) {
+  // If there are no captures preceding this capture, remove the
+  // following comma.
+  FixItRange = SourceRange(CaptureRange.getBegin(),
+   getLocForEndOfToken(CaptureRange.getEnd()));
+} else {
+  // Otherwise, remove the comma since the last used capture.
+  FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc),
+   CaptureRange.getEnd());
+}
+  }
+
+  IsCaptureUsed = !DiagnoseUnusedLambdaCapture(FixItRange, From);
+}
+  }
+
+  if (CaptureRange.isValid()) {
+CurHasPreviousCapture |= IsCaptureUsed;
+PrevCaptureLoc = CaptureRange.getEnd();
   }
 
   // Handle 'this' capture.
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -808,6 +808,7 @@
 IdentifierInfo *Id = nullptr;
 SourceLocation EllipsisLoc;
 ExprResult Init;
+SourceLocation LocStart = Tok.getLocation();
 
 if (Tok.is(tok::star)) {
   Loc = ConsumeToken(); 
@@ -981,8 +982,11 @@
   Loc, Kind == LCK_ByRef, Id, InitKind, InitExpr);
   Init = InitExpr;
 }
+
+SourceLocation LocEnd = PrevTokLocation;
+
 Intro.addCapture(Kind, Loc, Id, EllipsisLoc, InitKind, Init,
- InitCaptureType);
+   

[PATCH] D49245: [ASTImporter] Import implicit methods of existing class.

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

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

- Small style and comment changes.


Repository:
  rC Clang

https://reviews.llvm.org/D49245

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2343,6 +2343,117 @@
  compoundStmt(has(callExpr(has(unresolvedMemberExpr());
 }
 
+class ImportImplicitMethods : public ASTImporterTestBase {
+public:
+  static constexpr auto DefaultCode = R"(
+  struct A { int x; };
+  void f() {
+A a;
+A a1(a);
+A a2(A{});
+a = a1;
+a = A{};
+a.~A();
+  })";
+
+  template 
+  void testImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/1u);
+  }
+
+  template 
+  void testNoImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/0u);
+  }
+
+private:
+  template 
+  void test(const MatcherType &MethodMatcher,
+  const char *Code, unsigned int ExpectedCount) {
+auto ClassMatcher = cxxRecordDecl(unless(isImplicit()));
+
+Decl *ToTU = getToTuDecl(Code, Lang_CXX11);
+auto *ToClass = FirstDeclMatcher().match(
+ToTU, ClassMatcher);
+
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1);
+
+{
+  CXXMethodDecl *Method =
+  FirstDeclMatcher().match(ToClass, MethodMatcher);
+  ToClass->removeDecl(Method);
+}
+
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0);
+
+Decl *ImportedClass = nullptr;
+{
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input1.cc");
+  auto *FromClass = FirstDeclMatcher().match(
+  FromTU, ClassMatcher);
+  ImportedClass = Import(FromClass, Lang_CXX11);
+}
+
+EXPECT_EQ(ToClass, ImportedClass);
+EXPECT_EQ(DeclCounter().match(ToClass, MethodMatcher),
+ExpectedCount);
+  }
+};
+
+TEST_P(ImportImplicitMethods, DefaultConstructor) {
+  testImportOf(cxxConstructorDecl(isDefaultConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, CopyConstructor) {
+  testImportOf(cxxConstructorDecl(isCopyConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, MoveConstructor) {
+  testImportOf(cxxConstructorDecl(isMoveConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, Destructor) {
+  testImportOf(cxxDestructorDecl());
+}
+
+TEST_P(ImportImplicitMethods, CopyAssignment) {
+  testImportOf(cxxMethodDecl(isCopyAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, MoveAssignment) {
+  testImportOf(cxxMethodDecl(isMoveAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportUserProvided) {
+  auto Code = R"(
+  struct A { A() { int x; } };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDefault) {
+  auto Code = R"(
+  struct A { A() = default; };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDeleted) {
+  auto Code = R"(
+  struct A { A() = delete; };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportOtherMethod) {
+  auto Code = R"(
+  struct A { void f() { } };
+  )";
+  testNoImportOf(cxxMethodDecl(hasName("f")), Code);
+}
+
 TEST_P(ASTImporterTestBase, ImportOfEquivalentRecord) {
   Decl *ToR1;
   {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -228,6 +228,7 @@
 void ImportDeclarationNameLoc(const DeclarationNameInfo &From,
   DeclarationNameInfo& To);
 void ImportDeclContext(DeclContext *FromDC, bool ForceImport = false);
+void ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To);
 
 bool ImportCastPath(CastExpr *E, CXXCastPath &Path);
 
@@ -1253,6 +1254,16 @@
 Importer.Import(From);
 }
 
+void ASTNodeImporter::ImportImplicitMethods(
+const CXXRecordDecl *From, CXXRecordDecl *To) {
+  assert(From->isCompleteDefinition() && To->getDefinition() == To &&
+  "Import implicit methods to or from non-definition");
+  
+  for (CXXMethodDecl *FromM : From->methods())
+if (FromM->isImplicit())
+  Importer.Import(FromM);
+}
+
 static void setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
@@ -2199,8 +2210,19 @@
 // The record types structurally match, or the "from" translation
 // unit only had a forward declaration anyway; call it the same
 // function.
-// FIXME: For C++,

[PATCH] D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr.

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

balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6741
+
+  auto *Ctor = dyn_cast(Importer.Import(
+  E->getConstructor()));

a_sidorin wrote:
> cast_or_null?
dyn_cast_or_null: Import may return nullptr, but if not, the cast should 
succeed (not a CXXConstructorDecl would be error).
Or (but some lines more code) check separately for null return from Import, and 
then use `cast`?


Repository:
  rC Clang

https://reviews.llvm.org/D49293



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

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

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

balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2715
+if (auto *ToFT = dyn_cast(Importer.Import(FromFT)))
+  ToFunction->setDescribedFunctionTemplate(ToFT);
+else

a_sidorin wrote:
> The function template should be already set after 
> `getDescribedFunctionTemplate()` is imported in 
> `VisitFunctionTemplateDecl()`. Are there still cases not covered by this?
Yes this call can be omitted. But check of return value of Import is needed 
(the current code is not good: nullptr is not accepted by dyn_cast).


Repository:
  rC Clang

https://reviews.llvm.org/D49235



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

[PATCH] D48395: Added PublicOnly flag

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D48395#1158869, @juliehockett wrote:

> Remember to mark comments as done when they are. Otherwise, LGTM unless 
> @ioeric has any concerns.


No concern if this looks good to Julie.




Comment at: clang-tools-extra/test/clang-doc/yaml-module.cpp:14
+// CHECK-A: ---
+// CHECK-A-NEXT: USR: 
'{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-A-NEXT: Name:'moduleFunction'

This could be `[0-9A-Z]{N}` where N = length(USR), right?


https://reviews.llvm.org/D48395



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

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

Ping?


https://reviews.llvm.org/D46013



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

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

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

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

- Removed setDescribedFunctionTemplate call.


Repository:
  rC Clang

https://reviews.llvm.org/D49235

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1125,7 +1125,7 @@
 }
 
 TEST_P(ASTImporterTestBase,
-   DISABLED_ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) {
+   ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) {
   Decl *FromTU = getTuDecl("template void f(){}", Lang_CXX);
   auto FromFT = FirstDeclMatcher().match(
   FromTU, functionTemplateDecl());
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2499,6 +2499,7 @@
 return ToD;
 
   const FunctionDecl *FoundByLookup = nullptr;
+  FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
 
   // If this is a function template specialization, then try to find the same
   // existing specialization in the "to" context. The localUncachedLookup
@@ -2525,6 +2526,14 @@
   if (!FoundDecl->isInIdentifierNamespace(IDNS))
 continue;
 
+  // If template was found, look at the templated function.
+  if (FromFT) {
+if (auto *Template = dyn_cast(FoundDecl))
+  FoundDecl = Template->getTemplatedDecl();
+else
+  continue;
+  }
+
   if (auto *FoundFunction = dyn_cast(FoundDecl)) {
 if (FoundFunction->hasExternalFormalLinkage() &&
 D->hasExternalFormalLinkage()) {
@@ -2700,6 +2709,11 @@
 ToFunction->setType(T);
   }
 
+  // Import the describing template function, if any.
+  if (FromFT)
+if (!Importer.Import(FromFT))
+  return nullptr;
+
   if (D->doesThisDeclarationHaveABody()) {
 if (Stmt *FromBody = D->getBody()) {
   if (Stmt *ToBody = Importer.Import(FromBody)) {


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1125,7 +1125,7 @@
 }
 
 TEST_P(ASTImporterTestBase,
-   DISABLED_ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) {
+   ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) {
   Decl *FromTU = getTuDecl("template void f(){}", Lang_CXX);
   auto FromFT = FirstDeclMatcher().match(
   FromTU, functionTemplateDecl());
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2499,6 +2499,7 @@
 return ToD;
 
   const FunctionDecl *FoundByLookup = nullptr;
+  FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
 
   // If this is a function template specialization, then try to find the same
   // existing specialization in the "to" context. The localUncachedLookup
@@ -2525,6 +2526,14 @@
   if (!FoundDecl->isInIdentifierNamespace(IDNS))
 continue;
 
+  // If template was found, look at the templated function.
+  if (FromFT) {
+if (auto *Template = dyn_cast(FoundDecl))
+  FoundDecl = Template->getTemplatedDecl();
+else
+  continue;
+  }
+
   if (auto *FoundFunction = dyn_cast(FoundDecl)) {
 if (FoundFunction->hasExternalFormalLinkage() &&
 D->hasExternalFormalLinkage()) {
@@ -2700,6 +2709,11 @@
 ToFunction->setType(T);
   }
 
+  // Import the describing template function, if any.
+  if (FromFT)
+if (!Importer.Import(FromFT))
+  return nullptr;
+
   if (D->doesThisDeclarationHaveABody()) {
 if (Stmt *FromBody = D->getBody()) {
   if (Stmt *ToBody = Importer.Import(FromBody)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337151 - [Analyzer] Mark `SymbolData` parts of iterator position as live in program state maps

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Mon Jul 16 02:27:27 2018
New Revision: 337151

URL: http://llvm.org/viewvc/llvm-project?rev=337151&view=rev
Log:
[Analyzer] Mark `SymbolData` parts of iterator position as live in program 
state maps

Marking a symbolic expression as live is non-recursive. In our checkers we
either use conjured symbols or conjured symbols plus/minus integers to
represent abstract position of iterators, so in this latter case we also
must mark the `SymbolData` part of these symbolic expressions as live to
prevent them from getting reaped.

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=337151&r1=337150&r2=337151&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Mon Jul 16 
02:27:27 2018
@@ -488,14 +488,18 @@ void IteratorChecker::checkLiveSymbols(P
   // alive
   auto RegionMap = State->get();
   for (const auto Reg : RegionMap) {
-const auto Pos = Reg.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Reg.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto SymbolMap = State->get();
   for (const auto Sym : SymbolMap) {
-const auto Pos = Sym.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Sym.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto ContMap = State->get();
@@ -1157,21 +1161,31 @@ ProgramStateRef relateIteratorPositions(
 const IteratorPosition &Pos2,
 bool Equal) {
   auto &SVB = State->getStateManager().getSValBuilder();
+
+  // FIXME: This code should be reworked as follows:
+  // 1. Subtract the operands using evalBinOp().
+  // 2. Assume that the result doesn't overflow.
+  // 3. Compare the result to 0.
+  // 4. Assume the result of the comparison.
   const auto comparison =
   SVB.evalBinOp(State, BO_EQ, nonloc::SymbolVal(Pos1.getOffset()),
-nonloc::SymbolVal(Pos2.getOffset()), 
SVB.getConditionType())
-  .getAs();
+nonloc::SymbolVal(Pos2.getOffset()),
+SVB.getConditionType());
 
-  if (comparison) {
-auto NewState = State->assume(*comparison, Equal);
-if (const auto CompSym = comparison->getAsSymbol()) {
-  return assumeNoOverflow(NewState, cast(CompSym)->getLHS(), 
2);
-}
+  assert(comparison.getAs() &&
+"Symbol comparison must be a `DefinedSVal`");
 
-return NewState;
+  auto NewState = State->assume(comparison.castAs(), Equal);
+  if (const auto CompSym = comparison.getAsSymbol()) {
+assert(isa(CompSym) &&
+   "Symbol comparison must be a `SymIntExpr`");
+assert(BinaryOperator::isComparisonOp(
+   cast(CompSym)->getOpcode()) &&
+   "Symbol comparison must be a comparison");
+return assumeNoOverflow(NewState, cast(CompSym)->getLHS(), 2);
   }
 
-  return State;
+  return NewState;
 }
 
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
@@ -1225,14 +1239,12 @@ bool compare(ProgramStateRef State, NonL
   auto &SVB = State->getStateManager().getSValBuilder();
 
   const auto comparison =
-  SVB.evalBinOp(State, Opc, NL1, NL2, SVB.getConditionType())
-  .getAs();
+SVB.evalBinOp(State, Opc, NL1, NL2, SVB.getConditionType());
 
-  if (comparison) {
-return !State->assume(*comparison, false);
-  }
+  assert(comparison.getAs() &&
+"Symbol comparison must be a `DefinedSVal`");
 
-  return false;
+  return !State->assume(comparison.castAs(), false);
 }
 
 } // namespace


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

[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337151: [Analyzer] Mark `SymbolData` parts of iterator 
position as live in program… (authored by baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48764?vs=153698&id=155634#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48764

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -488,14 +488,18 @@
   // alive
   auto RegionMap = State->get();
   for (const auto Reg : RegionMap) {
-const auto Pos = Reg.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Reg.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto SymbolMap = State->get();
   for (const auto Sym : SymbolMap) {
-const auto Pos = Sym.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Sym.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto ContMap = State->get();
@@ -1157,21 +1161,31 @@
 const IteratorPosition &Pos2,
 bool Equal) {
   auto &SVB = State->getStateManager().getSValBuilder();
+
+  // FIXME: This code should be reworked as follows:
+  // 1. Subtract the operands using evalBinOp().
+  // 2. Assume that the result doesn't overflow.
+  // 3. Compare the result to 0.
+  // 4. Assume the result of the comparison.
   const auto comparison =
   SVB.evalBinOp(State, BO_EQ, nonloc::SymbolVal(Pos1.getOffset()),
-nonloc::SymbolVal(Pos2.getOffset()), 
SVB.getConditionType())
-  .getAs();
+nonloc::SymbolVal(Pos2.getOffset()),
+SVB.getConditionType());
 
-  if (comparison) {
-auto NewState = State->assume(*comparison, Equal);
-if (const auto CompSym = comparison->getAsSymbol()) {
-  return assumeNoOverflow(NewState, cast(CompSym)->getLHS(), 
2);
-}
+  assert(comparison.getAs() &&
+"Symbol comparison must be a `DefinedSVal`");
 
-return NewState;
+  auto NewState = State->assume(comparison.castAs(), Equal);
+  if (const auto CompSym = comparison.getAsSymbol()) {
+assert(isa(CompSym) &&
+   "Symbol comparison must be a `SymIntExpr`");
+assert(BinaryOperator::isComparisonOp(
+   cast(CompSym)->getOpcode()) &&
+   "Symbol comparison must be a comparison");
+return assumeNoOverflow(NewState, cast(CompSym)->getLHS(), 2);
   }
 
-  return State;
+  return NewState;
 }
 
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
@@ -1225,14 +1239,12 @@
   auto &SVB = State->getStateManager().getSValBuilder();
 
   const auto comparison =
-  SVB.evalBinOp(State, Opc, NL1, NL2, SVB.getConditionType())
-  .getAs();
+SVB.evalBinOp(State, Opc, NL1, NL2, SVB.getConditionType());
 
-  if (comparison) {
-return !State->assume(*comparison, false);
-  }
+  assert(comparison.getAs() &&
+"Symbol comparison must be a `DefinedSVal`");
 
-  return false;
+  return !State->assume(comparison.castAs(), false);
 }
 
 } // namespace


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -488,14 +488,18 @@
   // alive
   auto RegionMap = State->get();
   for (const auto Reg : RegionMap) {
-const auto Pos = Reg.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Reg.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto SymbolMap = State->get();
   for (const auto Sym : SymbolMap) {
-const auto Pos = Sym.second;
-SR.markLive(Pos.getOffset());
+const auto Offset = Sym.second.getOffset();
+for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i)
+  if (isa(*i))
+SR.markLive(*i);
   }
 
   auto ContMap = State->get();
@@ -1157,21 +1161,31 @@
 const IteratorPosition &Pos2,
 bool Equal) {
   auto &SVB = State->getStateManager().getSValBuilder();
+
+  // FIXME: This code should be reworked as follows:
+  // 1. Subtract the operands using evalBinOp().
+  // 2. Assume that the result doesn't overflow.
+  // 3. Compare the result to 0.
+  // 4. Assume the result of the comparison.
   const auto comparison =
   SVB.eval

[PATCH] D48560: [clangd] JSON <-> XPC conversions

[Sam McCall via Phabricator via cfe-commits]

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

Sorry it took so long for these to get attention.
Starting here because it's simple and helps me understand the bigger patches
This one looks good, really just nits.




Comment at: clangd/xpc/CMakeLists.txt:1
+set(LLVM_LINK_COMPONENTS
+  support

I think it'd be helpful for some of us confused linux people to have a brief 
README in this directory giving some context about XPC in clangd.

Anything you want to write about the architecture is great but the essentials 
would be:
 - alternative transport layer to JSON-RPC
 - semantically equivalent, uses same LSP request/response message types (?)
 - MacOS only. Feature is guarded by `CLANGD_BUILD_XPC_SUPPORT`, including 
whole `xpc/` dir.



Comment at: clangd/xpc/CMakeLists.txt:15
+
+# FIXME: Factor out json::Expr from clangDaemon
+target_link_libraries(ClangdXpcTests

this is done :-)



Comment at: xpc/XPCJSONConversions.cpp:22
+case json::Expr::Boolean: return 
xpc_bool_create(json.asBoolean().getValue());
+case json::Expr::Number:  return 
xpc_double_create(json.asNumber().getValue());
+case json::Expr::String:  return 
xpc_string_create(json.asString().getValue().str().c_str());

The underlying JSON library actually has both `int64_t` and `double` 
representations, and I see XPC has both as well.
If `double` works for all the data sent over this channel, then this approach 
is simplest. But for completeness, this is also possible:
```
if (auto I = json.asInteger())
  return xpc_int64_create(*I);
return xpc_double_create(*json.asNumber());
```

(I don't know of any reason clangd would use large integers today unless they 
arrived as incoming JSON-RPC request IDs or similar)



Comment at: xpc/XPCJSONConversions.cpp:40
+  }
+  // Get pointers only now so there's no possible re-allocation of keys.
+  for(const auto& k : keys)

(you could also pre-size the vector, or use a deque)



Comment at: xpc/XPCJSONConversions.cpp:62
+  if (objType == XPC_TYPE_UINT64)
+return json::Expr(xpc_uint64_get_value(xpcObj));
+  if (objType == XPC_TYPE_STRING)

hmm, I think this shouldn't compile anymore, rather require you to explicitly 
do the narrowing conversion to int64 or double.



Comment at: xpc/XPCJSONConversions.cpp:69
+  xpcObj,
+  ^bool(size_t /* index */, xpc_object_t value) {
+result.emplace_back(xpcToJson(value));

this looks like objC syntax, I'm not familiar with it, but should this file be 
`.mm`?

Alternatively, it seems like you could write a C++ loop with 
`xpc_array_get_value` (though I don't know if there are performance concerns).

(and dictionary)



Comment at: xpc/XPCJSONConversions.cpp:87
+  }
+  llvm_unreachable("unsupported JSON data type in xpcToJson() conversion");
+}

there are other data types (error, fd, ...) and this data presumably comes over 
a socket or something, so you may not actually want UB here.
Consider an assert and returning json::Expr(nullptr)



Comment at: xpc/XPCJSONConversions.h:1
+//===--- XPCDispatcher.h - Main XPC service entry point -*- C++ 
-*-===//
+//

nit: header names wrong file.



Comment at: xpc/XPCJSONConversions.h:1
+//===--- XPCDispatcher.h - Main XPC service entry point -*- C++ 
-*-===//
+//

sammccall wrote:
> nit: header names wrong file.
consider just calling this `Conversion.h` or similar - it's already in the 
`xpc/` dir and, it seems like XPC conversions that **weren't** JSON-related 
would be likely to go here anyhow?



Comment at: xpc/XPCJSONConversions.h:19
+
+xpc_object_t jsonToXpc(const json::Expr& json);
+json::Expr xpcToJson(const xpc_object_t& json);

you could consider calling these `json::Expr toJSON(const xpc_object_t&)` and 
`bool fromJSON(const json::Expr&, xpc_object_t&)` to fit in with the json lib's 
convention - not sure if it actually buys you anything though.

(If so they should be in the global namespace so ADL works)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48560



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

r337152 - [Sema] Reword warning for constant captures that are not required

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Jul 16 02:52:02 2018
New Revision: 337152

URL: http://llvm.org/viewvc/llvm-project?rev=337152&view=rev
Log:
[Sema] Reword warning for constant captures that are not required

This is one of the darker corners of C++, make it clear that this is
about constants and rephrase it a bit.

Before: lambda capture 'i' is not required to be captured for this use
After:  lambda capture of constant 'i' is not required for this use

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337152&r1=337151&r2=337152&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 16 02:52:02 
2018
@@ -341,8 +341,8 @@ def warn_unneeded_member_function : Warn
   InGroup, DefaultIgnore;
 def warn_unused_private_field: Warning<"private field %0 is not used">,
   InGroup, DefaultIgnore;
-def warn_unused_lambda_capture: Warning<"lambda capture %0 is not "
-  "%select{used|required to be captured for this use}1">,
+def warn_unused_lambda_capture: Warning<"lambda capture "
+  "%select{|of constant }1%0 is not %select{used|required for this use}1">,
   InGroup, DefaultIgnore;
 
 def warn_parameter_size: Warning<

Modified: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp?rev=337152&r1=337151&r2=337152&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp Mon Jul 16 02:52:02 
2018
@@ -41,9 +41,9 @@ void test() {
   auto explicit_by_value_used = [i] { return i + 1; };
   auto explicit_by_value_used_void = [i] { (void)i; };
   auto explicit_by_value_unused = [i] {}; // expected-warning{{lambda capture 
'i' is not used}}
-  auto explicit_by_value_unused_sizeof = [i] { return sizeof(i); }; // 
expected-warning{{lambda capture 'i' is not required to be captured for this 
use}}
-  auto explicit_by_value_unused_decltype = [i] { decltype(i) j = 0; }; // 
expected-warning{{lambda capture 'i' is not required to be captured for this 
use}}
-  auto explicit_by_value_unused_const = [k] { return k + 1; }; // 
expected-warning{{lambda capture 'k' is not required to be captured for this 
use}}
+  auto explicit_by_value_unused_sizeof = [i] { return sizeof(i); }; // 
expected-warning{{lambda capture of constant 'i' is not required for this use}}
+  auto explicit_by_value_unused_decltype = [i] { decltype(i) j = 0; }; // 
expected-warning{{lambda capture of constant 'i' is not required for this use}}
+  auto explicit_by_value_unused_const = [k] { return k + 1; }; // 
expected-warning{{lambda capture of constant 'k' is not required for this use}}
 
   auto explicit_by_reference_used = [&i] { i++; };
   auto explicit_by_reference_unused = [&i] {}; // expected-warning{{lambda 
capture 'i' is not used}}
@@ -146,10 +146,10 @@ void test_templated() {
   auto explicit_by_value_used_void = [i] { (void)i; };
 
   auto explicit_by_value_unused = [i] {}; // expected-warning{{lambda capture 
'i' is not used}}
-  auto explicit_by_value_unused_sizeof = [i] { return sizeof(i); }; // 
expected-warning{{lambda capture 'i' is not required to be captured for this 
use}}
+  auto explicit_by_value_unused_sizeof = [i] { return sizeof(i); }; // 
expected-warning{{lambda capture of constant 'i' is not required for this use}}
   auto explicit_by_value_unused_decltype = [i] { decltype(i) j = 0; }; // 
expected-warning{{lambda capture 'i' is not used}}
-  auto explicit_by_value_unused_const = [k] { return k + 1; }; // 
expected-warning{{lambda capture 'k' is not required to be captured for this 
use}}
-  auto explicit_by_value_unused_const_generic = [k](auto c) { return k + 1; }; 
// expected-warning{{lambda capture 'k' is not required to be captured for this 
use}}
+  auto explicit_by_value_unused_const = [k] { return k + 1; }; // 
expected-warning{{lambda capture of constant 'k' is not required for this use}}
+  auto explicit_by_value_unused_const_generic = [k](auto c) { return k + 1; }; 
// expected-warning{{lambda capture of constant 'k' is not required for this 
use}}
 
   auto explicit_by_reference_used = [&i] { i++; };
   auto explicit_by_reference_unused = [&i] {}; // expected-warning{{lambda 
capture 'i' is not used}}


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

[PATCH] D48562: [clangd] XPC transport layer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Just an initial couple of thoughts here, haven't yet been through in detail.

Mostly I wonder if we can use slightly different abstractions in 
https://reviews.llvm.org/D48559 to so the JSON/XPC parts get the code reuse we 
want but the work required to call one vs the other is smaller. Need to think 
about this more.




Comment at: Features.inc.in:1
+#define CLANGD_ENABLE_XPC_SUPPORT @CLANGD_BUILD_XPC_SUPPORT@

nit: can we give these the same name?
I'd vote for dropping the `_SUPPORT` too: either `CLANGD_ENABLE_XPC` or 
`CLANGD_BUILD_XPC`.

Maybe the latter is better, since an environment variable controls the runtime 
behavior anyway.



Comment at: tool/ClangdMain.cpp:261
+#ifdef CLANGD_ENABLE_XPC_SUPPORT
+  if (getenv("CLANGD_AS_XPC_SERVICE")) {
+XPCDispatcher Dispatcher([](const json::Expr &Params) {

is there a reason this is an environment variable rather than an `-xpc` flag?

Apart from this being (mostly) what we do elsewhere, it seems like if the 
process spawning clangd wants XPC suppport, but clangd was built without it, 
then you want the process to fail and exit rather than sit there listening on 
stdin. Flags have this behavior, env variables do not.



Comment at: tool/ClangdMain.cpp:262
+  if (getenv("CLANGD_AS_XPC_SERVICE")) {
+XPCDispatcher Dispatcher([](const json::Expr &Params) {
+  replyError(ErrorCode::MethodNotFound, "method not found");

the duplication here suggests to me the factoring isn't quite right.

naively, ISTM we should be able to have two implementations of an interface 
here rather than an actual ifdef'd server loop. But I'll dig into the 
implementation to try to understand a bit more.


https://reviews.llvm.org/D48562



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

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D49074#1160793, @NoQ wrote:

> I'd also rather stick to integer arithmetic and avoid using floats even in 
> intermediate calculations. It'd be hard to make sure that no rounding errors 
> kick in if we use floats.


Yes, I agree. I think an integer plus a flag (whether to take the reciprocal) 
is better since we only use n or 1/n for scaling.


https://reviews.llvm.org/D49074



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

r337162 - [ASTImporter] Import implicit methods of existing class.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Mon Jul 16 05:16:39 2018
New Revision: 337162

URL: http://llvm.org/viewvc/llvm-project?rev=337162&view=rev
Log:
[ASTImporter] Import implicit methods of existing class.

Summary:
When an already existing class is encountered during import,
check if it has implicit methods that are missing in the existing one,
and import these.
The to-be-imported code may use the same class in different way than the
existing (before the import) code. This may result in that there are
implicit methods that are not generated for the existing code.

Reviewers: a.sidorin, a_sidorin

Reviewed By: a_sidorin

Subscribers: a_sidorin, martong, cfe-commits

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=337162&r1=337161&r2=337162&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Jul 16 05:16:39 2018
@@ -228,6 +228,7 @@ namespace clang {
 void ImportDeclarationNameLoc(const DeclarationNameInfo &From,
   DeclarationNameInfo& To);
 void ImportDeclContext(DeclContext *FromDC, bool ForceImport = false);
+void ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To);
 
 bool ImportCastPath(CastExpr *E, CXXCastPath &Path);
 
@@ -1253,6 +1254,16 @@ void ASTNodeImporter::ImportDeclContext(
 Importer.Import(From);
 }
 
+void ASTNodeImporter::ImportImplicitMethods(
+const CXXRecordDecl *From, CXXRecordDecl *To) {
+  assert(From->isCompleteDefinition() && To->getDefinition() == To &&
+  "Import implicit methods to or from non-definition");
+  
+  for (CXXMethodDecl *FromM : From->methods())
+if (FromM->isImplicit())
+  Importer.Import(FromM);
+}
+
 static void setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
@@ -2199,8 +2210,19 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
 // The record types structurally match, or the "from" translation
 // unit only had a forward declaration anyway; call it the same
 // function.
-// FIXME: For C++, we should also merge methods here.
-return Importer.MapImported(D, FoundDef);
+// FIXME: Structural equivalence check should check for same
+// user-defined methods.
+Importer.MapImported(D, FoundDef);
+if (const auto *DCXX = dyn_cast(D)) {
+  auto *FoundCXX = dyn_cast(FoundDef);
+  assert(FoundCXX && "Record type mismatch");
+
+  if (D->isCompleteDefinition() && !Importer.isMinimalImport())
+// FoundDef may not have every implicit method that D has
+// because implicit methods are created only if they are used.
+ImportImplicitMethods(DCXX, FoundCXX);
+}
+return FoundDef;
   }
 } else if (!D->isCompleteDefinition()) {
   // We have a forward declaration of this type, so adopt that forward

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=337162&r1=337161&r2=337162&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Jul 16 05:16:39 2018
@@ -2343,6 +2343,117 @@ TEST_P(ImportExpr, UnresolvedMemberExpr)
  compoundStmt(has(callExpr(has(unresolvedMemberExpr());
 }
 
+class ImportImplicitMethods : public ASTImporterTestBase {
+public:
+  static constexpr auto DefaultCode = R"(
+  struct A { int x; };
+  void f() {
+A a;
+A a1(a);
+A a2(A{});
+a = a1;
+a = A{};
+a.~A();
+  })";
+
+  template 
+  void testImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/1u);
+  }
+
+  template 
+  void testNoImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/0u);
+  }
+
+private:
+  template 
+  void test(const MatcherType &MethodMatcher,
+  const char *Code, unsigned int ExpectedCount) {
+auto ClassMatcher = cxxRecordDecl(unless(isImplicit()));
+
+Decl *ToTU = getToTuDecl(Code, Lang_CXX11);
+auto *ToClass = FirstDeclMatcher().match(
+ToTU, ClassMatcher);
+
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1);
+
+{
+  CXXMethodDecl *Method =
+  FirstDeclMatcher().match(ToClass, Metho

[PATCH] D49245: [ASTImporter] Import implicit methods of existing class.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337162: [ASTImporter] Import implicit methods of existing 
class. (authored by balazske, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D49245?vs=155625&id=155648#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49245

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2343,6 +2343,117 @@
  compoundStmt(has(callExpr(has(unresolvedMemberExpr());
 }
 
+class ImportImplicitMethods : public ASTImporterTestBase {
+public:
+  static constexpr auto DefaultCode = R"(
+  struct A { int x; };
+  void f() {
+A a;
+A a1(a);
+A a2(A{});
+a = a1;
+a = A{};
+a.~A();
+  })";
+
+  template 
+  void testImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/1u);
+  }
+
+  template 
+  void testNoImportOf(
+  const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+test(MethodMatcher, Code, /*ExpectedCount=*/0u);
+  }
+
+private:
+  template 
+  void test(const MatcherType &MethodMatcher,
+  const char *Code, unsigned int ExpectedCount) {
+auto ClassMatcher = cxxRecordDecl(unless(isImplicit()));
+
+Decl *ToTU = getToTuDecl(Code, Lang_CXX11);
+auto *ToClass = FirstDeclMatcher().match(
+ToTU, ClassMatcher);
+
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1);
+
+{
+  CXXMethodDecl *Method =
+  FirstDeclMatcher().match(ToClass, MethodMatcher);
+  ToClass->removeDecl(Method);
+}
+
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0);
+
+Decl *ImportedClass = nullptr;
+{
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input1.cc");
+  auto *FromClass = FirstDeclMatcher().match(
+  FromTU, ClassMatcher);
+  ImportedClass = Import(FromClass, Lang_CXX11);
+}
+
+EXPECT_EQ(ToClass, ImportedClass);
+EXPECT_EQ(DeclCounter().match(ToClass, MethodMatcher),
+ExpectedCount);
+  }
+};
+
+TEST_P(ImportImplicitMethods, DefaultConstructor) {
+  testImportOf(cxxConstructorDecl(isDefaultConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, CopyConstructor) {
+  testImportOf(cxxConstructorDecl(isCopyConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, MoveConstructor) {
+  testImportOf(cxxConstructorDecl(isMoveConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, Destructor) {
+  testImportOf(cxxDestructorDecl());
+}
+
+TEST_P(ImportImplicitMethods, CopyAssignment) {
+  testImportOf(cxxMethodDecl(isCopyAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, MoveAssignment) {
+  testImportOf(cxxMethodDecl(isMoveAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportUserProvided) {
+  auto Code = R"(
+  struct A { A() { int x; } };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDefault) {
+  auto Code = R"(
+  struct A { A() = default; };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDeleted) {
+  auto Code = R"(
+  struct A { A() = delete; };
+  )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportOtherMethod) {
+  auto Code = R"(
+  struct A { void f() { } };
+  )";
+  testNoImportOf(cxxMethodDecl(hasName("f")), Code);
+}
+
 TEST_P(ASTImporterTestBase, ImportOfEquivalentRecord) {
   Decl *ToR1;
   {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -228,6 +228,7 @@
 void ImportDeclarationNameLoc(const DeclarationNameInfo &From,
   DeclarationNameInfo& To);
 void ImportDeclContext(DeclContext *FromDC, bool ForceImport = false);
+void ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To);
 
 bool ImportCastPath(CastExpr *E, CXXCastPath &Path);
 
@@ -1253,6 +1254,16 @@
 Importer.Import(From);
 }
 
+void ASTNodeImporter::ImportImplicitMethods(
+const CXXRecordDecl *From, CXXRecordDecl *To) {
+  assert(From->isCompleteDefinition() && To->getDefinition() == To &&
+  "Import implicit methods to or from non-definition");
+  
+  for (CXXMethodDecl *FromM : From->methods())
+if (FromM->isImplicit())
+  Importer.Import(FromM);
+}
+
 static void setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
@@ -2199,8 +2210,19 @@
 // The record types structur

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Yes.

https://github.com/JonasToth/clang-tools-extra/tree/check_const

This is the branch i work on. I got it up to date with the current
master for CTE. :)

Am 14.07.2018 um 20:59 schrieb Florin Iucha via Phabricator:

> 0x8000- added a comment.
> 
> In https://reviews.llvm.org/D45444#1162715, @JonasToth wrote:
> 
>> Time :/
> 
> Is this available as a public Git branch somewhere, or is downloading the 
> diff the preferred way to interact with this code?
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D45444


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D49356: [clang-tidy: modernize] Fix modernize-use-equals-default with {} brackets list initialization: patch

[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 with a formatting nit.




Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:100
 AccessToFieldInParam,
+   initListExpr(has(AccessToFieldInParam)),
 cxxConstructExpr(allOf(

Did clang-format produce this formatting? If not, can you clang-format this 
patch?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49356



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

r337167 - [analyzer] Fix constraint being dropped when analyzing a program without taint tracking enabled

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Mon Jul 16 06:14:46 2018
New Revision: 337167

URL: http://llvm.org/viewvc/llvm-project?rev=337167&view=rev
Log:
[analyzer] Fix constraint being dropped when analyzing a program without taint 
tracking enabled

Summary:
This patch removes the constraint dropping when taint tracking is disabled.

It also voids the crash reported in D28953 by treating a SymSymExpr with non 
pointer symbols as an opaque expression.

Updated the regressions and verifying the big projects now; I'll update here 
when they're done.

Based on the discussion on the mailing list and the patches by @ddcc.

Reviewers: george.karpenkov, NoQ, ddcc, baloghadamsoftware

Reviewed By: george.karpenkov

Subscribers: delcypher, llvm-commits, rnkovacs, xazax.hun, szepet, a.sidorin, 
ddcc

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/test/Analysis/PR37855.c
cfe/trunk/test/Analysis/bitwise-ops.c
cfe/trunk/test/Analysis/std-c-library-functions.c
cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=337167&r1=337166&r2=337167&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Mon Jul 16 06:14:46 
2018
@@ -390,7 +390,7 @@ unsigned AnalyzerOptions::getGraphTrimIn
 
 unsigned AnalyzerOptions::getMaxSymbolComplexity() {
   if (!MaxSymbolComplexity.hasValue())
-MaxSymbolComplexity = getOptionAsInteger("max-symbol-complexity", 1);
+MaxSymbolComplexity = getOptionAsInteger("max-symbol-complexity", 25);
   return MaxSymbolComplexity.getValue();
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp?rev=337167&r1=337166&r2=337167&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp Mon Jul 16 
06:14:46 2018
@@ -52,17 +52,18 @@ ProgramStateRef RangedConstraintManager:
 assert(BinaryOperator::isComparisonOp(Op));
 
 // For now, we only support comparing pointers.
-assert(Loc::isLocType(SSE->getLHS()->getType()));
-assert(Loc::isLocType(SSE->getRHS()->getType()));
-QualType DiffTy = SymMgr.getContext().getPointerDiffType();
-SymbolRef Subtraction =
-SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);
+if (Loc::isLocType(SSE->getLHS()->getType()) &&
+Loc::isLocType(SSE->getRHS()->getType())) {
+  QualType DiffTy = SymMgr.getContext().getPointerDiffType();
+  SymbolRef Subtraction =
+  SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);
 
-const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
-Op = BinaryOperator::reverseComparisonOp(Op);
-if (!Assumption)
-  Op = BinaryOperator::negateComparisonOp(Op);
-return assumeSymRel(State, Subtraction, Op, Zero);
+  const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
+  Op = BinaryOperator::reverseComparisonOp(Op);
+  if (!Assumption)
+Op = BinaryOperator::negateComparisonOp(Op);
+  return assumeSymRel(State, Subtraction, Op, Zero);
+}
   }
 
   // If we get here, there's nothing else we can do but treat the symbol as

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=337167&r1=337166&r2=337167&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Mon Jul 16 06:14:46 2018
@@ -379,11 +379,9 @@ SVal SValBuilder::makeSymExprValNN(Progr
BinaryOperator::Opcode Op,
NonLoc LHS, NonLoc RHS,
QualType ResultTy) {
-  if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();
-
   const SymExpr *symLHS = LHS.getAsSymExpr();
   const SymExpr *symRHS = RHS.getAsSymExpr();
+
   // TODO: When the Max Complexity is reached, we should conjure a symbol
   // instead of generating an Unknown value and propagate the taint info to it.
   const unsigned MaxComp = StateMgr.getOwningEngine()

Modified: cfe/trunk/test/Analysis/PR37855.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/PR37855.c?rev=337167&

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 155660.
martong marked 6 inline comments as done.
martong added a comment.

Address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D49296

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/StructuralEquivalenceTest.cpp

Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -42,6 +42,21 @@
 return std::make_tuple(D0, D1);
   }
 
+  std::tuple makeTuDecls(
+  const std::string &SrcCode0, const std::string &SrcCode1, Language Lang) {
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
+   AST1->getASTContext().getTranslationUnitDecl());
+  }
+
   // Get a pair of node pointers into the synthesized AST from the given code
   // snippets. The same matcher is used for both snippets.
   template 
@@ -62,15 +77,15 @@
 return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
-  bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+  bool testStructuralMatch(Decl *D0, Decl *D1) {
 llvm::DenseSet> NonEquivalentDecls;
 StructuralEquivalenceContext Ctx(
 D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
 StructuralEquivalenceKind::Default, false, false);
 return Ctx.IsStructurallyEquivalent(D0, D1);
   }
 
-  bool testStructuralMatch(std::tuple t) {
+  bool testStructuralMatch(std::tuple t) {
 return testStructuralMatch(get<0>(t), get<1>(t));
   }
 };
@@ -468,6 +483,11 @@
 }
 
 struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+  // FIXME Use a common getRecordDecl with ASTImporterTest.cpp!
+  RecordDecl* getRecordDecl(FieldDecl *FD) {
+auto *ET = cast(FD->getType().getTypePtr());
+return cast(ET->getNamedType().getTypePtr())->getDecl();
+  };
 };
 
 TEST_F(StructuralEquivalenceRecordTest, Name) {
@@ -535,6 +555,71 @@
   EXPECT_TRUE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceRecordTest, UnnamedRecordsShouldBeInequivalent) {
+  auto t = makeTuDecls(
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )",
+  "", Lang_C);
+  auto *TU = get<0>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry0")));
+  auto *Entry1 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry1")));
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+TEST_F(StructuralEquivalenceRecordTest,
+   UnnamedRecordsShouldBeInequivalentEvenIfTheSecondIsBeingDefined) {
+  auto Code =
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )";
+  auto t = makeTuDecls(Code, Code, Lang_C);
+
+  auto *FromTU = get<0>(t);
+  auto *Entry1 =
+  FirstDeclMatcher().match(FromTU, fieldDecl(hasName("entry1")));
+
+  auto *ToTU = get<1>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(ToTU, fieldDecl(hasName("entry0")));
+  auto *A =
+  FirstDeclMatcher().match(ToTU, recordDecl(hasName("A")));
+  A->startDefinition(); // Set isBeingDefined, getDefinition() will return a
+// nullptr. This may be the case during ASTImport.
+
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+  ASSERT_TRUE(R0);
+  ASSERT_TRUE(R1);
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+
 TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) {
   auto t = makeNamedDecls(
   "struct A{ }; struct B{ }; void foo(A a, A b);",
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -273,6 +273,11 @@
   }
 };
 
+template  RecordDecl *getRecordDecl(T *D) {
+  auto *ET = cast(D->getType().getTypePtr());
+  return cast(ET->getNamedType().getTypePtr())->getDecl();
+};
+
 // This class provides generic methods to write tests which can check internal
 // attributes of AST nodes like getPreviousDecl(), isVirtual(), etc. Also,
 // this fixture makes it possible to import from several "From" cont

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTStructuralEquivalence.cpp:913
 
-  if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) {
+  if (!D1->getDeclName() && !D2->getDeclName()) {
 // If both anonymous structs/unions are in a record context, make sure

balazske wrote:
> The problem was that value of `isAnonymousStructOrUnion` is not (or not yet) 
> set correctly during the import? But `getDeclName` is always correct (if it 
> is a `false` the record is anonymous struct)?
There is a strange thing here: A struct without a name is not necessarily an 
anonymous struct!
For example the RecordDecl in `struct { int i; float f; } obj;` is not 
anonymous.
See the docs for `RecordDecl::isAnonymousStructOrUnion`:
```
  /// Whether this is an anonymous struct or union. To be an anonymous
  /// struct or union, it must have been declared without a name and
  /// there must be no objects of this type declared, e.g.,
  /// @code
  ///   union { int i; float f; };
  /// @endcode
  /// is an anonymous union but neither of the following are:
  /// @code
  ///  union X { int i; float f; };
  ///  union { int i; float f; } obj;
  /// @endcode
  bool isAnonymousStructOrUnion() const { return AnonymousStructOrUnion; }
```
This might seem very perplexed, but this is aligned with the C++17 Standard:
```
12.3.1 Anonymous unions
para 3. A union for which objects, pointers, or references are declared is not 
an anonymous union.
```

Consequently, we should check whether the Decl has a name, 
`isAnonymousStructOrUnion` is misleading here.



Comment at: unittests/AST/ASTImporterTest.cpp:2495
+  FirstDeclMatcher().match(ToTU, fieldDecl(hasName("entry1")));
+  auto getRecordDecl = [](FieldDecl *FD) {
+auto *ET = cast(FD->getType().getTypePtr());

a_sidorin wrote:
> There is already a similar lambda definitions for `getRecordDecl()` in 
> ObjectsWithUnnamedStructType test. Can we make it a function, like it is done 
> for StructuralEquivalenceContext?
OK, I added a new function template to handle both cases.



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:578
+
+  ASSERT_TRUE(R0);
+  ASSERT_TRUE(R1);

balazske wrote:
> Is it possible at all that getRecordDecl returns nullptr? It uses cast (nor 
> cast_or_null and not dyn_cast).
Yes, good catch, changed it.



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:581
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));

balazske wrote:
> a_sidorin wrote:
> > Do we really want to test the equivalence of decl to itself, not to its 
> > imported version?
> I think this is correct: `R0` should be equivalent with itself but not with 
> `R1`.
I think it makes sense, because from the `StructuralEquivalence`'s viewpoint 
there is no imported version, just two Decls/Types which may or may not have 
the same ASTContext.


Repository:
  rC Clang

https://reviews.llvm.org/D49296



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

r337169 - [analyzer] Fix the Z3 backend always generating unsigned APSInt

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Mon Jul 16 06:32:22 2018
New Revision: 337169

URL: http://llvm.org/viewvc/llvm-project?rev=337169&view=rev
Log:
[analyzer] Fix the Z3 backend always generating unsigned APSInt

Summary:
In `toAPSInt`, the Z3 backend was not checking the variable `Int`'s type and 
was always generating unsigned `APSInt`s.

This was found by accident when I removed:
```
llvm::APSInt ConvertedLHS, ConvertedRHS;
QualType LTy, RTy;
std::tie(ConvertedLHS, LTy) = fixAPSInt(*LHS);
std::tie(ConvertedRHS, RTy) = fixAPSInt(*RHS);
-doIntTypePromotion(
-ConvertedLHS, LTy, ConvertedRHS, RTy);
return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
```

And the `BasicValueFactory` started to complain about different `signedness`.

Reviewers: george.karpenkov, NoQ, ddcc

Reviewed By: ddcc

Subscribers: xazax.hun, szepet, a.sidorin

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

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp?rev=337169&r1=337168&r2=337169&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp Mon Jul 16 
06:32:22 2018
@@ -681,12 +681,14 @@ public:
   Z3_get_numeral_uint64(Z3Context::ZC, AST,
 reinterpret_cast<__uint64 *>(&Value[0]));
   if (Sort.getBitvectorSortSize() <= 64) {
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]),
+   Int.isUnsigned());
   } else if (Sort.getBitvectorSortSize() == 128) {
 Z3Expr ASTHigh = Z3Expr(Z3_mk_extract(Z3Context::ZC, 127, 64, AST));
 Z3_get_numeral_uint64(Z3Context::ZC, AST,
   reinterpret_cast<__uint64 *>(&Value[1]));
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value),
+   Int.isUnsigned());
   } else {
 assert(false && "Bitwidth not supported!");
 return false;
@@ -702,7 +704,7 @@ public:
   llvm::APInt(Int.getBitWidth(),
   Z3_get_bool_value(Z3Context::ZC, AST) == Z3_L_TRUE ? 1
  : 0),
-  true);
+  Int.isUnsigned());
   return true;
 }
   }


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

[PATCH] D49305: [analyzer] Fix the Z3 backend always generating unsigned APSInt

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337169: [analyzer] Fix the Z3 backend always generating 
unsigned APSInt (authored by mramalho, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D49305

Files:
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp


Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -681,12 +681,14 @@
   Z3_get_numeral_uint64(Z3Context::ZC, AST,
 reinterpret_cast<__uint64 *>(&Value[0]));
   if (Sort.getBitvectorSortSize() <= 64) {
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]),
+   Int.isUnsigned());
   } else if (Sort.getBitvectorSortSize() == 128) {
 Z3Expr ASTHigh = Z3Expr(Z3_mk_extract(Z3Context::ZC, 127, 64, AST));
 Z3_get_numeral_uint64(Z3Context::ZC, AST,
   reinterpret_cast<__uint64 *>(&Value[1]));
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value),
+   Int.isUnsigned());
   } else {
 assert(false && "Bitwidth not supported!");
 return false;
@@ -702,7 +704,7 @@
   llvm::APInt(Int.getBitWidth(),
   Z3_get_bool_value(Z3Context::ZC, AST) == Z3_L_TRUE ? 1
  : 0),
-  true);
+  Int.isUnsigned());
   return true;
 }
   }


Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -681,12 +681,14 @@
   Z3_get_numeral_uint64(Z3Context::ZC, AST,
 reinterpret_cast<__uint64 *>(&Value[0]));
   if (Sort.getBitvectorSortSize() <= 64) {
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value[0]),
+   Int.isUnsigned());
   } else if (Sort.getBitvectorSortSize() == 128) {
 Z3Expr ASTHigh = Z3Expr(Z3_mk_extract(Z3Context::ZC, 127, 64, AST));
 Z3_get_numeral_uint64(Z3Context::ZC, AST,
   reinterpret_cast<__uint64 *>(&Value[1]));
-Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value), true);
+Int = llvm::APSInt(llvm::APInt(Int.getBitWidth(), Value),
+   Int.isUnsigned());
   } else {
 assert(false && "Bitwidth not supported!");
 return false;
@@ -702,7 +704,7 @@
   llvm::APInt(Int.getBitWidth(),
   Z3_get_bool_value(Z3Context::ZC, AST) == Z3_L_TRUE ? 1
  : 0),
-  true);
+  Int.isUnsigned());
   return true;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Aleksei,

Unfortunately, it seems that it is very to hard synthesize a lit or unit test 
for this fix.
I have found this flaw during the CTU analysis of Bitcoin, where the AST was 
immense.

Because of the lack of tests, now I am trying to persuade you based on some 
theory.
Let's consider this simple BFS algorithm from the `s` source:

  void bfs(Graph G, int s)
  {
Queue queue = new Queue();
marked[s] = true; // Mark the source
queue.enqueue(s); // and put it on the queue.
while (!q.isEmpty()) {
  int v = queue.dequeue(); // Remove next vertex from the queue.
  for (int w : G.adj(v))
if (!marked[w]) // For every unmarked adjacent vertex,
{
  marked[w] = true;
  queue.enqueue(w);
}
}
  }

I believe , the structural equivalence check could be implemented as a parallel 
BFS on a pair of graphs.
And, I think that must have been the original approach at the beginning.
Indeed, it has it's queue, which holds pairs of nodes, one from each graph, 
this is the `DeclsToCheck` and it's pair is in `TentativeEquivalences`.
`TentativeEquivalences` also plays the role of the marking (`marked`) 
functionality above, we use it to check whether we've already seen a pair of 
nodes.

We put in the elements into the queue only in the toplevel decl check function:

  static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
   Decl *D1, Decl *D2);

The `while` loop where we iterate over the children is implemented in 
`Finish()`.
And `Finish` is called only from the two **member** functions which check the 
equivalency of two Decls or two Types. ASTImporter (and other clients) call 
only these functions.

The `static` implementation functions are called from `Finish`, these push the 
children nodes to the queue.
So far so good, this is almost like the BFS.
However, if we let a static implementation function to call `Finish` via 
another **member** function that means we end up with two nested while loops 
each of them working on the same queue. This is wrong and nobody can reason 
about it's doing.

So, now `TentativeEquivalences` plays two roles. It is used to store the second 
half of the decls which we want to compare, plus it plays a role in closing the 
recursion. On a long term, I think, (after this change) we could refactor 
structural equivalency to be more alike to the traditional BFS.


Repository:
  rC Clang

https://reviews.llvm.org/D49300



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

r337172 - [ASTImporter] Changed constant int to unsigned int in test code.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Mon Jul 16 07:05:18 2018
New Revision: 337172

URL: http://llvm.org/viewvc/llvm-project?rev=337172&view=rev
Log:
[ASTImporter] Changed constant int to unsigned int in test code.

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=337172&r1=337171&r2=337172&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Jul 16 07:05:18 2018
@@ -2378,7 +2378,7 @@ private:
 auto *ToClass = FirstDeclMatcher().match(
 ToTU, ClassMatcher);
 
-ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1);
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1u);
 
 {
   CXXMethodDecl *Method =
@@ -2386,7 +2386,7 @@ private:
   ToClass->removeDecl(Method);
 }
 
-ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0);
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0u);
 
 Decl *ImportedClass = nullptr;
 {


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

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
kosarev added reviewers: SjoerdMeijer, jgreenhalgh, rengolin.
kosarev added a project: clang.
Herald added a reviewer: javed.absar.

https://reviews.llvm.org/D49375

Files:
  include/clang/Basic/arm_neon.td
  test/Sema/arm-no-fp16.c


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon 
-target-feature -fp16 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon \
+// RUN:   -fallow-half-arguments-and-returns -target-feature -fp16 \
+// RUN:   -fsyntax-only -verify
 
 #include 
 
@@ -9,3 +11,19 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of 
function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vmaxnm_f16(float16x4_t a, float16x4_t b) {
+  return vmaxnm_f16(a, b); // expected-warning{{implicit declaration of 
function 'vmaxnm_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vmaxnmq_f16(float16x8_t a, float16x8_t b) {
+  return vmaxnmq_f16(a, b); // expected-warning{{implicit declaration of 
function 'vmaxnmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vminnm_f16(float16x4_t a, float16x4_t b) {
+  return vminnm_f16(a, b); // expected-warning{{implicit declaration of 
function 'vminnm_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vminnmq_f16(float16x8_t a, float16x8_t b) {
+  return vminnmq_f16(a, b); // expected-warning{{implicit declaration of 
function 'vminnmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1463,8 +1463,10 @@
   // Max/Min
   def VMAXH : SInst<"vmax", "ddd", "hQh">;
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_NUMERIC_MAXMIN) && 
defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
+def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  }
 
   // Multiplication/Division
   def VMULH : SOpInst<"vmul", "ddd", "hQh", OP_MUL>;


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon -target-feature -fp16 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon \
+// RUN:   -fallow-half-arguments-and-returns -target-feature -fp16 \
+// RUN:   -fsyntax-only -verify
 
 #include 
 
@@ -9,3 +11,19 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vmaxnm_f16(float16x4_t a, float16x4_t b) {
+  return vmaxnm_f16(a, b); // expected-warning{{implicit declaration of function 'vmaxnm_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vmaxnmq_f16(float16x8_t a, float16x8_t b) {
+  return vmaxnmq_f16(a, b); // expected-warning{{implicit declaration of function 'vmaxnmq_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vminnm_f16(float16x4_t a, float16x4_t b) {
+  return vminnm_f16(a, b); // expected-warning{{implicit declaration of function 'vminnm_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vminnmq_f16(float16x8_t a, float16x8_t b) {
+  return vminnmq_f16(a, b); // expected-warning{{implicit declaration of function 'vminnmq_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1463,8 +1463,10 @@
   // Max/Min
   def VMAXH : SInst<"vmax", "ddd", "hQh">;
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : SInst<"

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
kosarev added reviewers: SjoerdMeijer, jgreenhalgh, rengolin.
kosarev added a project: clang.
Herald added a reviewer: javed.absar.

https://reviews.llvm.org/D49376

Files:
  include/clang/Basic/arm_neon.td
  test/Sema/arm-no-fp16.c


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -9,3 +9,59 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of 
function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vrnda_f16(float16x4_t a) {
+  return vrnda_f16(a); // expected-warning{{implicit declaration of function 
'vrnda_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndaq_f16(float16x8_t a) {
+  return vrndaq_f16(a); // expected-warning{{implicit declaration of function 
'vrndaq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrnd_f16(float16x4_t a) {
+  return vrnd_f16(a); // expected-warning{{implicit declaration of function 
'vrnd_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndq_f16(float16x8_t a) {
+  return vrndq_f16(a); // expected-warning{{implicit declaration of function 
'vrndq_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndi_f16(float16x4_t a) {
+  return vrndi_f16(a); // expected-warning{{implicit declaration of function 
'vrndi_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndiq_f16(float16x8_t a) {
+  return vrndiq_f16(a); // expected-warning{{implicit declaration of function 
'vrndiq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndm_f16(float16x4_t a) {
+  return vrndm_f16(a); // expected-warning{{implicit declaration of function 
'vrndm_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndmq_f16(float16x8_t a) {
+  return vrndmq_f16(a); // expected-warning{{implicit declaration of function 
'vrndmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndn_f16(float16x4_t a) {
+  return vrndn_f16(a); // expected-warning{{implicit declaration of function 
'vrndn_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndnq_f16(float16x8_t a) {
+  return vrndnq_f16(a); // expected-warning{{implicit declaration of function 
'vrndnq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndp_f16(float16x4_t a) {
+  return vrndp_f16(a); // expected-warning{{implicit declaration of function 
'vrndp_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndpq_f16(float16x8_t a) {
+  return vrndpq_f16(a); // expected-warning{{implicit declaration of function 
'vrndpq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndx_f16(float16x4_t a) {
+  return vrndx_f16(a); // expected-warning{{implicit declaration of function 
'vrndx_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndxq_f16(float16x8_t a) {
+  return vrndxq_f16(a); // expected-warning{{implicit declaration of function 
'vrndxq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1416,12 +1416,14 @@
   def VCVTP_U16: SInst<"vcvtp_u16", "ud", "hQh">;
 
   // Vector rounding
-  def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
-  def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
-  def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
-  def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
-  def FRINTMH  : SInst<"vrndm", "dd", "hQh">;
-  def FRINTXH  : SInst<"vrndx", "dd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_DIRECTED_ROUNDING) 
&& defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
+def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
+def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
+def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
+def FRINTMH  : SInst<"vr

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 155678.
martong added a comment.

RecordDecl* -> RecordDecl *


Repository:
  rC Clang

https://reviews.llvm.org/D49296

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/StructuralEquivalenceTest.cpp

Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -42,6 +42,21 @@
 return std::make_tuple(D0, D1);
   }
 
+  std::tuple makeTuDecls(
+  const std::string &SrcCode0, const std::string &SrcCode1, Language Lang) {
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
+   AST1->getASTContext().getTranslationUnitDecl());
+  }
+
   // Get a pair of node pointers into the synthesized AST from the given code
   // snippets. The same matcher is used for both snippets.
   template 
@@ -62,15 +77,15 @@
 return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
-  bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+  bool testStructuralMatch(Decl *D0, Decl *D1) {
 llvm::DenseSet> NonEquivalentDecls;
 StructuralEquivalenceContext Ctx(
 D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
 StructuralEquivalenceKind::Default, false, false);
 return Ctx.IsStructurallyEquivalent(D0, D1);
   }
 
-  bool testStructuralMatch(std::tuple t) {
+  bool testStructuralMatch(std::tuple t) {
 return testStructuralMatch(get<0>(t), get<1>(t));
   }
 };
@@ -468,6 +483,11 @@
 }
 
 struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+  // FIXME Use a common getRecordDecl with ASTImporterTest.cpp!
+  RecordDecl *getRecordDecl(FieldDecl *FD) {
+auto *ET = cast(FD->getType().getTypePtr());
+return cast(ET->getNamedType().getTypePtr())->getDecl();
+  };
 };
 
 TEST_F(StructuralEquivalenceRecordTest, Name) {
@@ -535,6 +555,71 @@
   EXPECT_TRUE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceRecordTest, UnnamedRecordsShouldBeInequivalent) {
+  auto t = makeTuDecls(
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )",
+  "", Lang_C);
+  auto *TU = get<0>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry0")));
+  auto *Entry1 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry1")));
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+TEST_F(StructuralEquivalenceRecordTest,
+   UnnamedRecordsShouldBeInequivalentEvenIfTheSecondIsBeingDefined) {
+  auto Code =
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )";
+  auto t = makeTuDecls(Code, Code, Lang_C);
+
+  auto *FromTU = get<0>(t);
+  auto *Entry1 =
+  FirstDeclMatcher().match(FromTU, fieldDecl(hasName("entry1")));
+
+  auto *ToTU = get<1>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(ToTU, fieldDecl(hasName("entry0")));
+  auto *A =
+  FirstDeclMatcher().match(ToTU, recordDecl(hasName("A")));
+  A->startDefinition(); // Set isBeingDefined, getDefinition() will return a
+// nullptr. This may be the case during ASTImport.
+
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+  ASSERT_TRUE(R0);
+  ASSERT_TRUE(R1);
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+
 TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) {
   auto t = makeNamedDecls(
   "struct A{ }; struct B{ }; void foo(A a, A b);",
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -273,6 +273,11 @@
   }
 };
 
+template  RecordDecl *getRecordDecl(T *D) {
+  auto *ET = cast(D->getType().getTypePtr());
+  return cast(ET->getNamedType().getTypePtr())->getDecl();
+};
+
 // This class provides generic methods to write tests which can check internal
 // attributes of AST nodes like getPreviousDecl(), isVirtual(), etc. Also,
 // this fixture makes it possible to import from several "From" contexts.
@@ -1755,11 +1760,6 @@
   )"

r337185 - Restore "[ThinLTO] Ensure we always select the same function copy to import"

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Mon Jul 16 08:30:36 2018
New Revision: 337185

URL: http://llvm.org/viewvc/llvm-project?rev=337185&view=rev
Log:
Restore "[ThinLTO] Ensure we always select the same function copy to import"

This reverts commit r337082, restoring r337051, since the LLVM side
patch has been restored.

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=337185&r1=337184&r2=337185&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Jul 16 08:30:36 2018
@@ -1127,9 +1127,8 @@ static void runThinLTOBackend(ModuleSumm
 // e.g. record required linkage changes.
 if (Summary->modulePath() == M->getModuleIdentifier())
   continue;
-// Doesn't matter what value we plug in to the map, just needs an entry
-// to provoke importing by thinBackend.
-ImportList[Summary->modulePath()][GUID] = 1;
+// Add an entry to provoke importing by thinBackend.
+ImportList[Summary->modulePath()].insert(GUID);
   }
 
   std::vector> OwnedImports;


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

[PATCH] D49338: Implement - P0122R7

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Need to add an entry to `include/CMakeLists.txt` as well.


https://reviews.llvm.org/D49338



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:372
+
+auto __len = __last - __p;
+if (__value != 0 || !__len)

Are you missing an edge case here? What happens if `__last == __first && 
__value == 0`?




Comment at: test/support/charconv_test_helpers.h:40
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{

We don't need to use ugly names here in the test suite.



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D45805: [libcxx] Remove redundant specializations in type_traits.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a subscriber: mclow.lists.
ldionne added inline comments.



Comment at: include/type_traits:595
-template
-struct __and_<_B0, _B1> : conditional<_B0::value, _B1, _B0>::type {};
-

It is not impossible that this was a compile-time optimization for the 
(probably very common) case where there's only 2 arguments. I'd like to have 
Marshall's input on that, since he seems to have written that code. Otherwise, 
LGTM. Ping @mclow.lists 


Repository:
  rCXX libc++

https://reviews.llvm.org/D45805



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

Same comment as above about `read` and `inner_product` - they need to be "ugly 
names"



Comment at: include/charconv:358
+
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];

I just want you to reassure me here - this lambda gets inlined, right?


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:359
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];
+};

Thinking some more - did this used to do more? Because I don't see why having a 
lambda here is a benefit, as compared to:

const char *__digits = "0123456789abcdefghijklmnopqrstuvwxyz";

and
*--p = digits[__c];



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

mclow.lists wrote:
> Same comment as above about `read` and `inner_product` - they need to be 
> "ugly names"
Unlike `traits` which is a template parameter name in the standard, `read` and 
`inner_product` are function names in the standard, which means the users 
cannot make a macro for them (and there is no guarantee about what name you 
make **not** get by including certain headers), so we don't need to use ugly 
names here, am I right?



Comment at: include/charconv:358
+
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];

mclow.lists wrote:
> I just want you to reassure me here - this lambda gets inlined, right?
Yes -- I read my code in assembly.



Comment at: include/charconv:359
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];
+};

mclow.lists wrote:
> Thinking some more - did this used to do more? Because I don't see why having 
> a lambda here is a benefit, as compared to:
> 
> const char *__digits = "0123456789abcdefghijklmnopqrstuvwxyz";
> 
> and
> *--p = digits[__c];
> 
I use a lambda here because it may do more in the future.  If someone wants to 
let it support non-ASCII platforms, then they only need to make a patch against 
this lambda rather than changing the sites of uses.  After all, there is 
nothing wrong to abstract out anything into a function, I think...


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Any chance this can/should be unit tested? (also, in general (though might
not matter in this instance), symmetric operators like == should be
implemented as non-members (though they can still be friends and if they
are, can be defined inline in the class definition as a member could be),
so any implicit conversions apply equally to the LHS as the RHS of the
expression)


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

Re: [PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via cfe-commits]

Any chance this can/should be unit tested? (also, in general (though might
not matter in this instance), symmetric operators like == should be
implemented as non-members (though they can still be friends and if they
are, can be defined inline in the class definition as a member could be),
so any implicit conversions apply equally to the LHS as the RHS of the
expression)

On Thu, Jul 12, 2018 at 1:48 PM Simon Marchi via Phabricator via
cfe-commits  wrote:

> simark created this revision.
> Herald added subscribers: cfe-commits, ioeric, ilya-biryukov.
>
> It does the obvious thing of comparing all fields.  This will be needed
> for a clangd patch I have in the pipeline.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D49265
>
> Files:
>   include/clang/Tooling/CompilationDatabase.h
>
>
> Index: include/clang/Tooling/CompilationDatabase.h
> ===
> --- include/clang/Tooling/CompilationDatabase.h
> +++ include/clang/Tooling/CompilationDatabase.h
> @@ -59,6 +59,11 @@
>
>/// The output file associated with the command.
>std::string Output;
> +
> +  bool operator==(const CompileCommand &RHS) const {
> +return Directory == RHS.Directory && Filename == RHS.Filename &&
> +   CommandLine == RHS.CommandLine && Output == RHS.Output;
> +  }
>  };
>
>  /// Interface for compilation databases.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337191 - [OPENMP, NVPTX] Globalize only captured variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 09:49:20 2018
New Revision: 337191

URL: http://llvm.org/viewvc/llvm-project?rev=337191&view=rev
Log:
[OPENMP, NVPTX] Globalize only captured variables.

Sometimes we can try to globalize non-variable declarations, which may
lead to compiler crash.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=337191&r1=337190&r2=337191&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Jul 16 09:49:20 2018
@@ -204,7 +204,7 @@ class CheckVarsEscapingDeclContext final
 
   void markAsEscaped(const ValueDecl *VD) {
 // Do not globalize declare target variables.
-if (isDeclareTargetDeclaration(VD))
+if (!isa(VD) || isDeclareTargetDeclaration(VD))
   return;
 VD = cast(VD->getCanonicalDecl());
 // Variables captured by value must be globalized.

Modified: cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp?rev=337191&r1=337190&r2=337191&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Mon Jul 16 09:49:20 2018
@@ -9,12 +9,12 @@
 #define HEADER
 
 // Check that the execution mode of all 6 target regions is set to Generic 
Mode.
-// CHECK-DAG: {{@__omp_offloading_.+l102}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l179}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l289}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l326}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l344}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l309}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l103}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l180}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l290}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l328}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l346}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l311}}_exec_mode = weak constant i8 1
 
 __thread int id;
 
@@ -24,6 +24,7 @@ template
 struct TT{
   tx X;
   ty Y;
+  tx &operator[](int i) { return X; }
 };
 
 int foo(int n) {
@@ -35,7 +36,7 @@ int foo(int n) {
   double cn[5][n];
   TT d;
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l102}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l103}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -66,7 +67,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l102]]()
+  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l103]]()
   // CHECK-DAG: [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
   // CHECK-DAG: [[NTH:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
   // CHECK-DAG: [[WS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
@@ -108,7 +109,7 @@ int foo(int n) {
   {
   }
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l179}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l180}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -139,7 +140,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void 
[[T2:@__omp_offloading_.+foo.+l179]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]], 
i[[SZ:32|64]] [[ID:%[a-zA-Z_]+]])
+  // CHECK: define {{.*}}void 
[[T2:@__omp_offloading_.+foo.+l180]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]], 
i[[SZ:32|64]] [[ID:%[a-zA-Z_]+]])
   // CHECK: [[AA_ADDR:%.+]] = alloca i[[SZ]],
   // CHECK: store i[[SZ]] [[ARG1]], i[[SZ]]* [[AA_ADDR]],
   // CHECK: [[AA_CADDR:%.+]] = bitcast i[[SZ]]* [[AA_ADDR]] to i16*
@@ -182,7 +183,7 @@ int foo(int n) {
 id = aa;
   }
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l289}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l290}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -213,7 +214,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T3:@__omp_offloading_.+foo.+l289]](i[[SZ]]
+  // CH

[PATCH] D49058: [analyzer] Move DanglingInternalBufferChecker out of alpha

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

In https://reviews.llvm.org/D49058#1159533, @george.karpenkov wrote:

> @rnkovacs Do you have evaluation statistics handy for this checker? How many 
> bugs it finds, on which projects? How many of those are real bugs?


In its present form, it does not produce many reports, so it might be worth 
waiting for patches like https://reviews.llvm.org/D49360 and 
https://reviews.llvm.org/D49361 that add more functionality.
A recent run on LibreOffice showed one report by this checker, and it seems 
like a real bug (in an external package called GPGME).
Can be seen here 
,
 or if it does not work, line 135 here 
 
(quite a one-liner).


https://reviews.llvm.org/D49058



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:372
+
+auto __len = __last - __p;
+if (__value != 0 || !__len)

mclow.lists wrote:
> Are you missing an edge case here? What happens if `__last == __first && 
> __value == 0`?
> 
Nevermind. Apparently I can't look two lines down.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

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

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+  const LocationContext *LC = Context.getLocationContext();
+  while ((LC = LC->getParent())) {
+

Szelethus wrote:
> NoQ wrote:
> > george.karpenkov wrote:
> > > nit: could we have `while (LC)` followed by `LC = LC->getParent()` ? Do 
> > > you intentionally skip the first location context?
> > I guess the predicate we're checking is trivially true for the current 
> > location context.
> Completely missed this inline, sorry!
> 
> As @NoQ said, since this checker only fires after a constructor call, the 
> first location context will surely be that, and I'm only checking whether the 
> current ctor was called by another.
I'm actually not sure how you imagined that. The loop condition is `LC`, but if 
I assign it to its parent right after that, I won't be sure that it's non-null. 
The reason why I placed it there is that early exits could restart the loop at 
"random" places.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+  continue;
+
+// If the CurrentObject is a field of OtherObject, it will be analyzed
+// during the analysis of OtherObject.

NoQ wrote:
> It seems safer to look at `CXXConstructExpr::getConstructionKind()`.
> 
> Taking a `LazyCompoundVal` and converting it back to a region is definitely a 
> bad idea because the region within `LazyCompoundVal` is completely immaterial 
> and carries no meaning for the value represented by this `SVal`; it's not 
> necessarily the region you're looking for.
Looking at the singleton test case, I think I need to get that region somehow, 
and I'm not too sure what you meant under using 
`CXXConstructExpr::getConstructionKind()`. One thing I could do, is similar to 
how `getObjectVal` works:
```
  Loc Tmp = Context.getSValBuilder().getCXXThis(OtherCtor->getParent(),
Context.getStackFrame());

  auto OtherThis = Context.getState()->getSVal(Tmp).castAs();

  OtherThis.getRegion(); // Hooray!
```

I have tested it, and it works wonders. Is this a "safe-to-use" region?


https://reviews.llvm.org/D48436



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

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet, 
baloghadamsoftware, whisperity.

Repository:
  rC Clang

https://reviews.llvm.org/D49387

Files:
  include/clang/StaticAnalyzer/Core/Checker.h
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  lib/StaticAnalyzer/Core/CheckerManager.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2297,9 +2297,9 @@
 
 // Notify checkers.
 for (const auto I : AfterRemovedDead)
-  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this);
+  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this, RS);
   } else {
-getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this);
+getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
   }
 
   Engine.enqueueEndOfFunction(Dst, RS);
Index: lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -439,7 +439,8 @@
 void CheckerManager::runCheckersForEndFunction(NodeBuilderContext &BC,
ExplodedNodeSet &Dst,
ExplodedNode *Pred,
-   ExprEngine &Eng) {
+   ExprEngine &Eng,
+   const ReturnStmt *RS) {
   // We define the builder outside of the loop bacause if at least one checkers
   // creates a sucsessor for Pred, we do not need to generate an
   // autotransition for it.
@@ -449,7 +450,7 @@
   Pred->getLocationContext(),
   checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
-checkFn(C);
+checkFn(RS, C);
   }
 }
 
Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -48,7 +48,7 @@
   DefaultBool IsPureOnly;
 
   void checkBeginFunction(CheckerContext &C) const;
-  void checkEndFunction(CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
 private:
@@ -167,7 +167,8 @@
 }
 
 // The EndFunction callback when leave a constructor or a destructor.
-void VirtualCallChecker::checkEndFunction(CheckerContext &C) const {
+void VirtualCallChecker::checkEndFunction(const ReturnStmt *RS,
+  CheckerContext &C) const {
   registerCtorDtorCallInState(false, C);
 }
 
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -47,7 +47,7 @@
 
   UninitializedObjectChecker()
   : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {}
-  void checkEndFunction(CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 };
 
 /// Represents a field chain. A field chain is a vector of fields where the
@@ -241,7 +241,7 @@
 //===--===//
 
 void UninitializedObjectChecker::checkEndFunction(
-CheckerContext &Context) const {
+const ReturnStmt *RS, CheckerContext &Context) const {
 
   const auto *CtorDecl = dyn_cast_or_null(
   Context.getLocationContext()->getDecl());
Index: lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
+++ lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
@@ -30,7 +30,7 @@
 public:
   void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
   void checkBeginFunction(CheckerContext &C) const;
-  void checkEndFunction(CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C)

[PATCH] D48862: [OpenEmbedded] Fix lib paths for OpenEmbedded targets

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping 2 for reviews please.


https://reviews.llvm.org/D48862



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

[PATCH] D49356: [clang-tidy: modernize] Fix modernize-use-equals-default with {} brackets list initialization: patch

[Idriss via Phabricator via cfe-commits]

IdrissRio updated this revision to Diff 155715.
IdrissRio added a comment.

Fixed the formatting error with clang-format


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49356

Files:
  clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  test/clang-tidy/modernize-use-equals-default-copy.cpp


Index: test/clang-tidy/modernize-use-equals-default-copy.cpp
===
--- test/clang-tidy/modernize-use-equals-default-copy.cpp
+++ test/clang-tidy/modernize-use-equals-default-copy.cpp
@@ -497,3 +497,11 @@
 STRUCT_WITH_COPY_ASSIGN(unsigned char, Hex8CopyAssign)
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a 
trivial copy-assignment operator
 // CHECK-MESSAGES: :[[@LINE-9]]:40: note:
+
+// Use of braces
+struct UOB{
+  UOB(const UOB &Other):j{Other.j}{}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' to define a 
trivial copy constructor [modernize-use-equals-default]
+  // CHECK-FIXES: UOB(const UOB &Other)= default;
+  int j;
+};
Index: clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -97,6 +97,7 @@
 isMemberInitializer(), forField(equalsNode(Field)),
 withInitializer(anyOf(
 AccessToFieldInParam,
+initListExpr(has(AccessToFieldInParam)),
 cxxConstructExpr(allOf(
 
hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
 argumentCountIs(1),


Index: test/clang-tidy/modernize-use-equals-default-copy.cpp
===
--- test/clang-tidy/modernize-use-equals-default-copy.cpp
+++ test/clang-tidy/modernize-use-equals-default-copy.cpp
@@ -497,3 +497,11 @@
 STRUCT_WITH_COPY_ASSIGN(unsigned char, Hex8CopyAssign)
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial copy-assignment operator
 // CHECK-MESSAGES: :[[@LINE-9]]:40: note:
+
+// Use of braces
+struct UOB{
+  UOB(const UOB &Other):j{Other.j}{}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' to define a trivial copy constructor [modernize-use-equals-default]
+  // CHECK-FIXES: UOB(const UOB &Other)= default;
+  int j;
+};
Index: clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -97,6 +97,7 @@
 isMemberInitializer(), forField(equalsNode(Field)),
 withInitializer(anyOf(
 AccessToFieldInParam,
+initListExpr(has(AccessToFieldInParam)),
 cxxConstructExpr(allOf(
 hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
 argumentCountIs(1),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337196 - [OPENMP] Fix syntactic errors in error messages.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 11:12:18 2018
New Revision: 337196

URL: http://llvm.org/viewvc/llvm-project?rev=337196&view=rev
Log:
[OPENMP] Fix syntactic errors in error messages.

Fixed spelling of the offloading error messages.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/target_messages.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=337196&r1=337195&r2=337196&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Jul 16 11:12:18 2018
@@ -3970,7 +3970,7 @@ void CGOpenMPRuntime::createOffloadEntri
   if (!CE->getID() || !CE->getAddress()) {
 unsigned DiagID = CGM.getDiags().getCustomDiagID(
 DiagnosticsEngine::Error,
-"Offloading entry for target region is incorect: either the "
+"Offloading entry for target region is incorrect: either the "
 "address or the ID is invalid.");
 CGM.getDiags().Report(DiagID);
 continue;
@@ -3983,7 +3983,7 @@ void CGOpenMPRuntime::createOffloadEntri
   if (!CE->getAddress()) {
 unsigned DiagID = CGM.getDiags().getCustomDiagID(
 DiagnosticsEngine::Error,
-"Offloading entry for declare target varible is inccorect: the "
+"Offloading entry for declare target variable is incorrect: the "
 "address is invalid.");
 CGM.getDiags().Report(DiagID);
 continue;

Modified: cfe/trunk/test/OpenMP/target_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_messages.cpp?rev=337196&r1=337195&r2=337196&view=diff
==
--- cfe/trunk/test/OpenMP/target_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_messages.cpp Mon Jul 16 11:12:18 2018
@@ -14,7 +14,7 @@
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc 
-DREGION_HOST
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1 | FileCheck 
%s --check-prefix NO-REGION
-// NO-REGION: Offloading entry for target region is incorect: either the 
address or the ID is invalid.
+// NO-REGION: Offloading entry for target region is incorrect: either the 
address or the ID is invalid.
 
 #if defined(REGION_HOST) || defined(REGION_DEVICE)
 void foo() {


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

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/Sema/conditional-expr.c:20
   vp = 0 ? (double *)0 : (void *)0;
-  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double *'}}
+  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double * _Nullable'}}
 

ahatanak wrote:
> jordan_rose wrote:
> > This seems like an unfortunate change to make, since most people do not 
> > bother with nullability.
> Yes, this is unfortunate, but I'm not sure what's the right way to avoid 
> printing nullability specifiers in the diagnostic message. Do you have any 
> suggestions?
It looks like I can use PrintingPolicy to print the nullability specifier only 
when needed.

I think it's also possible to add a flag to Expr that indicates the Expr is 
possibly null. For example, when an Expr is an IntegerLiteral of value 0, or a 
CastExpr or a ConditionalOperator has a subexpression whose flag is set. This 
could be a better solution than the current solution in this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D22391



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

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

[Jonas Hahnfeld via cfe-commits]

[ Moving discussion from https://reviews.llvm.org/D49386 to the relevant 
comment on cfe-commits, CC'ing Hal who commented on the original issue ]


Is this change really a good idea? It always requires libatomic for all 
OpenMP applications, even if there is no 'omp atomic' directive or all 
of them can be lowered to atomic instructions that don't require a 
runtime library. I'd argue that it's a larger restriction than the 
problem it solves.
Per https://clang.llvm.org/docs/Toolchain.html#libatomic-gnu the user is 
expected to manually link -latomic whenever Clang can't lower atomic 
instructions - including C11 atomics and C++ atomics. In my opinion 
OpenMP is just another abstraction that doesn't require a special 
treatment.


Thoughts?
Jonas

On 2018-07-06 23:13, Alexey Bataev via cfe-commits wrote:

Author: abataev
Date: Fri Jul  6 14:13:41 2018
New Revision: 336467

URL: http://llvm.org/viewvc/llvm-project?rev=336467&view=rev
Log:
[OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

On Linux atomic constructs in OpenMP require libatomic library. Patch
links libatomic when -fopenmp is used.

Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/OpenMP/linking.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=336467&r1=336466&r2=336467&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Jul  6 14:13:41 2018
@@ -479,6 +479,7 @@ void tools::gnutools::Linker::ConstructJ

   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
+  bool WantAtomic = false;

   // FIXME: Only pass GompNeedsRT = true for platforms with 
libgomp that
   // require librt. Most modern Linux platforms do, but some may 
not.

@@ -487,13 +488,16 @@ void tools::gnutools::Linker::ConstructJ
/* GompNeedsRT= */ true))
 // OpenMP runtimes implies pthreads when using the GNU 
toolchain.

 // FIXME: Does this really make sense for all GNU toolchains?
-WantPthread = true;
+WantAtomic = WantPthread = true;

   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);

   if (WantPthread && !isAndroid)
 CmdArgs.push_back("-lpthread");

+  if (WantAtomic)
+CmdArgs.push_back("-latomic");
+
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("--wrap=pthread_create");


Modified: cfe/trunk/test/OpenMP/linking.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/linking.c?rev=336467&r1=336466&r2=336467&view=diff
==
--- cfe/trunk/test/OpenMP/linking.c (original)
+++ cfe/trunk/test/OpenMP/linking.c Fri Jul  6 14:13:41 2018
@@ -8,14 +8,14 @@
 // RUN:   | FileCheck --check-prefix=CHECK-LD-32 %s
 // CHECK-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-32: "-lpthread" "-lc"
+// CHECK-LD-32: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-64 %s
 // CHECK-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-64: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-64: "-lpthread" "-lc"
+// CHECK-LD-64: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libgomp -target i386-unknown-linux 
-rtlib=platform \

@@ -27,7 +27,7 @@
 // SIMD-ONLY2-NOT: liomp
 // CHECK-GOMP-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-GOMP-LD-32: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-32: "-lpthread" "-lc"
+// CHECK-GOMP-LD-32: "-lpthread" "-latomic" "-lc"

 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1
-fopenmp-simd -target i386-unknown-linux -rtlib=platform | FileCheck
--check-prefix SIMD-ONLY2 %s
 // SIMD-ONLY2-NOT: lgomp
@@ -39,21 +39,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-GOMP-LD-64 %s
 // CHECK-GOMP-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-GOMP-LD-64: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-64: "-lpthread" "-lc"
+// CHECK-GOMP-LD-64: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target i386-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-IOMP5-LD-32 %s
 // CHECK-IOMP5-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-IOMP5-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-IOMP5-LD-32: "-lpthread" "-lc"
+// CHECK-IOMP5-LD-32: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-IOMP5-LD-64 %s
 // CHECK-IOMP5-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-IOMP5-LD-64: "-l[[DE

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: test/Sema/conditional-expr.c:20
   vp = 0 ? (double *)0 : (void *)0;
-  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double *'}}
+  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double * _Nullable'}}
 

ahatanak wrote:
> ahatanak wrote:
> > jordan_rose wrote:
> > > This seems like an unfortunate change to make, since most people do not 
> > > bother with nullability.
> > Yes, this is unfortunate, but I'm not sure what's the right way to avoid 
> > printing nullability specifiers in the diagnostic message. Do you have any 
> > suggestions?
> It looks like I can use PrintingPolicy to print the nullability specifier 
> only when needed.
> 
> I think it's also possible to add a flag to Expr that indicates the Expr is 
> possibly null. For example, when an Expr is an IntegerLiteral of value 0, or 
> a CastExpr or a ConditionalOperator has a subexpression whose flag is set. 
> This could be a better solution than the current solution in this patch.
Whew. I hadn't had the chance to look at PrintingPolicy and was afraid you'd 
have to add a new flag to diagnostics or something to specify whether 
nullability was relevant.

An additional flag on Expr seems like overkill to me, given that 
`isNullPointerConstant` already exists. But I don't work in Clang these days, 
so maybe I'm wrong and it is something worth caching. 


Repository:
  rC Clang

https://reviews.llvm.org/D22391



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

[PATCH] D49389: [clangd] Transport abstraction WIP

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov.

Ideas about abstracting JSON transport away to allow XPC and improve layering.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49389

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

Index: clangd/Transport.h
===
--- /dev/null
+++ clangd/Transport.h
@@ -0,0 +1,90 @@
+//===--- Transport.h - sending and receiving LSP messages ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// The language server protocol is usually implemented by writing messages as
+// JSON-RPC over the stdin/stdout of a subprocess. However other communications
+// mechanisms are possible, such as XPC on mac (see xpc/ directory).
+//
+// The Transport interface allows the mechanism to be replaced, and the JSONRPC
+// Transport is the standard implementation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+
+// A transport is responsible for maintaining the connection to a client
+// application, and reading/writing structured messages to it.
+//
+// Transports have limited thread safety requirements:
+//  - messages will not be sent concurrently
+//  - messages MAY be sent while loop() is reading, or its callback is active
+class Transport {
+public:
+  virtual ~Transport() = default;
+
+  // Called by Clangd to send messages to the client.
+  // (Because JSON and XPC are so similar, these are concrete and delegate to
+  // sendMessage. We could change this to support more diverse transports).
+  void notify(llvm::StringRef Method, llvm::json::Value Params);
+  void call(llvm::StringRef Method, llvm::json::Value Params,
+llvm::json::Value ID);
+  void reply(llvm::json::Value ID, llvm::Expected Result);
+
+  // Implemented by Clangd to handle incoming messages. (See loop() below).
+  class MessageHandler {
+  public:
+virtual ~MessageHandler() = 0;
+virtual bool notify(llvm::StringRef Method, llvm::json::Value ) = 0;
+virtual bool call(llvm::StringRef Method, llvm::json::Value Params,
+  llvm::json::Value ID) = 0;
+virtual bool reply(llvm::json::Value ID,
+   llvm::Expected Result) = 0;
+  };
+  // Called by Clangd to receive messages from the client.
+  // The transport should in turn invoke the handler to process messages.
+  // If handler returns true, the transport should immedately return success.
+  // Otherwise, it returns an error when the transport becomes unusable.
+  // (Because JSON and XPC are so similar, they share handleMessage()).
+  virtual llvm::Error loop(MessageHandler &) = 0;
+
+protected:
+  // Common implementation for notify(), call(), and reply().
+  virtual void sendMessage(llvm::json::Value) = 0;
+  // Delegates to notify(), call(), and reply().
+  bool handleMessage(llvm::json::Value, MessageHandler&);
+};
+
+// Controls the way JSON-RPC messages are encoded (both input and output).
+enum JSONStreamStyle {
+  // Encoding per the LSP specification, with mandatory Content-Length header.
+  Standard,
+  // Messages are delimited by a '---' line. Comment lines start with #.
+  Delimited
+};
+
+// Returns a Transport that speaks JSON-RPC over a pair of streams.
+// The input stream must be opened in binary mode.
+std::unique_ptr
+newJSONTransport(std::FILE *In, llvm::raw_ostream &Out,
+ JSONStreamStyle = JSONStreamStyle::Standard);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
+
+
Index: clangd/Transport.cpp
===
--- /dev/null
+++ clangd/Transport.cpp
@@ -0,0 +1,225 @@
+//===--- Transport.cpp - sending and receiving LSP messages -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// The JSON-RPC transport is implemented here.
+// The alternative Mac-only XPC transport is in the xpc/ directory.
+//
+//===--===//
+#include "Transport.h"
+#include "Logger.h"
+#include "Protocol.h"
+#include "llvm/Support/Errno.h"
+
+using namespace llvm;
+namespace clang {
+namespac

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hi Jan,

Thanks for putting this together, and apologies - this is one of the places 
where we don't have nice abstractions/layering, so adding XPC was harder than 
it should be.

As I mentioned on the other review, I think maybe this patch isn't invasive 
enough, we could do a more thorough job of making the json transport a 
standalone, first-class thing. This makes it easier to swap out for XPC but 
also would improve the layering in the core clangd classes.

I put together a sketch of a `Transport` interface in 
https://reviews.llvm.org/D49389 (that patch is extremely unfinished and won't 
compile, but the idea might be interesting).
The idea is that we should be able to implement that class for XPC and then it 
should just drop-in as a replacement for the JSON one.
I've indicated there where XPC and JSON implementation can share code (assuming 
you wouldn't rather "tighten up" the protocol and eliminate some JSON-RPC 
noise).

If you like this direction I'm happy for you to pick the bits you like, or I 
can finish it as a refactoring and we can make sure it works for XPC.
If not, that's fine too - happy to look at other ways to reduce duplication 
between them.




Comment at: DispatcherCommon.h:38
+  // Return a context that's aware of the enclosing request, identified by 
Span.
+  static Context stash(const trace::Span &Span);
+

I think I wrote this bit... it's a hack that I'm not sure deserves to be 
visible in a header (it was bad enough in JSONRPCDispatcher.cpp!)

Rather than exposing it so we can use it twice, I'd hope we can manage to keep 
it hidden so that JSON and XPC can share it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48559



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

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great, thanks!




Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:501
 
-  void _registerForBeginFunction(CheckEndFunctionFunc checkfn);
+  void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);

:D


Repository:
  rC Clang

https://reviews.llvm.org/D49387



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D49265#1163740, @dblaikie wrote:

> Any chance this can/should be unit tested? (also, in general (though might
>  not matter in this instance), symmetric operators like == should be
>  implemented as non-members (though they can still be friends and if they
>  are, can be defined inline in the class definition as a member could be),
>  so any implicit conversions apply equally to the LHS as the RHS of the
>  expression)


Of course, I'm on it.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Sorry about missing this. Looks great, thanks for doing this. This was a pretty 
big omission, glad to see it's done now :-)

I always imagined we might reuse the precompiled preamble stuff 
(Lexer::ComputePreamble() etc), but it's pretty clear as-is.


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: cfe-commits, sunfish, jgravelle-google, sbc100.

Add support for atomic.wait / atomic.wake builtins based on the Wasm
thread proposal.


Repository:
  rC Clang

https://reviews.llvm.org/D49396

Files:
  include/clang/Basic/BuiltinsWebAssembly.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins-wasm.c


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,20 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i32);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i64: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i64);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wake: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *WakeCount = EmitScalarExpr(E->getArg(1));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wake);
+return Builder.CreateCall(Callee, {Addr, WakeCount});
+  }
 
   default:
 return nullptr;
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -34,4 +34,9 @@
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
 BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uiv*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "Uiv*LLiLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wake, "ULLiv*LLi", "n")
+
 #undef BUILTIN


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,20 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExp

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Also, were you planning on also adding support for the (filename-less version 
of) hdrstop pragma? After this change, that should probably be fairly 
straightforward.


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Heejin Ahn via Phabricator via cfe-commits]

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

- test fix


Repository:
  rC Clang

https://reviews.llvm.org/D49396

Files:
  include/clang/Basic/BuiltinsWebAssembly.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins-wasm.c


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i32);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i64: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i64);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wake: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *WakeCount = EmitScalarExpr(E->getArg(1));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wake);
+return Builder.CreateCall(Callee, {Addr, WakeCount});
+  }
 
   default:
 return nullptr;
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -34,4 +34,9 @@
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
 BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uiv*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "Uiv*LLiLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wake, "ULLiv*LLi", "n")
+
 #undef BUILTIN


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = Emi

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

And finally (sorry about all the mails), this should probably be mentioned in 
the release notes (docs/ReleaseNotes.rst in the clang repo) since it's a 
notable new feature :-)


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Probably compiler-rt should be fixed so it doesn't need libm, rather than 
fixing clang to add -lm.  (All the functions it currently uses can be 
implemented with simple bit manipulation.)


Repository:
  rC Clang

https://reviews.llvm.org/D49330



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

[PATCH] D49338: Implement - P0122R7

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added inline comments.



Comment at: test/std/containers/views/span.comparison/op.eq.pass.cpp:23
+#include 
+#include 
+

The comparison tests appear to be unnecessarily including ``.



Comment at: test/std/containers/views/span.cons/assign.pass.cpp:25
+{
+static_assert(noexcept(std::declval() = rhs), "");
+lhs = rhs;

Technically need `` for declval.



Comment at: test/std/containers/views/span.cons/assign.pass.cpp:73
+
+//  No for loops in constexpr land :-(
+static_assert(doAssign(spans[0], spans[0]), "");

span is C++20, so you can use C++14 extended constexpr's for-loops. Just write 
a helper function.



Comment at: test/std/containers/views/span.cons/container.fail.cpp:44
+
+constexpr T const *getV() const {return &v_;} // for checking
+T v_;

This file is inconsistent about left vs right const (see line immediately 
above).



Comment at: test/std/containers/views/span.cons/deduct.pass.cpp:45
+using S = decltype(s);
+static_assert(std::is_same_v>, "");
+assert((std::equal(std::begin(arr), std::end(arr), s.begin(), s.end(;

Technically need `` for is_same_v.



Comment at: test/std/containers/views/span.cons/default.pass.cpp:61
+std::span s2;
+return
+assert(s1.data() == nullptr && s1.size() == 0);

This return appears to be spurious.



Comment at: test/std/containers/views/span.cons/span.pass.cpp:96
+std::spans2(s1); // static -> dynamic
+return
+assert(s1.data() == nullptr && s1.size() == 0);

Spurious return occurs in multiple files.



Comment at: test/std/containers/views/span.iterators/begin.pass.cpp:31
+{
+ret &= ( b ==  s.end());
+ret &= (cb == s.cend());

Using bitwise &= on a bool isn't very cromulent.



Comment at: test/std/containers/views/span.iterators/rbegin.pass.cpp:115
+std::string s;
+testRuntimeSpan(std::span(&s, (std::ptrdiff_t) 0));
+testRuntimeSpan(std::span(&s, 1));

static_cast would be nicer than C cast.



Comment at: test/std/containers/views/span.objectrep/as_bytes.pass.cpp:31
+{
+static_assert(noexcept(std::as_bytes(sp)));
+

This is a C++17 terse static assert unlike your others. Should you consistently 
use this form, or the other one?



Comment at: 
test/std/containers/views/span.objectrep/as_writeable_bytes.pass.cpp:42
+
+assert((void *) spBytes.data() == (void *) sp.data());
+assert(spBytes.size() == sp.size_bytes());

static_cast?



Comment at: test/std/containers/views/span.sub/last.pass.cpp:44
+ && s1.size() == s2.size()
+ && std::equal(s1.begin(), s1.end(), sp.end() - Count);
+}

Need ``.



Comment at: test/std/containers/views/span.sub/subspan.pass.cpp:45
+ && s1.size() == s2.size()
+ && std::equal(s1.begin(), s1.end(), sp.begin() + Offset);
+}

Also need ``, presumably occurs in more files.



Comment at: test/std/containers/views/types.pass.cpp:41
+{
+typedef std::iterator_traits ItT;
+

Need ``.


https://reviews.llvm.org/D49338



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

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: zturner.

Wmsvc-not-found was added in r297851 to help diagnose why link.exe can't be 
executed. However, it's emitted even when using -fuse-ld=lld, and in cross 
builds there's no way to get rid of the warning other than disabling it.

Instead, emit it when we look up link.exe. That way, when passing -fuse-ld=lld 
it will never be printed.

(We might want to eventually default to lld one day, at least when running on a 
non-Win host, but that's for another day.)

Fixes PR38016.


https://reviews.llvm.org/D49398

Files:
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MSVC.h


Index: lib/Driver/ToolChains/MSVC.h
===
--- lib/Driver/ToolChains/MSVC.h
+++ lib/Driver/ToolChains/MSVC.h
@@ -123,6 +123,8 @@
 
   void printVerboseInfo(raw_ostream &OS) const override;
 
+  bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -464,6 +464,9 @@
 Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
@@ -684,8 +687,6 @@
 }
 
 Tool *MSVCToolChain::buildLinker() const {
-  if (VCToolChainPath.empty())
-getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
   return new tools::visualstudio::Linker(*this);
 }
 


Index: lib/Driver/ToolChains/MSVC.h
===
--- lib/Driver/ToolChains/MSVC.h
+++ lib/Driver/ToolChains/MSVC.h
@@ -123,6 +123,8 @@
 
   void printVerboseInfo(raw_ostream &OS) const override;
 
+  bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -464,6 +464,9 @@
 Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
@@ -684,8 +687,6 @@
 }
 
 Tool *MSVCToolChain::buildLinker() const {
-  if (VCToolChainPath.empty())
-getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
   return new tools::visualstudio::Linker(*this);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r337205 - Fix PR38160 - init_priority attribute not supported by GCC on Apple.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Jul 16 13:01:59 2018
New Revision: 337205

URL: http://llvm.org/viewvc/llvm-project?rev=337205&view=rev
Log:
Fix PR38160 - init_priority attribute not supported by GCC on Apple.

This patch guards the use of __attribute__((init_priority(101)))
within memory_resource.cpp when building with compilers that don't
support it. Specifically GCC on Apple platforms, and MSVC.

Modified:
libcxx/trunk/src/experimental/memory_resource.cpp

Modified: libcxx/trunk/src/experimental/memory_resource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/memory_resource.cpp?rev=337205&r1=337204&r2=337205&view=diff
==
--- libcxx/trunk/src/experimental/memory_resource.cpp (original)
+++ libcxx/trunk/src/experimental/memory_resource.cpp Mon Jul 16 13:01:59 2018
@@ -68,12 +68,23 @@ union ResourceInitHelper {
   _LIBCPP_CONSTEXPR_AFTER_CXX11 ResourceInitHelper() : resources() {}
   ~ResourceInitHelper() {}
 };
+
+// Detect if the init_priority attribute is supported.
+#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
+  || defined(_LIBCPP_COMPILER_MSVC)
+// GCC on Apple doesn't support the init priority attribute,
+// and MSVC doesn't support any GCC attributes.
+# define _LIBCPP_INIT_PRIORITY_MAX
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#endif
+
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
 _LIBCPP_SAFE_STATIC
 #endif
-ResourceInitHelper res_init  __attribute__((init_priority (101)));
+ResourceInitHelper res_init _LIBCPP_INIT_PRIORITY_MAX;
 
 } // end namespace
 


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

r337207 - [OPENMP] Fix checks for declare target link entries.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 13:05:25 2018
New Revision: 337207

URL: http://llvm.org/viewvc/llvm-project?rev=337207&view=rev
Log:
[OPENMP] Fix checks for declare target link entries.

If the declare target link entries are created but not used, the
compiler will produce an error message. Patch improves handling of such
situations + improves checks for possibly lost declare target variables.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=337207&r1=337206&r2=337207&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Jul 16 13:05:25 2018
@@ -3980,16 +3980,39 @@ void CGOpenMPRuntime::createOffloadEntri
 } else if (const auto *CE =
dyn_cast(E)) {
-  if (!CE->getAddress()) {
-unsigned DiagID = CGM.getDiags().getCustomDiagID(
-DiagnosticsEngine::Error,
-"Offloading entry for declare target variable is incorrect: the "
-"address is invalid.");
-CGM.getDiags().Report(DiagID);
-continue;
+  OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryKind Flags =
+  
static_cast(
+  CE->getFlags());
+  switch (Flags) {
+  case OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo: {
+if (!CE->getAddress()) {
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "Offloading entry for declare target variable is incorrect: the "
+  "address is invalid.");
+  CGM.getDiags().Report(DiagID);
+  continue;
+}
+break;
+  }
+  case OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryLink:
+assert(((CGM.getLangOpts().OpenMPIsDevice && !CE->getAddress()) ||
+(!CGM.getLangOpts().OpenMPIsDevice && CE->getAddress())) &&
+   "Declaret target link address is set.");
+if (CGM.getLangOpts().OpenMPIsDevice)
+  continue;
+if (!CE->getAddress()) {
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "Offloading entry for declare target variable is incorrect: the "
+  "address is invalid.");
+  CGM.getDiags().Report(DiagID);
+  continue;
+}
+break;
   }
   createOffloadEntry(CE->getAddress(), CE->getAddress(),
- CE->getVarSize().getQuantity(), CE->getFlags(),
+ CE->getVarSize().getQuantity(), Flags,
  CE->getLinkage());
 } else {
   llvm_unreachable("Unsupported entry kind.");
@@ -7889,14 +7912,15 @@ void CGOpenMPRuntime::registerTargetGlob
   Linkage = CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false);
   break;
 case OMPDeclareTargetDeclAttr::MT_Link:
-  // Map type 'to' because we do not map the original variable but the
-  // reference.
-  Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo;
-  if (!CGM.getLangOpts().OpenMPIsDevice) {
+  Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryLink;
+  if (CGM.getLangOpts().OpenMPIsDevice) {
+VarName = Addr->getName();
+Addr = nullptr;
+  } else {
+VarName = getAddrOfDeclareTargetLink(VD).getName();
 Addr =
 cast(getAddrOfDeclareTargetLink(VD).getPointer());
   }
-  VarName = Addr->getName();
   VarSize = CGM.getPointerSize();
   Linkage = llvm::GlobalValue::WeakAnyLinkage;
   break;

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=337207&r1=337206&r2=337207&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Mon Jul 16 13:05:25 2018
@@ -522,6 +522,8 @@ private:
 enum OMPTargetGlobalVarEntryKind : uint32_t {
   /// Mark the entry as a to declare target.
   OMPTargetGlobalVarEntryTo = 0x0,
+  /// Mark the entry as a to declare target link.
+  OMPTargetGlobalVarEntryLink = 0x1,
 };
 
 /// Device global variable entries info.

Modified: cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp?rev=337207&r1=337206&r2=337207&view=diff
==
--- cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp Mon Jul 16 13:0

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

[Zachary Turner via Phabricator via cfe-commits]

zturner added inline comments.



Comment at: lib/Driver/ToolChains/MSVC.cpp:467-468
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+

It looks like it's possible for this warning to be emitted even when 
`FindVisualStudioExecutable` succeeds (after looking in the install location it 
checks `PATH`).  Would it make more sense to put this check after the call to 
`FindVisualStudioExecutable`, but only if it fails?


https://reviews.llvm.org/D49398



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

r337209 - [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and `isInstanceMessage` for ObjCMessageExpr

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:22:12 2018
New Revision: 337209

URL: http://llvm.org/viewvc/llvm-project?rev=337209&view=rev
Log:
[ASTMatchers] Introduce Objective-C matchers `hasReceiver` and 
`isInstanceMessage` for ObjCMessageExpr

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=337209&r1=337208&r2=337209&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Mon Jul 16 13:22:12 2018
@@ -3268,6 +3268,19 @@ represent an error condition in the tree
 
 
 
+MatcherObjCMessageExpr>isInstanceMessage
+Returns true when 
the Objective-C message is sent to an instance.
+
+Example
+matcher = objcMessagaeExpr(isInstanceMessage())
+matches
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+but not
+  [NSString stringWithFormat:@"format"]
+
+
+
 MatcherObjCMessageExpr>matchesSelectorstd::string 
RegExp
 Matches ObjC 
selectors whose name contains
 a substring matched by the given RegExp.
@@ -5875,6 +5888,18 @@ Example matches y in x(y)
 
 
 
+MatcherObjCMessageExpr>hasReceiverMatcherExpr> 
InnerMatcher
+Matches if the 
Objective-C message is sent to an instance,
+and the inner matcher matches on that instance.
+
+For example the method call in
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+is matched by
+objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+
+
+
 MatcherObjCMessageExpr>hasReceiverTypeMatcherQualType>
 InnerMatcher
 Matches on the 
receiver of an ObjectiveC Message expression.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=337209&r1=337208&r2=337209&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Jul 16 13:22:12 2018
@@ -2736,6 +2736,41 @@ AST_MATCHER_P(ObjCMessageExpr, hasReceiv
   return InnerMatcher.matches(TypeDecl, Finder, Builder);
 }
 
+/// Returns true when the Objective-C message is sent to an instance.
+///
+/// Example
+/// matcher = objcMessagaeExpr(isInstanceMessage())
+/// matches
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// but not
+/// \code
+///   [NSString stringWithFormat:@"format"];
+/// \endcode
+AST_MATCHER(ObjCMessageExpr, isInstanceMessage) {
+  return Node.isInstanceMessage();
+}
+
+/// Matches if the Objective-C message is sent to an instance,
+/// and the inner matcher matches on that instance.
+///
+/// For example the method call in
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// is matched by
+/// objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+AST_MATCHER_P(ObjCMessageExpr, hasReceiver, internal::Matcher,
+  InnerMatcher) {
+  const Expr *ReceiverNode = Node.getInstanceReceiver();
+  return (ReceiverNode != nullptr &&
+  InnerMatcher.matches(*ReceiverNode->IgnoreParenImpCasts(), Finder,
+   Builder));
+}
+
 /// Matches when BaseName == Selector.getAsString()
 ///
 ///  matcher = objCMessageExpr(hasSelector("loadHTMLString:baseURL:"));

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=337209&r1=337208&r2=337209&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Mon Jul 16 13:22:12 2018
@@ -283,6 +283,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
   REGISTER_MATCHER(hasRangeInit);
+  REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
@@ -349,6 +350,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isImplicit);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
+ 

[PATCH] D49333: [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and `isInstanceMessage` for ObjCMessageExpr

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337209: [ASTMatchers] Introduce Objective-C matchers 
`hasReceiver` and… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49333?vs=155734&id=155747#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49333

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

Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -283,6 +283,7 @@
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
   REGISTER_MATCHER(hasRangeInit);
+  REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
@@ -349,6 +350,7 @@
   REGISTER_MATCHER(isImplicit);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
+  REGISTER_MATCHER(isInstanceMessage);
   REGISTER_MATCHER(isInstantiated);
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isInteger);
Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -422,6 +422,35 @@
   EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument));
 }
 
+TEST(Matcher, HasReceiver) {
+  EXPECT_TRUE(matchesObjC(
+  "@interface NSString @end"
+  "void f(NSString *x) {"
+  "[x containsString]"
+  "}",
+  objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
+
+  EXPECT_FALSE(matchesObjC(
+  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "void f() { [NSString stringWithFormat]; }",
+  objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
+}
+
+TEST(Matcher, isInstanceMessage) {
+  EXPECT_TRUE(matchesObjC(
+  "@interface NSString @end"
+  "void f(NSString *x) {"
+  "[x containsString]"
+  "}",
+  objcMessageExpr(isInstanceMessage(;
+
+  EXPECT_FALSE(matchesObjC(
+  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "void f() { [NSString stringWithFormat]; }",
+  objcMessageExpr(isInstanceMessage(;
+
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
 declRefExpr(to(varDecl(hasName("y".bind("arg");
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -3268,6 +3268,19 @@
 
 
 
+MatcherObjCMessageExpr>isInstanceMessage
+Returns true when the Objective-C message is sent to an instance.
+
+Example
+matcher = objcMessagaeExpr(isInstanceMessage())
+matches
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+but not
+  [NSString stringWithFormat:@"format"]
+
+
+
 MatcherObjCMessageExpr>matchesSelectorstd::string RegExp
 Matches ObjC selectors whose name contains
 a substring matched by the given RegExp.
@@ -5875,6 +5888,18 @@
 
 
 
+MatcherObjCMessageExpr>hasReceiverMatcherExpr> InnerMatcher
+Matches if the Objective-C message is sent to an instance,
+and the inner matcher matches on that instance.
+
+For example the method call in
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+is matched by
+objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+
+
+
 MatcherObjCMessageExpr>hasReceiverTypeMatcherQualType> InnerMatcher
 Matches on the receiver of an ObjectiveC Message expression.
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2736,6 +2736,41 @@
   return InnerMatcher.matches(TypeDecl, Finder, Builder);
 }
 
+/// Returns true when the Objective-C message is sent to an instance.
+///
+/// Example
+/// matcher = objcMessagaeExpr(isInstanceMessage())
+/// matches
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// but not
+/// \code
+///   [NSString stringWithFormat:@"format"];
+/// \endcode
+AST_MATCHER(ObjCMessageExpr, isInstanceMessage) {
+  return Node.isInstanceMessage();
+}
+
+/// Matches if 

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

lichray wrote:
> mclow.lists wrote:
> > Same comment as above about `read` and `inner_product` - they need to be 
> > "ugly names"
> Unlike `traits` which is a template parameter name in the standard, `read` 
> and `inner_product` are function names in the standard, which means the users 
> cannot make a macro for them (and there is no guarantee about what name you 
> make **not** get by including certain headers), so we don't need to use ugly 
> names here, am I right?
I understand your reasoning, but I don't agree. 

Just last month, I had to rename a function in `vector` from `allocate` to 
`__vallocate` because it confused our "is this an allocator" detection. The 
function in question was private, so it shouldn't have mattered, but GCC has a 
bug where sometimes it partially ignores access restrictions in non-deduced 
contexts, and then throws a hard error when it comes back to a different 
context. The easiest workaround was to rename the function in `vector`.

Since then, I've been leery of public names that match others. This is pretty 
obscure, since it's in a private namespace, but I'd feel better if they were 
`__read` and `__inner_product`.



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

r337211 - [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation from Decl

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:32:32 2018
New Revision: 337211

URL: http://llvm.org/viewvc/llvm-project?rev=337211&view=rev
Log:
[analyzer] Provide a symmetric method for generating a PathDiagnosticLocation 
from Decl

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=337211&r1=337210&r2=337211&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Mon Jul 16 13:32:32 2018
@@ -216,6 +216,15 @@ public:
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager &SM);
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager &SM,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager &SM,


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

[PATCH] D49166: [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation from Decl

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337211: [analyzer] Provide a symmetric method for generating 
a PathDiagnosticLocation… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49166?vs=154917&id=155750#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49166

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h


Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -216,6 +216,15 @@
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager &SM);
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager &SM,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager &SM,


Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -216,6 +216,15 @@
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager &SM);
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager &SM,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager &SM,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337212 - [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is initialized to zero

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:32:57 2018
New Revision: 337212

URL: http://llvm.org/viewvc/llvm-project?rev=337212&view=rev
Log:
[analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is 
initialized to zero

Initializing a semaphore with a different constant most likely signals a 
different intent

rdar://41802552

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
cfe/trunk/test/Analysis/gcdantipatternchecker_test.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp?rev=337212&r1=337211&r2=337212&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp Mon Jul 16 
13:32:57 2018
@@ -93,7 +93,9 @@ static bool isTest(const Decl *D) {
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(

Modified: cfe/trunk/test/Analysis/gcdantipatternchecker_test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gcdantipatternchecker_test.m?rev=337212&r1=337211&r2=337212&view=diff
==
--- cfe/trunk/test/Analysis/gcdantipatternchecker_test.m (original)
+++ cfe/trunk/test/Analysis/gcdantipatternchecker_test.m Mon Jul 16 13:32:57 
2018
@@ -333,3 +333,13 @@ void dispatch_group_and_semaphore_use(My
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a 
callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+


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

[PATCH] D48911: [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is initialized to zero

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337212: [analyzer] Fix GCDAntipatternChecker to only fire 
when the semaphore is… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D48911

Files:
  lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  test/Analysis/gcdantipatternchecker_test.m


Index: test/Analysis/gcdantipatternchecker_test.m
===
--- test/Analysis/gcdantipatternchecker_test.m
+++ test/Analysis/gcdantipatternchecker_test.m
@@ -333,3 +333,13 @@
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a 
callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+
Index: lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -93,7 +93,9 @@
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(


Index: test/Analysis/gcdantipatternchecker_test.m
===
--- test/Analysis/gcdantipatternchecker_test.m
+++ test/Analysis/gcdantipatternchecker_test.m
@@ -333,3 +333,13 @@
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+
Index: lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -93,7 +93,9 @@
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337213 - [analyzer] Bugfix for an overly eager suppression for null pointer return from macros.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:33:25 2018
New Revision: 337213

URL: http://llvm.org/viewvc/llvm-project?rev=337213&view=rev
Log:
[analyzer] Bugfix for an overly eager suppression for null pointer return from 
macros.

Only suppress those cases where the null which came from the macro is
relevant to the bug, and was not overwritten in between.

rdar://41497323

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=337213&r1=337212&r2=337213&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Jul 16 
13:33:25 2018
@@ -228,6 +228,38 @@ static bool isFunctionMacroExpansion(Sou
   return EInfo.isFunctionMacroExpansion();
 }
 
+/// \return Whether \c RegionOfInterest was modified at \p N,
+/// where \p ReturnState is a state associated with the return
+/// from the current frame.
+static bool wasRegionOfInterestModifiedAt(
+const SubRegion *RegionOfInterest,
+const ExplodedNode *N,
+SVal ValueAfter) {
+  ProgramStateRef State = N->getState();
+  ProgramStateManager &Mgr = N->getState()->getStateManager();
+
+  if (!N->getLocationAs()
+  && !N->getLocationAs()
+  && !N->getLocationAs())
+return false;
+
+  // Writing into region of interest.
+  if (auto PS = N->getLocationAs())
+if (auto *BO = PS->getStmtAs())
+  if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
+N->getSVal(BO->getLHS()).getAsRegion()))
+return true;
+
+  // SVal after the state is possibly different.
+  SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
+  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, 
ValueAfter).isConstrainedTrue() &&
+  (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
+return true;
+
+  return false;
+}
+
+
 namespace {
 
 /// Put a diagnostic on return statement of all inlined functions
@@ -346,7 +378,7 @@ private:
   FramesModifyingCalculated.insert(
 N->getLocationContext()->getStackFrame());
 
-  if (wasRegionOfInterestModifiedAt(N, LastReturnState, ValueAtReturn)) {
+  if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) {
 const StackFrameContext *SCtx = N->getStackFrame();
 while (!SCtx->inTopFrame()) {
   auto p = FramesModifyingRegion.insert(SCtx);
@@ -365,33 +397,6 @@ private:
 } while (N);
   }
 
-  /// \return Whether \c RegionOfInterest was modified at \p N,
-  /// where \p ReturnState is a state associated with the return
-  /// from the current frame.
-  bool wasRegionOfInterestModifiedAt(const ExplodedNode *N,
- ProgramStateRef ReturnState,
- SVal ValueAtReturn) {
-if (!N->getLocationAs()
-&& !N->getLocationAs()
-&& !N->getLocationAs())
-  return false;
-
-// Writing into region of interest.
-if (auto PS = N->getLocationAs())
-  if (auto *BO = PS->getStmtAs())
-if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-
N->getSVal(BO->getLHS()).getAsRegion()))
-  return true;
-
-// SVal after the state is possibly different.
-SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() &&
-(!ValueAtN.isUndef() || !ValueAtReturn.isUndef()))
-  return true;
-
-return false;
-  }
-
   /// Get parameters associated with runtime definition in order
   /// to get the correct parameter name.
   ArrayRef getCallParameters(CallEventRef<> Call) {
@@ -524,25 +529,28 @@ private:
   }
 };
 
+/// Suppress null-pointer-dereference bugs where dereferenced null was returned
+/// the macro.
 class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor {
   const SubRegion *RegionOfInterest;
+  const SVal ValueAtDereference;
 
-public:
-  MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) 
{}
-
-  static void *getTag() {
-static int Tag = 0;
-return static_cast(&Tag);
-  }
+  // Do not invalidate the reports where the value was modified
+  // after it got assigned to from the macro.
+  bool WasModified = false;
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override {
-ID.AddPointer(getTag());
-  }
+public:
+  MacroNullReturnSuppressionVisitor(const SubRegion *R,
+const SVal V) : RegionOfInterest(R),
+ValueAtDereference(V) {}
 
   std::shared_ptr VisitNode(const Explod

[PATCH] D48856: [analyzer] Fix overly eager suppression of NPE when the value used is returned from a macro

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337213: [analyzer] Bugfix for an overly eager suppression 
for null pointer return from… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48856?vs=153815&id=155752#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48856

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/diagnostics/macro-null-return-suppression.cpp

Index: test/Analysis/diagnostics/macro-null-return-suppression.cpp
===
--- test/Analysis/diagnostics/macro-null-return-suppression.cpp
+++ test/Analysis/diagnostics/macro-null-return-suppression.cpp
@@ -43,3 +43,26 @@
 DEREF_IN_MACRO(0) // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{'p' initialized to a null}}
   // expected-note@-2{{Dereference of null pointer}}
+
+// Warning should not be suppressed if the null returned by the macro
+// is not related to the warning.
+#define RETURN_NULL() (0)
+extern int* returnFreshPointer();
+int noSuppressMacroUnrelated() {
+  int *x = RETURN_NULL();
+  x = returnFreshPointer();  // expected-note{{Value assigned to 'x'}}
+  if (x) {} // expected-note{{Taking false branch}}
+// expected-note@-1{{Assuming 'x' is null}}
+  return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
+
+// Value haven't changed by the assignment, but the null pointer
+// did not come from the macro.
+int noSuppressMacroUnrelatedOtherReason() {
+  int *x = RETURN_NULL();
+  x = returnFreshPointer();  
+  x = 0; // expected-note{{Null pointer value stored to 'x'}}
+  return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -228,6 +228,38 @@
   return EInfo.isFunctionMacroExpansion();
 }
 
+/// \return Whether \c RegionOfInterest was modified at \p N,
+/// where \p ReturnState is a state associated with the return
+/// from the current frame.
+static bool wasRegionOfInterestModifiedAt(
+const SubRegion *RegionOfInterest,
+const ExplodedNode *N,
+SVal ValueAfter) {
+  ProgramStateRef State = N->getState();
+  ProgramStateManager &Mgr = N->getState()->getStateManager();
+
+  if (!N->getLocationAs()
+  && !N->getLocationAs()
+  && !N->getLocationAs())
+return false;
+
+  // Writing into region of interest.
+  if (auto PS = N->getLocationAs())
+if (auto *BO = PS->getStmtAs())
+  if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
+N->getSVal(BO->getLHS()).getAsRegion()))
+return true;
+
+  // SVal after the state is possibly different.
+  SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
+  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() &&
+  (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
+return true;
+
+  return false;
+}
+
+
 namespace {
 
 /// Put a diagnostic on return statement of all inlined functions
@@ -346,7 +378,7 @@
   FramesModifyingCalculated.insert(
 N->getLocationContext()->getStackFrame());
 
-  if (wasRegionOfInterestModifiedAt(N, LastReturnState, ValueAtReturn)) {
+  if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) {
 const StackFrameContext *SCtx = N->getStackFrame();
 while (!SCtx->inTopFrame()) {
   auto p = FramesModifyingRegion.insert(SCtx);
@@ -365,33 +397,6 @@
 } while (N);
   }
 
-  /// \return Whether \c RegionOfInterest was modified at \p N,
-  /// where \p ReturnState is a state associated with the return
-  /// from the current frame.
-  bool wasRegionOfInterestModifiedAt(const ExplodedNode *N,
- ProgramStateRef ReturnState,
- SVal ValueAtReturn) {
-if (!N->getLocationAs()
-&& !N->getLocationAs()
-&& !N->getLocationAs())
-  return false;
-
-// Writing into region of interest.
-if (auto PS = N->getLocationAs())
-  if (auto *BO = PS->getStmtAs())
-if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-N->getSVal(BO->getLHS()).getAsRegion()))
-  return true;
-
-// SVal after the state is possibly different.
-SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() &&
-(!ValueAtN.isUndef() || !ValueAtReturn.isUndef()))
-  return true;
-
-return false;
-  }
-
   /// Get parameters associated with runtime definition in or

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 155753.
simark added a comment.

- Add test
- Make operator== a function instead of method
- Add operator!= (so I can use EXPECT_NE in the test, and because it may be 
useful in general)


Repository:
  rC Clang

https://reviews.llvm.org/D49265

Files:
  include/clang/Tooling/CompilationDatabase.h
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,28 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_EQ(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_NE(CCRef, CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand &LHS, const CompileCommand &RHS) 
{
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand &LHS, const CompileCommand &RHS) 
{
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,28 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_EQ(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_NE(CCRef, CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand &LHS, const CompileCommand &RHS) {
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand &LHS, const CompileCommand &RHS) {
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337214 - [ASTMatchers] Quickfix for tests.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:42:37 2018
New Revision: 337214

URL: http://llvm.org/viewvc/llvm-project?rev=337214&view=rev
Log:
[ASTMatchers] Quickfix for tests.

Modified:
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp?rev=337214&r1=337213&r2=337214&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Mon Jul 16 
13:42:37 2018
@@ -424,28 +424,28 @@ TEST(Matcher, AnyArgument) {
 
 TEST(Matcher, HasReceiver) {
   EXPECT_TRUE(matchesObjC(
-  "@interface NSString @end"
+  "@interface NSString @end "
   "void f(NSString *x) {"
-  "[x containsString]"
+  "[x containsString];"
   "}",
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 
   EXPECT_FALSE(matchesObjC(
-  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "@interface NSString +(NSString *) stringWithFormat; @end "
   "void f() { [NSString stringWithFormat]; }",
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 }
 
 TEST(Matcher, isInstanceMessage) {
   EXPECT_TRUE(matchesObjC(
-  "@interface NSString @end"
+  "@interface NSString @end "
   "void f(NSString *x) {"
-  "[x containsString]"
+  "[x containsString];"
   "}",
   objcMessageExpr(isInstanceMessage(;
 
   EXPECT_FALSE(matchesObjC(
-  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "@interface NSString +(NSString *) stringWithFormat; @end "
   "void f() { [NSString stringWithFormat]; }",
   objcMessageExpr(isInstanceMessage(;
 


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

[PATCH] D48560: [clangd] JSON <-> XPC conversions

[Jan Korous via Phabricator via cfe-commits]

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

Hi Sam, no worries! Thanks for making time to take a look! I am going to rebase 
all my patches on top of JSON library moved to llvm/Support and process your 
comments.




Comment at: clangd/xpc/CMakeLists.txt:1
+set(LLVM_LINK_COMPONENTS
+  support

sammccall wrote:
> I think it'd be helpful for some of us confused linux people to have a brief 
> README in this directory giving some context about XPC in clangd.
> 
> Anything you want to write about the architecture is great but the essentials 
> would be:
>  - alternative transport layer to JSON-RPC
>  - semantically equivalent, uses same LSP request/response message types (?)
>  - MacOS only. Feature is guarded by `CLANGD_BUILD_XPC_SUPPORT`, including 
> whole `xpc/` dir.
That's a good idea, thanks.



Comment at: clangd/xpc/CMakeLists.txt:15
+
+# FIXME: Factor out json::Expr from clangDaemon
+target_link_libraries(ClangdXpcTests

sammccall wrote:
> this is done :-)
Yes, I know - thanks for the good work with lifting the JSON library! I am 
going to rebase my patches.



Comment at: xpc/XPCJSONConversions.cpp:22
+case json::Expr::Boolean: return 
xpc_bool_create(json.asBoolean().getValue());
+case json::Expr::Number:  return 
xpc_double_create(json.asNumber().getValue());
+case json::Expr::String:  return 
xpc_string_create(json.asString().getValue().str().c_str());

sammccall wrote:
> The underlying JSON library actually has both `int64_t` and `double` 
> representations, and I see XPC has both as well.
> If `double` works for all the data sent over this channel, then this approach 
> is simplest. But for completeness, this is also possible:
> ```
> if (auto I = json.asInteger())
>   return xpc_int64_create(*I);
> return xpc_double_create(*json.asNumber());
> ```
> 
> (I don't know of any reason clangd would use large integers today unless they 
> arrived as incoming JSON-RPC request IDs or similar)
I was initially thinking along the same line but got stuck on the fact that 
JSON standard (modelled by llvm::json::Value) doesn't distinguish numeric types 
(it only has Number).

Relevant part of llvm::json::Value interface is:
```
llvm::Optional getAsNumber() const {
if (LLVM_LIKELY(Type == T_Double))
  return as();
if (LLVM_LIKELY(Type == T_Integer))
  return as();
return llvm::None;
  }
  // Succeeds if the Value is a Number, and exactly representable as int64_t.
  llvm::Optional getAsInteger() const {
if (LLVM_LIKELY(Type == T_Integer))
  return as();
if (LLVM_LIKELY(Type == T_Double)) {
  double D = as();
  if (LLVM_LIKELY(std::modf(D, &D) == 0.0 &&
  D >= double(std::numeric_limits::min()) &&
  D <= double(std::numeric_limits::max(
return D;
}
return llvm::None;
  }
```
while both
```enum ValueType```
and
```template T &as() const```
are private which makes sense since those are implementation details. 

Basically it seems to me there's no reliable way how to tell if Value is double 
or not (my example is 2.0) which means we couldn't preserve the "static" 
numeric type (it would be value dependent). I don't think changing the JSON 
library because of this would be a good idea. It's not a big deal for us and 
it's possible we'll skip JSON completely in XPC transport layer in the future.



Comment at: xpc/XPCJSONConversions.cpp:40
+  }
+  // Get pointers only now so there's no possible re-allocation of keys.
+  for(const auto& k : keys)

sammccall wrote:
> (you could also pre-size the vector, or use a deque)
Ok, I'll preallocate necessary space. I remember Bjarne Stroustrup saying he 
doesn't bother with reserve() anymore but it (most probably) won't do any harm 
here.

http://www.stroustrup.com/bs_faq2.html#slow-containers
"People sometimes worry about the cost of std::vector growing incrementally. I 
used to worry about that and used reserve() to optimize the growth. After 
measuring my code and repeatedly having trouble finding the performance 
benefits of reserve() in real programs, I stopped using it except where it is 
needed to avoid iterator invalidation (a rare case in my code). Again: measure 
before you optimize."



Comment at: xpc/XPCJSONConversions.cpp:69
+  xpcObj,
+  ^bool(size_t /* index */, xpc_object_t value) {
+result.emplace_back(xpcToJson(value));

sammccall wrote:
> this looks like objC syntax, I'm not familiar with it, but should this file 
> be `.mm`?
> 
> Alternatively, it seems like you could write a C++ loop with 
> `xpc_array_get_value` (though I don't know if there are performance concerns).
> 
> (and dictionary)
Oh, sorry. These are actually C blocks - a nonstandard C extension.
https://en.wikipedia.org/wiki/Blocks_(C_language_extension)
https://developer.appl

r337215 - [analyzer] Make checkEndFunction() give access to the return statement.

[Reka Kovacs via cfe-commits]

Author: rkovacs
Date: Mon Jul 16 13:47:45 2018
New Revision: 337215

URL: http://llvm.org/viewvc/llvm-project?rev=337215&view=rev
Log:
[analyzer] Make checkEndFunction() give access to the return statement.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h?rev=337215&r1=337214&r2=337215&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h Mon Jul 16 13:47:45 
2018
@@ -254,9 +254,9 @@ public:
 
 class EndFunction {
   template 
-  static void _checkEndFunction(void *checker,
+  static void _checkEndFunction(void *checker, const ReturnStmt *RS,
 CheckerContext &C) {
-((const CHECKER *)checker)->checkEndFunction(C);
+((const CHECKER *)checker)->checkEndFunction(RS, C);
   }
 
 public:

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h?rev=337215&r1=337214&r2=337215&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h Mon Jul 16 
13:47:45 2018
@@ -296,7 +296,8 @@ public:
   void runCheckersForEndFunction(NodeBuilderContext &BC,
  ExplodedNodeSet &Dst,
  ExplodedNode *Pred,
- ExprEngine &Eng);
+ ExprEngine &Eng,
+ const ReturnStmt *RS);
 
   /// Run checkers for branch condition.
   void runCheckersForBranchCondition(const Stmt *condition,
@@ -438,7 +439,8 @@ public:
 
   using CheckBeginFunctionFunc = CheckerFn;
 
-  using CheckEndFunctionFunc = CheckerFn;
+  using CheckEndFunctionFunc =
+  CheckerFn;
   
   using CheckBranchConditionFunc =
   CheckerFn;
@@ -496,7 +498,7 @@ public:
 
   void _registerForEndAnalysis(CheckEndAnalysisFunc checkfn);
 
-  void _registerForBeginFunction(CheckEndFunctionFunc checkfn);
+  void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);
 
   void _registerForBranchCondition(CheckBranchConditionFunc checkfn);

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=337215&r1=337214&r2=337215&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Mon Jul 16 
13:47:45 2018
@@ -126,7 +126,7 @@ public:
  const CallEvent *Call,
  PointerEscapeKind Kind) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
-  void checkEndFunction(CheckerContext &Ctx) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const;
 
 private:
   void diagnoseMissingReleases(CheckerContext &C) const;
@@ -398,7 +398,7 @@ void ObjCDeallocChecker::checkPostObjCMe
 /// Check for missing releases even when -dealloc does not call
 /// '[super dealloc]'.
 void ObjCDeallocChecker::checkEndFunction(
-CheckerContext &C) const {
+const ReturnStmt *RS, CheckerContext &C) const {
   diagnoseMissingReleases(C);
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp?rev=337215&r1=337214&r2=337215&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp (original)
+

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337215: [analyzer] Make checkEndFunction() give access to 
the return statement. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49387?vs=155713&id=155757#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49387

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -439,7 +439,8 @@
 void CheckerManager::runCheckersForEndFunction(NodeBuilderContext &BC,
ExplodedNodeSet &Dst,
ExplodedNode *Pred,
-   ExprEngine &Eng) {
+   ExprEngine &Eng,
+   const ReturnStmt *RS) {
   // We define the builder outside of the loop bacause if at least one checkers
   // creates a sucsessor for Pred, we do not need to generate an
   // autotransition for it.
@@ -449,7 +450,7 @@
   Pred->getLocationContext(),
   checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
-checkFn(C);
+checkFn(RS, C);
   }
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2297,9 +2297,9 @@
 
 // Notify checkers.
 for (const auto I : AfterRemovedDead)
-  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this);
+  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this, RS);
   } else {
-getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this);
+getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
   }
 
   Engine.enqueueEndOfFunction(Dst, RS);
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
@@ -30,7 +30,7 @@
 public:
   void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
   void checkBeginFunction(CheckerContext &C) const;
-  void checkEndFunction(CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 };
 }
 
@@ -56,7 +56,8 @@
   llvm::outs() << "--BEGIN FUNCTION--\n";
 }
 
-void TraversalDumper::checkEndFunction(CheckerContext &C) const {
+void TraversalDumper::checkEndFunction(const ReturnStmt *RS,
+   CheckerContext &C) const {
   llvm::outs() << "--END FUNCTION--\n";
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -2743,7 +2743,7 @@
 
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkBeginFunction(CheckerContext &C) const;
-  void checkEndFunction(CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 
   ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym,
RefVal V, ArgEffect E, RefVal::Kind &hasErr,
@@ -3991,7 +3991,8 @@
   Ctx.addTransition(state);
 }
 
-void RetainCountChecker::checkEndFunction(CheckerContext &Ctx) const {
+void RetainCountChecker::checkEndFunction(const ReturnStmt *RS,
+  CheckerContext &Ctx) const {
   ProgramStateRef state = Ctx.getState();
   RefBindingsTy B = state->get();
   ExplodedNode *Pred = Ctx.getPredecessor();
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cp

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via Phabricator via cfe-commits]

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

In theory you'd need separate tests for op== and op!= returning false 
(currently all the tests would pass if both implementations returned true in 
all cases), but not the biggest deal.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Bump!  Would love to have someone take a look!


https://reviews.llvm.org/D47474



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

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

Quuxplusone added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

mclow.lists wrote:
> lichray wrote:
> > mclow.lists wrote:
> > > Same comment as above about `read` and `inner_product` - they need to be 
> > > "ugly names"
> > Unlike `traits` which is a template parameter name in the standard, `read` 
> > and `inner_product` are function names in the standard, which means the 
> > users cannot make a macro for them (and there is no guarantee about what 
> > name you make **not** get by including certain headers), so we don't need 
> > to use ugly names here, am I right?
> I understand your reasoning, but I don't agree. 
> 
> Just last month, I had to rename a function in `vector` from `allocate` to 
> `__vallocate` because it confused our "is this an allocator" detection. The 
> function in question was private, so it shouldn't have mattered, but GCC has 
> a bug where sometimes it partially ignores access restrictions in non-deduced 
> contexts, and then throws a hard error when it comes back to a different 
> context. The easiest workaround was to rename the function in `vector`.
> 
> Since then, I've been leery of public names that match others. This is pretty 
> obscure, since it's in a private namespace, but I'd feel better if they were 
> `__read` and `__inner_product`.
> 
FWIW, +1 to ugly names. Even if the un-ugly code is "technically not broken 
yet", and besides the technical reason Marshall gives,
(1) it's nice that libc++ has style rules and sticks to them, precisely to 
*avoid* bikeshedding the name of every private member in the world;
(2) just because users can't `#define read write` doesn't mean they *won't* do 
it. I would actually be extremely surprised if `read` were *not* defined as a 
macro somewhere inside ``. :)

See also: "should this function call be `_VSTD::`-qualified?" Sometimes the 
answer is technically "no", but stylistically "yes", precisely to indicate that 
we *don't* intend for it to be an ADL customization point. Consistent style 
leads to maintainability.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1158811, @manojgupta wrote:

> @efriedma @jyknight Does the change match your expectations where warnings 
> are still generated but codeGen does not emit nonnull attribute?


Yes, this seems sensible IMO.




Comment at: include/clang/Driver/Options.td:1080
+def fdelete_null_pointer_checks : Flag<["-"],
+  "fdelete-null-pointer-checks">, Group;
+def fno_delete_null_pointer_checks : Flag<["-"],

Do we need help text for this one too?



Comment at: test/CodeGen/nonnull.c:1-2
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefix=PREFIX-NONNULL %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm 
-fno-delete-null-pointer-checks < %s | FileCheck -check-prefix=PREFIX-NO-NULL %s
 

These prefixes are very confusingly named. NONNULL and NO-NULL sound like the 
same thing.
I'd suggest using, across all the files here,
CHECK:
NULL-INVALID:
NULL-VALID:

That'd also avoid the need to rename all the "CHECK" lines to something else, 
which is most of the diff in these files.



Comment at: test/Sema/nonnull.c:2
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fno-delete-null-pointer-checks -verify %s
 // rdar://9584012

manojgupta wrote:
> All warnings are still issued if nullptr is passed to function nonnull 
> attribute.
SGTM, but this should be a comment in the file, not the code review.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: majnemer, rjmccall.

I discovered that function pointers inside a RecordType that had
its type-determination done in a function whose signature matched
said function pointer resulted in the function pointer type being
emitted empty.  This resulted in information being lost that is
interesting to certain optimizations.

See: https://godbolt.org/g/EegViY

This patch accomplishes this in 2 different situations:
1- When the function itself is being emitted, first convert all

  the return/parameter types to ensure that they are available when
  completing the function type.  This should not result in any additional
  work besides completing parameter types that previously were not completed.

2- When the function type is being evaluated, defer conversion of the record

  type until it is no longer dependent. 


https://reviews.llvm.org/D49403

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenTypes.cpp
  test/CodeGen/func_ptr_recursive_layout.c
  test/CodeGenCXX/pr18962.cpp


Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -756,6 +756,13 @@
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
+  // Prime the type-cache with the types for this function early.  This enables
+  // parameter types that are dependent on this function's type to be properly
+  // emitted.
+  ConvertType(resultType);
+  for (const CanQualType &QT : argTypes)
+ConvertType(QT);
+
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
   paramInfos, resultType, argTypes, required);
Index: lib/CodeGen/CodeGenTypes.cpp
===
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -169,6 +169,14 @@
   if (const auto *AT = CGT.getContext().getAsArrayType(T))
 return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked);
 
+  // If this type has a function pointer that is in the process of being laid
+  // out, delay emitting this RecordDecl until that function type is finished.
+  if (T->isFunctionPointerType())
+return isSafeToConvert(T->getPointeeType(), CGT, AlreadyChecked);
+
+  if (T->isFunctionType())
+return !CGT.isRecordBeingLaidOut(T.getCanonicalType().getTypePtr());
+
   // Otherwise, there is no concern about transforming this.  We only care 
about
   // things that are contained by-value in a structure that can have another 
   // structure as a member.
Index: test/CodeGen/func_ptr_recursive_layout.c
===
--- test/CodeGen/func_ptr_recursive_layout.c
+++ test/CodeGen/func_ptr_recursive_layout.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm -o - %s | FileCheck %s
+
+struct has_fp;
+typedef void (*fp)(const struct has_fp *f);
+struct has_fp {
+  fp i;
+};
+void func(const struct has_fp *f);
+
+void usage() {
+  (void)func;
+}
+
+struct has_fp2;
+typedef void (*fp2)(const struct has_fp2 *f);
+struct has_fp2 {
+  fp2 i;
+};
+void func2(const struct has_fp2 *f) {}
+
+// CHECK-DAG: %struct.has_fp = type { void (%struct.has_fp*)* }
+// CHECK-DAG: %struct.has_fp2 = type { void (%struct.has_fp2*)* }
+
Index: test/CodeGenCXX/pr18962.cpp
===
--- test/CodeGenCXX/pr18962.cpp
+++ test/CodeGenCXX/pr18962.cpp
@@ -25,7 +25,7 @@
 }
 
 // We end up using an opaque type for 'append' to avoid circular references.
-// CHECK: %class.A = type { {}* }
+// CHECK: %class.A = type { void (%class.A*)* }
 // CHECK: %class.C = type <{ %class.D*, %class.B, [3 x i8] }>
 // CHECK: %class.D = type { %class.C.base, [3 x i8] }
 // CHECK: %class.C.base = type <{ %class.D*, %class.B }>


Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -756,6 +756,13 @@
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
+  // Prime the type-cache with the types for this function early.  This enables
+  // parameter types that are dependent on this function's type to be properly
+  // emitted.
+  ConvertType(resultType);
+  for (const CanQualType &QT : argTypes)
+ConvertType(QT);
+
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
   paramInfos, resultType, argTypes, required);
Index: lib/CodeGen/CodeGenTypes.cpp
===
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -169,6 +169,14 @@
   if (const auto *AT = CGT.getContext().getAsArrayType(T))
 return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked);
 
+  // If this type has a function

[PATCH] D48395: Added PublicOnly flag

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/test/clang-doc/yaml-module.cpp:14
+// CHECK-A: ---
+// CHECK-A-NEXT: USR: 
'{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-A-NEXT: Name:'moduleFunction'

ioeric wrote:
> This could be `[0-9A-Z]{N}` where N = length(USR), right?
The `{n}` syntax doesn't work with FileCheck. 


https://reviews.llvm.org/D48395



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

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

Thanks Eli, I'll see how hard it is to remove those calls to logb/logbf (those 
seem to be the only ones causing link errors) and revert this change if that's 
possible.


Repository:
  rC Clang

https://reviews.llvm.org/D49330



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

[PATCH] D49119: [Sema][ObjC] Issue a warning when a method declared in a protocol is non-escaping but the corresponding method in the implementation is escaping.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Overall looks good to me. Maybe add a test when a protocol is declared for an 
interface, not for a category. Something like

  __attribute__((objc_root_class))
  @interface C4 
  -(void) m0:(int*) p; // expected-warning {{parameter of overriding method 
should be annotated with __attribute__((noescape))}}
  @end
  
  @implementation C4
  -(void) m0:(int*) p {
  }
  @end

Looks like there is no such test already and it seems different enough to be 
worth adding. The idea for this test was triggered by code `for (auto *C : 
IDecl->visible_categories())` and the goal is to cover a case when protocol is 
not for a category.

Also I had a few ideas for tests when the warning isn't required and it is 
absent. But I'm not sure they are actually valuable. If you are interested, we 
can discuss it in more details.

Another feedback is an idea for potential improvement: have a note pointing to 
the place where protocol conformity is declared. Currently the warning looks 
like

  code_review.m:16:19: warning: parameter of overriding method should be 
annotated with __attribute__((noescape))
[-Wmissing-noescape]
  -(void) m0:(int*) p { // expected-warning {{parameter of overriding method 
should be annotated with __attri...
^
  code_review.m:2:44: note: parameter of overridden method is annotated with 
__attribute__((noescape))
  -(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of 
overridden method is annotate...
 ^

and we can see the method both in implementation and in protocol. But in some 
cases it might be unclear where exactly that protocol was added to your class. 
I'm not sure this change is sufficiently useful, it's more for discussion.


Repository:
  rC Clang

https://reviews.llvm.org/D49119



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

[PATCH] D45124: [CodeGen] Record if a C++ record is a trivial type when emitting Codeview

[Aaron Smith via Phabricator via cfe-commits]

asmith added a reviewer: aleksandr.urakov.
asmith added a comment.

Adding Aleksandr


Repository:
  rC Clang

https://reviews.llvm.org/D45124



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D49265#1164227, @dblaikie wrote:

> In theory you'd need separate tests for op== and op!= returning false 
> (currently all the tests would pass if both implementations returned true in 
> all cases), but not the biggest deal.


Good point, I'll update it, it will take a second.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.

Just noticed: `getRuntimeDefinition()` has a lot of overrides in `CallEvent` 
sub-classes, and there paths that don't defer to 
`AnyFunctionCall::getRuntimeDefinition()`, eg., ` 
CXXInstanceCall::getRuntimeDefinition()` => `if (MD->isVirtual()) ...`. Which 
means that for some calls we aren't even trying to make a CTU lookup.


Repository:
  rC Clang

https://reviews.llvm.org/D30691



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

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hi Gabor,

Thank you for this explanation, it sounds reasonable. Ithink it should be 
placed into the commit message or into the comment somewhere.


Repository:
  rC Clang

https://reviews.llvm.org/D49300



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 155770.
rnkovacs marked an inline comment as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.

Added standard quote, marking the section about non-member functions that may 
also invalidate the buffer as a TODO.
Also changed the note message to that suggested by @NoQ (thanks!). All tests 
pass now.


https://reviews.llvm.org/D49360

Files:
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -2,13 +2,35 @@
 
 namespace std {
 
-template< typename CharT >
+typedef int size_type;
+
+template 
 class basic_string {
 public:
+  basic_string();
+  basic_string(const CharT *s);
+
   ~basic_string();
+  void clear();
+
+  basic_string &operator=(const basic_string &str);
+  basic_string &operator+=(const basic_string &str);
+
   const CharT *c_str() const;
   const CharT *data() const;
   CharT *data();
+
+  basic_string &append(size_type count, CharT ch);
+  basic_string &assign(size_type count, CharT ch);
+  basic_string &erase(size_type index, size_type count);
+  basic_string &insert(size_type index, size_type count, CharT ch);
+  basic_string &replace(size_type pos, size_type count, const basic_string &str);
+  void pop_back();
+  void push_back(CharT ch);
+  void reserve(size_type new_cap);
+  void resize(size_type count);
+  void shrink_to_fit();
+  void swap(basic_string &other);
 };
 
 typedef basic_string string;
@@ -23,73 +45,70 @@
 void consume(const char16_t *) {}
 void consume(const char32_t *) {}
 
-void deref_after_scope_char_cstr() {
-  const char *c;
+void deref_after_scope_char(bool cond) {
+  const char *c, *d;
   {
 std::string s;
 c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+d = s.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
+  }// expected-note {{Method call is allowed to invalidate the internal buffer}}
+  // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   const char *c2 = s.c_str();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
-}
-
-void deref_after_scope_char_data() {
-  const char *c;
-  {
-std::string s;
-c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::string s;
-  const char *c2 = s.data();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
+  if (cond) {
+// expected-note@-1 {{Assuming 'cond' is not equal to 0}}
+// expected-note@-2 {{Taking true branch}}
+// expected-note@-3 {{Assuming 'cond' is 0}}
+// expected-note@-4 {{Taking false branch}}
+consume(c); // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
+  } else {
+consume(d); // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
+  }
 }
 
 void deref_after_scope_char_data_non_const() {
   char *c;
   {
 std::string s;
 c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  }   // expected-note {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   char *c2 = s.data();
   consume(c); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
-
-void deref_after_scope_wchar_t_cstr() {
-  const wchar_t *w;
+void deref_after_scope_wchar_t(bool cond) {
+  const wchar_t *c, *d;
   {
-std::wstring ws;
-w = ws.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.c_str();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
-}
-
-void deref_after_scope_wchar_t_data() {
-  const wchar_t *w;
-  {
-std::wstring ws;
-w = ws.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.data();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

In https://reviews.llvm.org/D49360#1163113, @NoQ wrote:

> Also we rarely commit to adding a test for every single supported API 
> function; bonus points for that, but usually 2-3 functions from a series of 
> similar functions is enough :)


Um, okay, noted for next time :)


https://reviews.llvm.org/D49360



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 155773.
simark added a comment.

Add tests for both == and !=.

I need to rebuild ~800 files (because I pulled llvm/clang), so I did not
actually test it, but I'll do so before pushing tomorrow, of course.


Repository:
  rC Clang

https://reviews.llvm.org/D49265

Files:
  include/clang/Tooling/CompilationDatabase.h
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,33 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_TRUE(CCRef == CCTest);
+  EXPECT_FALSE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand &LHS, const CompileCommand &RHS) 
{
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand &LHS, const CompileCommand &RHS) 
{
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,33 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_TRUE(CCRef == CCTest);
+  EXPECT_FALSE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand &LHS, const CompileCommand &RHS) {
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand &LHS, const CompileCommand &RHS) {
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Thank you Gabor!


Repository:
  rC Clang

https://reviews.llvm.org/D49296



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

[PATCH] D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6741
+
+  auto *Ctor = dyn_cast(Importer.Import(
+  E->getConstructor()));

balazske wrote:
> a_sidorin wrote:
> > cast_or_null?
> dyn_cast_or_null: Import may return nullptr, but if not, the cast should 
> succeed (not a CXXConstructorDecl would be error).
> Or (but some lines more code) check separately for null return from Import, 
> and then use `cast`?
Usually, we explicitly check that the imported decl has the same kind as the 
source one by filtering lookup results or by creating a decl of same kind. So, 
it's better to assert with cast_or_null in case of kind mismatch.


Repository:
  rC Clang

https://reviews.llvm.org/D49293



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

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D49235



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

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


https://reviews.llvm.org/D48786



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

[PATCH] D48266: [Driver] Add -fno-digraphs

[Jacob Bandes-Storch via Phabricator via cfe-commits]

jtbandes added a comment.

Ping again 😇


Repository:
  rC Clang

https://reviews.llvm.org/D48266



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 155780.
lichray added a comment.

Uglify all the names


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458

Files:
  include/CMakeLists.txt
  include/charconv
  include/module.modulemap
  src/charconv.cpp
  test/libcxx/double_include.sh.cpp
  test/std/utilities/charconv/
  test/std/utilities/charconv/charconv.from.chars/
  test/std/utilities/charconv/charconv.from.chars/integral.bool.fail.cpp
  test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
  test/std/utilities/charconv/charconv.to.chars/
  test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
  test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
  test/support/charconv_test_helpers.h

Index: test/support/charconv_test_helpers.h
===
--- /dev/null
+++ test/support/charconv_test_helpers.h
@@ -0,0 +1,232 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef SUPPORT_CHARCONV_TEST_HELPERS_H
+#define SUPPORT_CHARCONV_TEST_HELPERS_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+#if TEST_STD_VER <= 11
+#error This file requires C++14
+#endif
+
+using std::false_type;
+using std::true_type;
+
+template 
+constexpr auto
+is_non_narrowing(From a) -> decltype(To{a}, true_type())
+{
+return {};
+}
+
+template 
+constexpr auto
+is_non_narrowing(...) -> false_type
+{
+return {};
+}
+
+template 
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{
+return true;
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed*/, true_type /* X signed */)
+{
+return xl::lowest() <= v && v <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed */, false_type /* X unsigned*/)
+{
+return 0 <= v && std::make_unsigned_t(v) <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, false_type /* T unsigned */, ...)
+{
+return v <= std::make_unsigned_t((xl::max)());
+}
+
+template 
+constexpr bool
+fits_in(T v)
+{
+return _fits_in(v, is_non_narrowing(v), std::is_signed(),
+   std::is_signed());
+}
+
+template 
+struct to_chars_test_base
+{
+template 
+void test(T v, char const (&expect)[N], Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+constexpr size_t len = N - 1;
+static_assert(len > 0, "expected output won't be empty");
+
+if (!fits_in(v))
+return;
+
+r = to_chars(buf, buf + len - 1, X(v), args...);
+assert(r.ptr == buf + len - 1);
+assert(r.ec == std::errc::value_too_large);
+
+r = to_chars(buf, buf + sizeof(buf), X(v), args...);
+assert(r.ptr == buf + len);
+assert(r.ec == std::errc{});
+assert(memcmp(buf, expect, len) == 0);
+}
+
+template 
+void test_value(X v, Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+*r.ptr = '\0';
+
+auto a = fromchars(buf, r.ptr, args...);
+assert(v == a);
+
+auto ep = r.ptr - 1;
+r = to_chars(buf, ep, v, args...);
+assert(r.ptr == ep);
+assert(r.ec == std::errc::value_too_large);
+}
+
+private:
+static auto fromchars(char const* p, char const* ep, int base, true_type)
+{
+char* last;
+auto r = strtoll(p, &last, base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base, false_type)
+{
+char* last;
+auto r = strtoull(p, &last, base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base = 10)
+{
+return fromchars(p, ep, base, std::is_signed());
+}
+
+char buf[100];
+};
+
+template 
+struct roundtrip_test_base
+{
+template 
+void test(T v, Ts... args)
+{
+using std::from_chars;
+using std::to_chars;
+std::from_chars_result r2;
+std::to_chars_result r;
+X x = 0xc;
+
+if (fits_in(v))
+{
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+
+r2 = from_chars(buf, r.ptr, x, args...);
+assert(r2.ptr == r.ptr);
+assert(x == X(v));
+}
+else
+{
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+
+r2 = from_chars(buf, r.p

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

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



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

Quuxplusone wrote:
> mclow.lists wrote:
> > lichray wrote:
> > > mclow.lists wrote:
> > > > Same comment as above about `read` and `inner_product` - they need to 
> > > > be "ugly names"
> > > Unlike `traits` which is a template parameter name in the standard, 
> > > `read` and `inner_product` are function names in the standard, which 
> > > means the users cannot make a macro for them (and there is no guarantee 
> > > about what name you make **not** get by including certain headers), so we 
> > > don't need to use ugly names here, am I right?
> > I understand your reasoning, but I don't agree. 
> > 
> > Just last month, I had to rename a function in `vector` from `allocate` to 
> > `__vallocate` because it confused our "is this an allocator" detection. The 
> > function in question was private, so it shouldn't have mattered, but GCC 
> > has a bug where sometimes it partially ignores access restrictions in 
> > non-deduced contexts, and then throws a hard error when it comes back to a 
> > different context. The easiest workaround was to rename the function in 
> > `vector`.
> > 
> > Since then, I've been leery of public names that match others. This is 
> > pretty obscure, since it's in a private namespace, but I'd feel better if 
> > they were `__read` and `__inner_product`.
> > 
> FWIW, +1 to ugly names. Even if the un-ugly code is "technically not broken 
> yet", and besides the technical reason Marshall gives,
> (1) it's nice that libc++ has style rules and sticks to them, precisely to 
> *avoid* bikeshedding the name of every private member in the world;
> (2) just because users can't `#define read write` doesn't mean they *won't* 
> do it. I would actually be extremely surprised if `read` were *not* defined 
> as a macro somewhere inside ``. :)
> 
> See also: "should this function call be `_VSTD::`-qualified?" Sometimes the 
> answer is technically "no", but stylistically "yes", precisely to indicate 
> that we *don't* intend for it to be an ADL customization point. Consistent 
> style leads to maintainability.
`read` is a function decl in ``, not redefined in other forms in 
``.



Comment at: test/support/charconv_test_helpers.h:40
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{

mclow.lists wrote:
> We don't need to use ugly names here in the test suite.
> 
Err, it's not?  Just an impl version of `fits_in` (without the _ prefix).


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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