[PATCH] D61118: [MinGW] Fix dllexport of explicit template instantiation

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, hans, smeenai.
Herald added a project: clang.

Contrary to MSVC, GCC/MinGW needs to have the dllexport attribute on the 
template instantiation declaration, not on the definition.

Previously clang never marked explicit template instantiations as dllexport in 
MinGW mode, regardless of where the attribute was placed. This makes Clang 
behave like GCC in this regard, and allows using the same attribute form for 
both MinGW compilers.

This fixes PR40256.


Repository:
  rC Clang

https://reviews.llvm.org/D61118

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/mingw-template-dllexport.cpp
  test/SemaCXX/dllexport.cpp
  test/SemaCXX/dllimport.cpp

Index: test/SemaCXX/dllimport.cpp
===
--- test/SemaCXX/dllimport.cpp
+++ test/SemaCXX/dllimport.cpp
@@ -1495,6 +1495,9 @@
 #endif
 template struct ExplicitlyInstantiatedTemplate;
 template  struct ExplicitlyExportInstantiatedTemplate { void func() {} };
+#ifndef MS
+// expected-warning@+2{{'dllexport' attribute ignored on explicit instantiation definition}}
+#endif
 template struct __declspec(dllexport) ExplicitlyExportInstantiatedTemplate;
 template  struct ExplicitlyImportInstantiatedTemplate { void func() {} };
 template struct __declspec(dllimport) ExplicitlyImportInstantiatedTemplate;
Index: test/SemaCXX/dllexport.cpp
===
--- test/SemaCXX/dllexport.cpp
+++ test/SemaCXX/dllexport.cpp
@@ -367,10 +367,16 @@
 
 // Don't instantiate class members of templates with explicit instantiation declarations, even if they are exported.
 struct IncompleteType2;
-template  struct __declspec(dllexport) ExportedTemplateWithExplicitInstantiationDecl { // expected-note{{attribute is here}}
+#ifdef MS
+// expected-note@+2{{attribute is here}}
+#endif
+template  struct __declspec(dllexport) ExportedTemplateWithExplicitInstantiationDecl {
   int f() { return sizeof(T); } // no-error
 };
-extern template struct ExportedTemplateWithExplicitInstantiationDecl; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}}
+#ifdef MS
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}}
+#endif
+extern template struct ExportedTemplateWithExplicitInstantiationDecl;
 
 // Instantiate class members for explicitly instantiated exported templates.
 struct IncompleteType3; // expected-note{{forward declaration of 'IncompleteType3'}}
@@ -402,10 +408,17 @@
 
 // Warn about explicit instantiation declarations of dllexport classes.
 template  struct ExplicitInstantiationDeclTemplate {};
-extern template struct __declspec(dllexport) ExplicitInstantiationDeclTemplate; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}} expected-note{{attribute is here}}
+#ifdef MS
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}} expected-note@+2{{attribute is here}}
+#endif
+extern template struct __declspec(dllexport) ExplicitInstantiationDeclTemplate;
 
-template  struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate {}; // expected-note{{attribute is here}}
-extern template struct ExplicitInstantiationDeclExportedTemplate; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}}
+template  struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate {};
+#ifdef MS
+// expected-note@-2{{attribute is here}}
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}}
+#endif
+extern template struct ExplicitInstantiationDeclExportedTemplate;
 
 namespace { struct InternalLinkageType {}; }
 struct __declspec(dllexport) PR23308 {
@@ -437,6 +450,9 @@
 #endif
 template struct ExplicitlyInstantiatedTemplate;
 template  struct ExplicitlyExportInstantiatedTemplate { void func() {} };
+#ifndef MS
+// expected-warning@+2{{'dllexport' attribute ignored on explicit instantiation definition}}
+#endif
 template struct __declspec(dllexport) ExplicitlyExportInstantiatedTemplate;
 template  struct ExplicitlyImportInstantiatedTemplate { void func() {} };
 template struct __declspec(dllimport) ExplicitlyImportInstantiatedTemplate;
Index: test/CodeGenCXX/mingw-template-dllexport.cpp
===
--- /dev/null
+++ test/CodeGenCXX/mingw-template-dllexport.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -triple i686-mingw32 %s -o - | FileCheck %s
+
+template 
+class c {
+  void f();
+};
+
+template  void c::f() {}
+
+template class __declspec(dllexport) c;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIiE1fEv
+
+extern template class __declspec(dllexport) c;
+template class c;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIcE1fEv
+
+extern template class c;
+template class __declspec(dllexport) c;
+
+// CHE

[PATCH] D61118: [MinGW] Fix dllexport of explicit template instantiation

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

LGTM, though I'd wait for Hans and/or Reid too.

I like the GCC/MinGW behavior here much more than the MSVC behavior. Having to 
mark the definitions only in the explicit instantiations case (for MSVC) 
whereas everything else requires you to mark the declarations is a weird 
inconsistency.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61118



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

r359172 - [NFC] test commit removing excess line

[Nikolai Kosjar via cfe-commits]

Author: nik
Date: Thu Apr 25 01:14:39 2019
New Revision: 359172

URL: http://llvm.org/viewvc/llvm-project?rev=359172&view=rev
Log:
[NFC] test commit removing excess line

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

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=359172&r1=359171&r2=359172&view=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Thu Apr 25 01:14:39 2019
@@ -171,7 +171,6 @@ CXSourceRange cxloc::translateSourceRang
 static SourceRange getRawCursorExtent(CXCursor C);
 static SourceRange getFullCursorExtent(CXCursor C, SourceManager &SrcMgr);
 
-
 RangeComparisonResult CursorVisitor::CompareRegionOfInterest(SourceRange R) {
   return RangeCompare(AU->getSourceManager(), R, RegionOfInterest);
 }


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

[PATCH] D61118: [MinGW] Fix dllexport of explicit template instantiation

[Hans Wennborg via Phabricator via cfe-commits]

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

Looks good to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61118



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

[PATCH] D61120: [clangd] Optimize "don't include me" check.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

llvm::Regex is really slow, and regex evaluation during preamble indexing was
showing up as 25% on a profile of clangd in a codebase with large preambles.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61120

Files:
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h


Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -120,12 +120,8 @@
 
   llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID);
   bool isSelfContainedHeader(FileID);
-  // Heuristic to detect headers that aren't self-contained, usually because
-  // they need to be included via an umbrella header. e.g. GTK matches this.
-  llvm::Regex DontIncludeMePattern = {
-  "^[ \t]*#[ \t]*if.*\n" // An #if, #ifndef etc directive, then
-  "[ \t]*#[ \t]*error.*include", // an #error directive mentioning 
"include"
-  llvm::Regex::Newline};
+  // Heuristically headers that only want to be included via an umbrella.
+  static bool isDontIncludeMeHeader(llvm::StringRef);
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -639,7 +639,7 @@
   return false;
 // This pattern indicates that a header can't be used without
 // particular preprocessor state, usually set up by another header.
-if (DontIncludeMePattern.match(SM.getBufferData(FID)))
+if (isDontIncludeMeHeader(SM.getBufferData(FID)))
   return false;
 return true;
   };
@@ -650,5 +650,36 @@
   return R.first->second;
 }
 
+// Is Line an #if or #ifdef directive?
+static bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+// Is Line an #error directive mentioning includes?
+static bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  if (! Line.startswith("error"))
+return false;
+  return Line.contains_lower("includ");
+}
+
+bool SymbolCollector::isDontIncludeMeHeader(llvm::StringRef Content) {
+  llvm::StringRef Line;
+  // Only sniff up to 100 lines or 10KB.
+  Content = Content.take_front(100*100);
+  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
+std::tie(Line, Content) = Content.split('\n');
+if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
+  return true;
+  }
+  return false;
+}
+
 } // namespace clangd
 } // namespace clang


Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -120,12 +120,8 @@
 
   llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID);
   bool isSelfContainedHeader(FileID);
-  // Heuristic to detect headers that aren't self-contained, usually because
-  // they need to be included via an umbrella header. e.g. GTK matches this.
-  llvm::Regex DontIncludeMePattern = {
-  "^[ \t]*#[ \t]*if.*\n" // An #if, #ifndef etc directive, then
-  "[ \t]*#[ \t]*error.*include", // an #error directive mentioning "include"
-  llvm::Regex::Newline};
+  // Heuristically headers that only want to be included via an umbrella.
+  static bool isDontIncludeMeHeader(llvm::StringRef);
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -639,7 +639,7 @@
   return false;
 // This pattern indicates that a header can't be used without
 // particular preprocessor state, usually set up by another header.
-if (DontIncludeMePattern.match(SM.getBufferData(FID)))
+if (isDontIncludeMeHeader(SM.getBufferData(FID)))
   return false;
 return true;
   };
@@ -650,5 +650,36 @@
   return R.first->second;
 }
 
+// Is Line an #if or #ifdef directive?
+static bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+// Is Line an #error directive mentioning includes?
+static bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  if (! Line.startswith("error"))
+return false;
+  return Line.contains_lower("includ");
+}
+
+bool SymbolCollector::isDontIncludeMeHeader(llvm:

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added a comment.

Thanks for working on this! :)




Comment at: clang/lib/Parse/ParseDecl.cpp:3533
+  if (ExplicitExpr.isInvalid()) {
+Diag(ParenLoc, diag::note_explicit_bool_breaking_change_cxx2a)
+<< FixItHint::CreateReplacement(

This is a useful diagnostic but you'll need to fix (a lot) of false positives:

```
template  struct Foo { explicit(T{} +) Foo(); };
```

gets me:

```
main.cpp:1:50: error: expected expression
template  struct Foo { explicit(T{} +) Foo(); };
 ^
main.cpp:1:44: note: this expression is parsed as explicit(bool) since C++2a
template  struct Foo { explicit(T{} +) Foo(); };
   ^
   explicit(true)
```

Fixit hints should only be used when it is 100% the right fix that works, 
always.



Comment at: clang/lib/Parse/ParseDecl.cpp:3534
+Diag(ParenLoc, diag::note_explicit_bool_breaking_change_cxx2a)
+<< FixItHint::CreateReplacement(
+   SourceRange(ExplicitLoc, ExplicitLoc.getLocWithOffset(

FixIt Hints also need tests :)



Comment at: clang/lib/Sema/DeclSpec.cpp:952
+  assert((ExplicitState != ESF_unresolved || ExplicitExpr) &&
+ "ExplicitExpr can't be null if the state is ESF_resolved_false or "
+ "ESF_unresolved");

The comment seems to explain something else than the code.



Comment at: clang/lib/Sema/DeclSpec.cpp:956
   // intended.
-  if (FS_explicit_specified) {
+  // TODO : it is unclear how to handle multiple explicit(bool) specifiers.
+  if (hasExplicitSpecifier()) {

We accept `explicit explicit`, but we really shouldn't accept 
`explicit(some-expr) explicit(some-other-expr)`. That would just be confusing. 
`explicit explicit(some-expr)` is also borderline.



Comment at: clang/lib/Sema/DeclSpec.cpp:1316
 FixItHint Hint = FixItHint::CreateRemoval(SCLoc);
 S.Diag(SCLoc, diag::err_friend_decl_spec)
   << Keyword << Hint;

Please change this warning so that the whole *explicit-specifier* is underlined 
(or even change the text for conditional explicit):

```
main.cpp:1:36: error: 'explicit' can only be applied to a constructor or 
conversion function
template  struct Foo { explicit(false) void foo(); };
   ^~~~
1 error generated.
```

This will also fix the FixIt Hint.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8641
   // C++0x explicit conversion operators.
-  if (DS.isExplicitSpecified())
+  if (DS.hasExplicitSpecifier())
 Diag(DS.getExplicitSpecLoc(),

You should amend the diagnostic, because as of right now `explicit(true)` is 
being diagnosed as C++11 feature.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10870
+  ExprResult Converted =
+  CheckBooleanCondition(Loc, ExplicitExpr, /*isConstexpr=*/true);
+  if (Converted.isInvalid())

This assumes that we are in a [constexpr] if, leading to errors like this:

```
int a;
template  struct Foo { explicit(a) Foo(); };
```

```
main.cpp:2:45: error: constexpr if condition is not a constant expression
template  struct Foo { explicit(a) Foo(); };
^
```



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10876-10881
+return Converted;
+  }
+
+  llvm::APSInt Result;
+  Converted = VerifyIntegerConstantExpression(
+  Converted.get(), &Result, diag::err_noexcept_needs_constant_expression,

Wrong diagnostic.



Comment at: clang/lib/Sema/SemaOverload.cpp:10359
+  default:
+llvm_unreachable("invalide Decl");
+  }

typo.




Comment at: clang/lib/Sema/SemaOverload.cpp:10361
+  }
+  assert(ESI.getPointer() && "null expression should be handled before");
+  S.Diag(Cand->Function->getLocation(),

This fires for an instantiation dependent (but not value dependent) explicit 
specifier that is invalid:

```
template  struct Foo { explicit((T{}, false)) Foo(int); };

int main() { Foo f = 1; }
```



Comment at: clang/test/SemaCXX/cxx2a-compat.cpp:46
+#if __cplusplus <= 201703L
+// expected-warning@-3 {{this expression would be parsed as explicit(bool) in 
C++2a}}
+#if defined(__cpp_conditional_explicit)

would -> will



Comment at: clang/test/SemaCXX/cxx2a-compat.cpp:51
+#else
+// expected-error@-8 {{does not refer to a value}}
+// expected-error@-9 {{expected member name or ';'}}

A fixit hint for this would be great. Also it would be nice if there was a 
nicer error message.


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

https://reviews.llvm.org/D60934



___
cfe-comm

[PATCH] D60764: Add clang cc1 option to generate OpenCL builtin functions

[Pierre via Phabricator via cfe-commits]

Pierre updated this revision to Diff 196590.
Pierre added a comment.

Forgot to update one argument


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

https://reviews.llvm.org/D60764

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -820,7 +820,7 @@
   }
 
   // Check if this is an OpenCL Builtin, and if so, insert the 
declarations.
-  if (S.getLangOpts().OpenCL) {
+  if (S.getLangOpts().OpenCL && S.getLangOpts().AddOpenCLBuiltins) {
 auto Index = isOpenCLBuiltin(II->getName());
 if (Index.first) {
   InsertBuiltinDeclarations(S, R, II, Index.first, Index.second);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2152,7 +2152,7 @@
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
 // Include default header file for OpenCL.
-if (Opts.IncludeDefaultHeader) {
+if (Opts.IncludeDefaultHeader && !Opts.AddOpenCLBuiltins) {
   PPOpts.Includes.push_back("opencl-c.h");
 }
   }
@@ -2355,6 +2355,7 @@
   }
 
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.AddOpenCLBuiltins = Args.hasArg(OPT_fadd_opencl_builtins);
 
   llvm::Triple T(TargetOpts.Triple);
   CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangStd);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -751,7 +751,9 @@
 def fdefault_calling_conv_EQ : Joined<["-"], "fdefault-calling-conv=">,
   HelpText<"Set default calling convention">, 
Values<"cdecl,fastcall,stdcall,vectorcall,regcall">;
 def finclude_default_header : Flag<["-"], "finclude-default-header">,
-  HelpText<"Include the default header file for OpenCL">;
+  HelpText<"Include default header file containing OpenCL builtin functions">;
+def fadd_opencl_builtins: Flag<["-"], "fadd-opencl-builtins">,
+  HelpText<"Add OpenCL builtin function declarations automatically">;
 def fpreserve_vec3_type : Flag<["-"], "fpreserve-vec3-type">,
   HelpText<"Preserve 3-component vector type">;
 def fwchar_type_EQ : Joined<["-"], "fwchar-type=">,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -254,7 +254,8 @@
 LANGOPT(CFProtectionBranch , 1, 0, "Control-Flow Branch Protection enabled")
 LANGOPT(FakeAddressSpaceMap , 1, 0, "OpenCL fake address space map")
 ENUM_LANGOPT(AddressSpaceMapMangling , AddrSpaceMapMangling, 2, ASMM_Target, 
"OpenCL address space map mangling mode")
-LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
+LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file containing 
OpenCL builtin functions")
+LANGOPT(AddOpenCLBuiltins, 1, 0, "Add OpenCL builtin function declarations 
automatically")
 BENIGN_LANGOPT(DelayedTemplateParsing , 1, 0, "delayed template parsing")
 LANGOPT(BlocksRuntimeOptional , 1, 0, "optional blocks runtime")
 LANGOPT(


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -820,7 +820,7 @@
   }
 
   // Check if this is an OpenCL Builtin, and if so, insert the declarations.
-  if (S.getLangOpts().OpenCL) {
+  if (S.getLangOpts().OpenCL && S.getLangOpts().AddOpenCLBuiltins) {
 auto Index = isOpenCLBuiltin(II->getName());
 if (Index.first) {
   InsertBuiltinDeclarations(S, R, II, Index.first, Index.second);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2152,7 +2152,7 @@
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
 // Include default header file for OpenCL.
-if (Opts.IncludeDefaultHeader) {
+if (Opts.IncludeDefaultHeader && !Opts.AddOpenCLBuiltins) {
   PPOpts.Includes.push_back("opencl-c.h");
 }
   }
@@ -2355,6 +2355,7 @@
   }
 
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.AddOpenCLBuiltins = Args.hasArg(OPT_fadd_opencl_builtins);
 
   llvm::Triple T(TargetOpts.Triple);
   CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangStd);
Index: clang/include/clang/Driver/CC1Options.td
=

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 196592.
ilya-biryukov added a comment.

- Add simple tests
- Add equality and stream output operators for Range


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814

Files:
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt
  llvm/unittests/Support/AnnotationsTest.cpp
  llvm/unittests/Support/CMakeLists.txt

Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -5,6 +5,7 @@
 add_llvm_unittest(SupportTests
   AlignOfTest.cpp
   AllocatorTest.cpp
+  AnnotationsTest.cpp
   ARMAttributeParser.cpp
   ArrayRecyclerTest.cpp
   BinaryStreamTest.cpp
Index: llvm/unittests/Support/AnnotationsTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/AnnotationsTest.cpp
@@ -0,0 +1,87 @@
+//===- unittests/AnnotationsTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-generated-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+namespace {
+llvm::Annotations::Range range(size_t Begin, size_t End) {
+  llvm::Annotations::Range R;
+  R.Begin = Begin;
+  R.End = End;
+  return R;
+}
+
+TEST(AnnotationsTest, CleanedCode) {
+  EXPECT_EQ(llvm::Annotations("foo^bar$nnn[[baz$^[[qux").code(),
+"foobarbazqux");
+}
+
+TEST(AnnotationsTest, Points) {
+  // A single point.
+  EXPECT_EQ(llvm::Annotations("^ab").point(), 0u);
+  EXPECT_EQ(llvm::Annotations("a^b").point(), 1u);
+  EXPECT_EQ(llvm::Annotations("ab^").point(), 2u);
+
+  // Multiple points.
+  EXPECT_THAT(llvm::Annotations("^a^bc^d^").points(),
+  ElementsAre(0u, 1u, 3u, 4u));
+
+  // No points.
+  EXPECT_THAT(llvm::Annotations("ab[[cd]]").points(), IsEmpty());
+
+  // Consecutive points.
+  EXPECT_THAT(llvm::Annotations("ab^^^cd").points(), ElementsAre(2u, 2u, 2u));
+}
+
+TEST(AnnotationsTest, Ranges) {
+  // A single range.
+  EXPECT_EQ(llvm::Annotations("[[a]]bc").range(), range(0, 1));
+  EXPECT_EQ(llvm::Annotations("a[[bc]]d").range(), range(1, 3));
+  EXPECT_EQ(llvm::Annotations("ab[[cd]]").range(), range(2, 4));
+
+  // Empty range.
+  EXPECT_EQ(llvm::Annotations("[[]]ab").range(), range(0, 0));
+  EXPECT_EQ(llvm::Annotations("a[[]]b").range(), range(1, 1));
+  EXPECT_EQ(llvm::Annotations("ab[[]]").range(), range(2, 2));
+
+  // Multiple ranges.
+  EXPECT_THAT(llvm::Annotations("[[a]][[b]]cd[[ef]]ef").ranges(),
+  ElementsAre(range(0, 1), range(1, 2), range(4, 6)));
+
+  // No ranges.
+  EXPECT_THAT(llvm::Annotations("ab^c^defef").ranges(), IsEmpty());
+}
+
+TEST(AnnotationsTest, Nested) {
+  llvm::Annotations Annotated("a[[f^oo^bar[[b[[a]]zbcdef");
+  EXPECT_THAT(Annotated.points(), ElementsAre(2u, 4u));
+  EXPECT_THAT(Annotated.ranges(),
+  ElementsAre(range(8, 9), range(7, 10), range(1, 10)));
+}
+
+TEST(AnnotationsTest, Named) {
+  // A single named point or range.
+  EXPECT_EQ(llvm::Annotations("a$foo^b").point("foo"), 1u);
+  EXPECT_EQ(llvm::Annotations("a$foo[[b]]cdef").range("foo"), range(1, 2));
+
+  // Empty names should also work.
+  EXPECT_EQ(llvm::Annotations("a$^b").point(""), 1u);
+  EXPECT_EQ(llvm::Annotations("a$[[b]]cdef").range(""), range(1, 2));
+
+  // Multiple named points.
+  llvm::Annotations Annotated("a$p1^bcd$p2^123$p1^345");
+  EXPECT_THAT(Annotated.points(), IsEmpty());
+  EXPECT_THAT(Annotated.points("p1"), ElementsAre(1u, 7u));
+  EXPECT_EQ(Annotated.point("p2"), 4u);
+}
+} // namespace
Index: llvm/lib/Testing/Support/CMakeLists.txt
===
--- llvm/lib/Testing/Support/CMakeLists.txt
+++ llvm/lib/Testing/Support/CMakeLists.txt
@@ -2,6 +2,7 @@
 add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
 
 add_llvm_library(LLVMTestingSupport
+  Annotations.cpp
   Error.cpp
   SupportHelpers.cpp
 
Index: llvm/lib/Testing/Support/Annotations.cpp
===
--- llvm/lib/Testing/Support/Annotations.cpp
+++ llvm/lib/Testing/Support/Annotations.cpp
@@ -6,11 +6,13 @@
 //
 //===--===//
 
-#include "An

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre updated this revision to Diff 196591.
Pierre marked 10 inline comments as done.
Pierre added a comment.

In this new patch:

- Documentation has been added
- The multiclasses in OpenCLBuiltins.td filehave been slighly changed to have a 
more generic way to generate function prototypes
- In ClangOpenCLBuiltinEmitter.cpp, the code of the Emit() function has been 
shifted to functions
- In SemaLookUp.cpp, the OCL2Qual function, used to retrieve the QualType 
instance of a type, is now generated by Tablegen backend


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

https://reviews.llvm.org/D60763

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/OpenCLBuiltins.td
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/builtin-new.cl
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -80,6 +80,7 @@
 void EmitTestPragmaAttributeSupportedAttributes(llvm::RecordKeeper &Records,
 llvm::raw_ostream &OS);
 
+void EmitClangOpenCLBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 } // end namespace clang
 
 #endif
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -61,7 +61,8 @@
   GenDiagDocs,
   GenOptDocs,
   GenDataCollectors,
-  GenTestPragmaAttributeSupportedAttributes
+  GenTestPragmaAttributeSupportedAttributes,
+  GenClangOpenCLBuiltins,
 };
 
 namespace {
@@ -163,7 +164,9 @@
 clEnumValN(GenTestPragmaAttributeSupportedAttributes,
"gen-clang-test-pragma-attribute-supported-attributes",
"Generate a list of attributes supported by #pragma clang "
-   "attribute for testing purposes")));
+   "attribute for testing purposes"),
+clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
+   "Generate OpenCL builtin handlers")));
 
 cl::opt
 ClangComponent("clang-component",
@@ -293,6 +296,9 @@
   case GenTestPragmaAttributeSupportedAttributes:
 EmitTestPragmaAttributeSupportedAttributes(Records, OS);
 break;
+  case GenClangOpenCLBuiltins:
+EmitClangOpenCLBuiltins(Records, OS);
+break;
   }
 
   return false;
Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -0,0 +1,320 @@
+//===- ClangOpenCLBuiltinEmitter.cpp - Generate Clang OpenCL Builtin handling
+//=-*- C++ -*--=//
+//
+// The LLVM Compiler Infrastructure
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits Clang OpenCL builtin functions checking code.
+//
+//===--===//
+
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include 
+
+using namespace llvm;
+
+namespace {
+class BuiltinNameEmitter {
+public:
+  BuiltinNameEmitter(RecordKeeper &Records, raw_ostream &OS)
+  : Records(Records), OS(OS) {}
+
+  void Emit();
+
+private:
+  RecordKeeper &Records;
+  raw_ostream &OS;
+
+  void EmitDeclarations();
+  void GetOverloads();
+  void EmitSignatureTable();
+  void EmitBuiltinTable();
+  void EmitStringMatcher();
+  void EmitQualTypeFinder();
+
+  // Contains a list of the available signatures, regardless the name of the
+  // function. Each pair consists in a signature and a cumulative index.
+  // E.g.:  <, 0>,
+  //<>,
+  //<, 5>,
+  //...
+  //<, 35>.
+  std::vector, unsigned>> SignatureSet;
+
+  // Map the name of a builtin function to its signatures.
+  // Each signature is registered as a pair of:
+  // 
+  // E.g.:  The function cos: (float cos(float), double cos(double), ...)
+  //<"cos", <,
+  //>
+  //>
+  // ptrToSignature35 points here to the following list: 
+  MapVector>>
+OverloadInfo;
+};
+} 

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre added a comment.

Other comments:
1-
When a header file is included, its function declarations are decorated with 
the "nounwind" attribute, meaning that the function is not supposed to throw an 
exception. This decorator is currently not added with the new mechanism.
The "readnone" decorator is also present for these builtin functions and added 
somewhere y clang, but not added with the new mechanism.
This can be tested with the following command line, on an .cl file containing a 
function calling a builtin function (e.g.: acos):

  clang -cc1 -cl-std=CL2.0 -emit-llvm -O0 
-I/llvm-project/clang/lib/Headers   
[-fadd-opencl-builtins|-finclude-default-header]

2-
When the function definition is inserted, it seems to be actually just 
resolving the identifier for the current lookup. Calling the same builtin 
function multiple times currently result in multiple lookup. Maybe there is a 
way to add the function declaration for the scope/file, so the lookup is only 
performed one time for each function. This part is done around the code taken 
from Sema::LazilyCreateBuiltin function, and I will spend more time on it.
3-
I haven't looked at the test file yet.
4-
If you have any suggestion/comment, feel free to share.




Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:65
+  def float_t  : Type<"float">;
+  def double_t : Type<"double">;
+}

Anastasia wrote:
> half?
The signature of OpenCL builtin functions taking/returning half types are not 
part of OpenCL by default, this is part of OpenCL 2.0 extensions (cf 
https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-extensions.pdf)



Comment at: clang/lib/Sema/SemaLookup.cpp:675
 
+// TODO: Auto-generate this from tablegen
+static QualType OCL2Qual(ASTContext &Context, OpenCLType Ty) {

Anastasia wrote:
> Does this mean we have to produce this in `ClangOpenCLBuiltinEmitter.cpp`?
Yes I think so, I have made a change in this way


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

https://reviews.llvm.org/D60763



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

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Djordje Todorovic via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Clang.cpp:3402
+CmdArgs.push_back("-femit-param-entry-values");
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);

probinson wrote:
> If this is now a cc1-only option, this part goes away right?
Yes, thanks for this!


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

https://reviews.llvm.org/D58033



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

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Djordje Todorovic via Phabricator via cfe-commits]

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

-Remove unneeded code


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

https://reviews.llvm.org/D58033

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/CC1Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Frontend/CompilerInvocation.cpp


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,7 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  Opts.EnableParamEntryValues = Args.hasArg(OPT_femit_param_entry_values);
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4558,7 +4558,10 @@
   // were part of DWARF v4.
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
-  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
+   (CGM.getCodeGenOpts().EnableParamEntryValues &&
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+
   if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
 return llvm::DINode::FlagZero;
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -463,6 +463,7 @@
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EnableParamEntryValues = CodeGenOpts.EnableParamEntryValues;
 
   if (CodeGenOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -364,6 +364,8 @@
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
 def fthin_link_bitcode_EQ : Joined<["-"], "fthin-link-bitcode=">,
 HelpText<"Write minimized bitcode to  for the ThinLTO thin link 
only">;
+def femit_param_entry_values : Flag<["-"], "femit-param-entry-values">,
+HelpText<"Enables debug info about call site parameter's entry values">;
 def fdebug_pass_manager : Flag<["-"], "fdebug-pass-manager">,
 HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
Index: include/clang/Basic/CodeGenOptions.def
===
--- include/clang/Basic/CodeGenOptions.def
+++ include/clang/Basic/CodeGenOptions.def
@@ -61,6 +61,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(EnableParamEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
  ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,7 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  Opts.EnableParamEntryValues = Args.hasArg(OPT_femit_param_entry_values);
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4558,7 +4558,10 @@
   // were part of DWARF v4.
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
-  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
+   (CGM.getCodeGenOpts().EnableParamEntryValues &&
+   CGM.getCodeGenOpts

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, hans, smeenai, thakis.
Herald added a project: clang.

Path lists on windows should always be separated by semicolons, not colons. 
Reuse llvm::sys::EnvPathSeparator for this purpose (as that's also a path list 
that is separated in the same way).

Alternatively, this could just be a local ifdef _WIN32 in this function, or 
should we generalize the existing EnvPathSeparator to e.g. a 
llvm::sys::path::PathListSeparator?

Does this change need a test? I guess we could add a separate file with 
"REQUIRES: system-windows" and check if the output does contain a semicolon, 
but I'm not sure how meaningful that is, especially as the default list printed 
only contains one single path element.

This was noted within Qt (and worked around with a creative regex) at 
https://codereview.qt-project.org/247331.


Repository:
  rC Clang

https://reviews.llvm.org/D61121

Files:
  lib/Driver/Driver.cpp


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1694,7 +1694,7 @@
 bool separator = false;
 for (const std::string &Path : TC.getProgramPaths()) {
   if (separator)
-llvm::outs() << ':';
+llvm::outs() << llvm::sys::EnvPathSeparator;
   llvm::outs() << Path;
   separator = true;
 }
@@ -1705,7 +1705,7 @@
 
 for (const std::string &Path : TC.getFilePaths()) {
   // Always print a separator. ResourceDir was the first item shown.
-  llvm::outs() << ':';
+  llvm::outs() << llvm::sys::EnvPathSeparator;
   // Interpretation of leading '=' is needed only for NetBSD.
   if (Path[0] == '=')
 llvm::outs() << sysroot << Path.substr(1);


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1694,7 +1694,7 @@
 bool separator = false;
 for (const std::string &Path : TC.getProgramPaths()) {
   if (separator)
-llvm::outs() << ':';
+llvm::outs() << llvm::sys::EnvPathSeparator;
   llvm::outs() << Path;
   separator = true;
 }
@@ -1705,7 +1705,7 @@
 
 for (const std::string &Path : TC.getFilePaths()) {
   // Always print a separator. ResourceDir was the first item shown.
-  llvm::outs() << ':';
+  llvm::outs() << llvm::sys::EnvPathSeparator;
   // Interpretation of leading '=' is needed only for NetBSD.
   if (Path[0] == '=')
 llvm::outs() << sysroot << Path.substr(1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r359179 - [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Apr 25 03:08:31 2019
New Revision: 359179

URL: http://llvm.org/viewvc/llvm-project?rev=359179&view=rev
Log:
[Testing] Move clangd::Annotations to llvm testing support

Summary:
Annotations allow writing nice-looking unit test code when one needs
access to locations from the source code, e.g. running code completion
at particular offsets in a file. See comments in Annotations.cpp for
more details on the API.

Also got rid of a duplicate annotations parsing code in clang's code
complete tests.

Reviewers: gribozavr, sammccall

Reviewed By: gribozavr

Subscribers: mgorny, hiraditya, ioeric, MaskRay, jkorous, arphaman, kadircet, 
jdoerfert, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Modified:
clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
clang-tools-extra/trunk/unittests/clangd/Annotations.h

Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.cpp?rev=359179&r1=359178&r2=359179&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp Thu Apr 25 
03:08:31 2019
@@ -12,74 +12,41 @@
 namespace clang {
 namespace clangd {
 
-// Crash if the assertion fails, printing the message and testcase.
-// More elegant error handling isn't needed for unit tests.
-static void require(bool Assertion, const char *Msg, llvm::StringRef Code) {
-  if (!Assertion) {
-llvm::errs() << "Annotated testcase: " << Msg << "\n" << Code << "\n";
-llvm_unreachable("Annotated testcase assertion failed!");
-  }
+Position Annotations::point(llvm::StringRef Name) const {
+  return offsetToPosition(code(), Base::point(Name));
 }
 
-Annotations::Annotations(llvm::StringRef Text) {
-  auto Here = [this] { return offsetToPosition(Code, Code.size()); };
-  auto Require = [Text](bool Assertion, const char *Msg) {
-require(Assertion, Msg, Text);
-  };
-  llvm::Optional Name;
-  llvm::SmallVector, 8> OpenRanges;
-
-  Code.reserve(Text.size());
-  while (!Text.empty()) {
-if (Text.consume_front("^")) {
-  Points[Name.getValueOr("")].push_back(Here());
-  Name = None;
-  continue;
-}
-if (Text.consume_front("[[")) {
-  OpenRanges.emplace_back(Name.getValueOr(""), Here());
-  Name = None;
-  continue;
-}
-Require(!Name, "$name should be followed by ^ or [[");
-if (Text.consume_front("]]")) {
-  Require(!OpenRanges.empty(), "unmatched ]]");
-  Ranges[OpenRanges.back().first].push_back(
-  {OpenRanges.back().second, Here()});
-  OpenRanges.pop_back();
-  continue;
-}
-if (Text.consume_front("$")) {
-  Name = Text.take_while(llvm::isAlnum);
-  Text = Text.drop_front(Name->size());
-  continue;
-}
-Code.push_back(Text.front());
-Text = Text.drop_front();
-  }
-  Require(!Name, "unterminated $name");
-  Require(OpenRanges.empty(), "unmatched [[");
-}
+std::vector Annotations::points(llvm::StringRef Name) const {
+  auto Offsets = Base::points(Name);
 
-Position Annotations::point(llvm::StringRef Name) const {
-  auto I = Points.find(Name);
-  require(I != Points.end() && I->getValue().size() == 1,
-  "expected exactly one point", Code);
-  return I->getValue()[0];
+  std::vector Ps;
+  Ps.reserve(Offsets.size());
+  for (size_t O : Offsets)
+Ps.push_back(offsetToPosition(code(), O));
+
+  return Ps;
 }
-std::vector Annotations::points(llvm::StringRef Name) const {
-  auto P = Points.lookup(Name);
-  return {P.begin(), P.end()};
+
+static clangd::Range toLSPRange(llvm::StringRef Code, Annotations::Range R) {
+  clangd::Range LSPRange;
+  LSPRange.start = offsetToPosition(Code, R.Begin);
+  LSPRange.end = offsetToPosition(Code, R.End);
+  return LSPRange;
 }
-Range Annotations::range(llvm::StringRef Name) const {
-  auto I = Ranges.find(Name);
-  require(I != Ranges.end() && I->getValue().size() == 1,
-  "expected exactly one range", Code);
-  return I->getValue()[0];
+
+clangd::Range Annotations::range(llvm::StringRef Name) const {
+  return toLSPRange(code(), Base::range(Name));
 }
-std::vector Annotations::ranges(llvm::StringRef Name) const {
-  auto R = Ranges.lookup(Name);
-  return {R.begin(), R.end()};
+
+std::vector Annotations::ranges(llvm::StringRef Name) const {
+  auto OffsetRanges = Base::ranges(Name);
+
+  std::vector Rs;
+  Rs.reserve(OffsetRanges.size());
+  for (Annotations::Range R : OffsetRanges)
+Rs.push_back(toLSPRange(code(), R));
+
+  return Rs;
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.h?rev=359179&r1=359178&r2=359179&view=diff
===

r359179 - [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Apr 25 03:08:31 2019
New Revision: 359179

URL: http://llvm.org/viewvc/llvm-project?rev=359179&view=rev
Log:
[Testing] Move clangd::Annotations to llvm testing support

Summary:
Annotations allow writing nice-looking unit test code when one needs
access to locations from the source code, e.g. running code completion
at particular offsets in a file. See comments in Annotations.cpp for
more details on the API.

Also got rid of a duplicate annotations parsing code in clang's code
complete tests.

Reviewers: gribozavr, sammccall

Reviewed By: gribozavr

Subscribers: mgorny, hiraditya, ioeric, MaskRay, jkorous, arphaman, kadircet, 
jdoerfert, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Modified:
cfe/trunk/unittests/Sema/CMakeLists.txt
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/unittests/Sema/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CMakeLists.txt?rev=359179&r1=359178&r2=359179&view=diff
==
--- cfe/trunk/unittests/Sema/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Sema/CMakeLists.txt Thu Apr 25 03:08:31 2019
@@ -16,4 +16,5 @@ target_link_libraries(SemaTests
   clangSema
   clangSerialization
   clangTooling
+  LLVMTestingSupport
   )

Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=359179&r1=359178&r2=359179&view=diff
==
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Apr 25 03:08:31 2019
@@ -13,6 +13,7 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -107,41 +108,18 @@ CompletionContext runCompletion(StringRe
   return ResultCtx;
 }
 
-struct ParsedAnnotations {
-  std::vector Points;
-  std::string Code;
-};
-
-ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) {
-  ParsedAnnotations R;
-  while (!AnnotatedCode.empty()) {
-size_t NextPoint = AnnotatedCode.find('^');
-if (NextPoint == StringRef::npos) {
-  R.Code += AnnotatedCode;
-  AnnotatedCode = "";
-  break;
-}
-R.Code += AnnotatedCode.substr(0, NextPoint);
-R.Points.push_back(R.Code.size());
-
-AnnotatedCode = AnnotatedCode.substr(NextPoint + 1);
-  }
-  return R;
-}
-
 CompletionContext runCodeCompleteOnCode(StringRef AnnotatedCode) {
-  ParsedAnnotations P = parseAnnotations(AnnotatedCode);
-  assert(P.Points.size() == 1 && "expected exactly one annotation point");
-  return runCompletion(P.Code, P.Points.front());
+  llvm::Annotations A(AnnotatedCode);
+  return runCompletion(A.code(), A.point());
 }
 
 std::vector
 collectPreferredTypes(StringRef AnnotatedCode,
   std::string *PtrDiffType = nullptr) {
-  ParsedAnnotations P = parseAnnotations(AnnotatedCode);
+  llvm::Annotations A(AnnotatedCode);
   std::vector Types;
-  for (size_t Point : P.Points) {
-auto Results = runCompletion(P.Code, Point);
+  for (size_t Point : A.points()) {
+auto Results = runCompletion(A.code(), Point);
 if (PtrDiffType) {
   assert(PtrDiffType->empty() || *PtrDiffType == Results.PtrDiffType);
   *PtrDiffType = Results.PtrDiffType;


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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: llvm/lib/Testing/Support/Annotations.cpp:94
+const llvm::Annotations::Range &R) {
+  return O << llvm::formatv("[{0}, {1})", R.Begin, R.End);
+}

You could consider including the stringref in the Range struct just to make it 
print more nicely.
Not sure of all the implications of this, up to you.



Comment at: llvm/unittests/Support/AnnotationsTest.cpp:87
+}
+} // namespace

you could consider some #ifndef NDEBUG EXPECT_DEATH tests for point() with 
no/multiple points etc. Up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814



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

[PATCH] D61122: [clangd] Don't build clangd or run its tests when LLVM_ENABLE_THREADS is off, unless specifically directed to do so

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: thakis, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
mgorny.
Herald added a project: clang.

Introduce a new CLANGD_BUILD option, only default to true if threads are on.

(the name doesn't seem ideal, I'm trying to follow precedent of 
CLANGD_BUILD_XPC)

This is a bit of a mess because of the parallel unittest/ and test/ trees in
clang-tools-extra (clangd has three root directories).

I'd like to move clang-tools-extra/test/clangd -> clang-tools-extra/clangd/test 
etc,
but will do that afterwards.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61122

Files:
  CMakeLists.txt
  test/CMakeLists.txt
  test/clangd/lit.local.cfg
  unittests/CMakeLists.txt

Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -21,4 +21,6 @@
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
 add_subdirectory(clang-tidy)
-add_subdirectory(clangd)
+if (CLANGD_BUILD)
+  add_subdirectory(clangd)
+endif()
Index: test/clangd/lit.local.cfg
===
--- test/clangd/lit.local.cfg
+++ test/clangd/lit.local.cfg
@@ -4,3 +4,5 @@
 # FIXME: make our tests less brittle instead.
 if re.match(r'.*-scei-ps4', config.target_triple):
   config.unsupported = True
+if 'CLANGD_BUILD' not in lit_config.params:
+  config.unsupported = True
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -70,33 +70,36 @@
   clang
 )
 
-if(CLANGD_BUILD_XPC)
-  list(APPEND CLANG_TOOLS_TEST_DEPS clangd-xpc-test-client)
-endif()
-
-set(CLANGD_TEST_DEPS
-  clangd
-  ClangdTests
-  # clangd-related tools which don't have tests, add them to the test to make
-  # sure we don't introduce new changes that break their compilations.
-  clangd-indexer
-  dexp
-  )
-
-# Add lit test dependencies.
 set(LLVM_UTILS_DEPS
   FileCheck count not
 )
 foreach(dep ${LLVM_UTILS_DEPS})
   if(TARGET ${dep})
-list(APPEND CLANGD_TEST_DEPS ${dep})
+list(APPEND CLANG_TOOLS_TEST_DEPS ${dep})
   endif()
 endforeach()
 
-foreach(clangd_dep ${CLANGD_TEST_DEPS})
-  list(APPEND CLANG_TOOLS_TEST_DEPS
-   ${clangd_dep})
-endforeach()
+if ( CLANGD_BUILD )
+  if(CLANGD_BUILD_XPC)
+list(APPEND CLANG_TOOLS_TEST_DEPS clangd-xpc-test-client)
+  endif()
+
+  set(CLANGD_TEST_DEPS
+clangd
+ClangdTests
+# clangd-related tools which don't have tests, add them to the test to make
+# sure we don't introduce new changes that break their compilations.
+clangd-indexer
+dexp
+)
+
+  foreach(clangd_dep ${CLANGD_TEST_DEPS})
+list(APPEND CLANG_TOOLS_TEST_DEPS
+ ${clangd_dep})
+  endforeach()
+
+  set(CLANG_TOOLS_TEST_EXTRA_ARGS ${CLANG_TEST_EXTRA_ARGS} "-DCLANGD_BUILD=1")
+endif()
 
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
@@ -106,14 +109,17 @@
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
 
-# Setup an individual test for building and testing clangd-only stuff.
-# Note: all clangd tests have been covered in check-clang-tools, this is a
-# convenient target for clangd developers.
-# Exclude check-clangd from check-all.
-set(EXCLUDE_FROM_ALL ON)
-add_lit_testsuite(check-clangd "Running the Clangd regression tests"
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
-  DEPENDS ${CLANGD_TEST_DEPS}
-)
-set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
-set(EXCLUDE_FROM_ALL OFF)
+if ( CLANGD_BUILD )
+  # Setup an individual test for building and testing clangd-only stuff.
+  # Note: all clangd tests have been covered in check-clang-tools, this is a
+  # convenient target for clangd developers.
+  # Exclude check-clangd from check-all.
+  set(EXCLUDE_FROM_ALL ON)
+  add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+DEPENDS ${CLANGD_TEST_DEPS}
+ARGS -DCLANGD_BUILD=1
+  )
+  set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
+  set(EXCLUDE_FROM_ALL OFF)
+endif()
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -9,6 +9,8 @@
 
   unset(CLANGD_BUILD_XPC_DEFAULT)
 endif ()
+# FIXME: move clangd to its own self-contained subtree and clean this up.
+option(CLANGD_BUILD "Build clangd" ${LLVM_ENABLE_THREADS})
 
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
@@ -21,7 +23,9 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
+if ( CLANGD_BUILD )
+  add_subdirectory(clangd)
+endif()
 add_subdirectory(pp-trace)
 add_subdir

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 196599.
ilya-biryukov added a comment.

- Added a death test for error conditions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814

Files:
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt
  llvm/unittests/Support/AnnotationsTest.cpp
  llvm/unittests/Support/CMakeLists.txt

Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -5,6 +5,7 @@
 add_llvm_unittest(SupportTests
   AlignOfTest.cpp
   AllocatorTest.cpp
+  AnnotationsTest.cpp
   ARMAttributeParser.cpp
   ArrayRecyclerTest.cpp
   BinaryStreamTest.cpp
Index: llvm/unittests/Support/AnnotationsTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/AnnotationsTest.cpp
@@ -0,0 +1,112 @@
+//===- unittests/AnnotationsTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+namespace {
+llvm::Annotations::Range range(size_t Begin, size_t End) {
+  llvm::Annotations::Range R;
+  R.Begin = Begin;
+  R.End = End;
+  return R;
+}
+
+TEST(AnnotationsTest, CleanedCode) {
+  EXPECT_EQ(llvm::Annotations("foo^bar$nnn[[baz$^[[qux").code(),
+"foobarbazqux");
+}
+
+TEST(AnnotationsTest, Points) {
+  // A single point.
+  EXPECT_EQ(llvm::Annotations("^ab").point(), 0u);
+  EXPECT_EQ(llvm::Annotations("a^b").point(), 1u);
+  EXPECT_EQ(llvm::Annotations("ab^").point(), 2u);
+
+  // Multiple points.
+  EXPECT_THAT(llvm::Annotations("^a^bc^d^").points(),
+  ElementsAre(0u, 1u, 3u, 4u));
+
+  // No points.
+  EXPECT_THAT(llvm::Annotations("ab[[cd]]").points(), IsEmpty());
+
+  // Consecutive points.
+  EXPECT_THAT(llvm::Annotations("ab^^^cd").points(), ElementsAre(2u, 2u, 2u));
+}
+
+TEST(AnnotationsTest, Ranges) {
+  // A single range.
+  EXPECT_EQ(llvm::Annotations("[[a]]bc").range(), range(0, 1));
+  EXPECT_EQ(llvm::Annotations("a[[bc]]d").range(), range(1, 3));
+  EXPECT_EQ(llvm::Annotations("ab[[cd]]").range(), range(2, 4));
+
+  // Empty range.
+  EXPECT_EQ(llvm::Annotations("[[]]ab").range(), range(0, 0));
+  EXPECT_EQ(llvm::Annotations("a[[]]b").range(), range(1, 1));
+  EXPECT_EQ(llvm::Annotations("ab[[]]").range(), range(2, 2));
+
+  // Multiple ranges.
+  EXPECT_THAT(llvm::Annotations("[[a]][[b]]cd[[ef]]ef").ranges(),
+  ElementsAre(range(0, 1), range(1, 2), range(4, 6)));
+
+  // No ranges.
+  EXPECT_THAT(llvm::Annotations("ab^c^defef").ranges(), IsEmpty());
+}
+
+TEST(AnnotationsTest, Nested) {
+  llvm::Annotations Annotated("a[[f^oo^bar[[b[[a]]zbcdef");
+  EXPECT_THAT(Annotated.points(), ElementsAre(2u, 4u));
+  EXPECT_THAT(Annotated.ranges(),
+  ElementsAre(range(8, 9), range(7, 10), range(1, 10)));
+}
+
+TEST(AnnotationsTest, Named) {
+  // A single named point or range.
+  EXPECT_EQ(llvm::Annotations("a$foo^b").point("foo"), 1u);
+  EXPECT_EQ(llvm::Annotations("a$foo[[b]]cdef").range("foo"), range(1, 2));
+
+  // Empty names should also work.
+  EXPECT_EQ(llvm::Annotations("a$^b").point(""), 1u);
+  EXPECT_EQ(llvm::Annotations("a$[[b]]cdef").range(""), range(1, 2));
+
+  // Multiple named points.
+  llvm::Annotations Annotated("a$p1^bcd$p2^123$p1^345");
+  EXPECT_THAT(Annotated.points(), IsEmpty());
+  EXPECT_THAT(Annotated.points("p1"), ElementsAre(1u, 7u));
+  EXPECT_EQ(Annotated.point("p2"), 4u);
+}
+
+TEST(AnnotationsTest, Errors) {
+  // Annotations use llvm_unreachable, it will only crash in debug mode.
+#ifndef NDEBUG
+  // point() and range() crash on zero or multiple ranges.
+  EXPECT_DEATH(llvm::Annotations("ab[[c]]def").point(),
+   "expected exactly one point");
+  EXPECT_DEATH(llvm::Annotations("a^b^cdef").point(),
+   "expected exactly one point");
+
+  EXPECT_DEATH(llvm::Annotations("a^bcdef").range(),
+   "expected exactly one range");
+  EXPECT_DEATH(llvm::Annotations("a[[b]]c[[d]]ef").range(),
+   "expected exactly one range");
+
+  EXPECT_DEATH(llvm::Annotations("$foo^a$foo^a").point("foo"),
+   "expected exactly one point");
+  EXPECT_DEATH(llvm::Annotations("$foo[[a]]bc$foo[[a]]"

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359179: [Testing] Move clangd::Annotations to llvm testing 
support (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59814?vs=196599&id=196602#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59814

Files:
  cfe/trunk/unittests/Sema/CMakeLists.txt
  cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
  clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
  clang-tools-extra/trunk/unittests/clangd/Annotations.h
  llvm/trunk/include/llvm/Testing/Support/Annotations.h
  llvm/trunk/lib/Testing/Support/Annotations.cpp
  llvm/trunk/lib/Testing/Support/CMakeLists.txt
  llvm/trunk/unittests/Support/AnnotationsTest.cpp
  llvm/trunk/unittests/Support/CMakeLists.txt

Index: cfe/trunk/unittests/Sema/CMakeLists.txt
===
--- cfe/trunk/unittests/Sema/CMakeLists.txt
+++ cfe/trunk/unittests/Sema/CMakeLists.txt
@@ -16,4 +16,5 @@
   clangSema
   clangSerialization
   clangTooling
+  LLVMTestingSupport
   )
Index: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
===
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
@@ -13,6 +13,7 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -107,41 +108,18 @@
   return ResultCtx;
 }
 
-struct ParsedAnnotations {
-  std::vector Points;
-  std::string Code;
-};
-
-ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) {
-  ParsedAnnotations R;
-  while (!AnnotatedCode.empty()) {
-size_t NextPoint = AnnotatedCode.find('^');
-if (NextPoint == StringRef::npos) {
-  R.Code += AnnotatedCode;
-  AnnotatedCode = "";
-  break;
-}
-R.Code += AnnotatedCode.substr(0, NextPoint);
-R.Points.push_back(R.Code.size());
-
-AnnotatedCode = AnnotatedCode.substr(NextPoint + 1);
-  }
-  return R;
-}
-
 CompletionContext runCodeCompleteOnCode(StringRef AnnotatedCode) {
-  ParsedAnnotations P = parseAnnotations(AnnotatedCode);
-  assert(P.Points.size() == 1 && "expected exactly one annotation point");
-  return runCompletion(P.Code, P.Points.front());
+  llvm::Annotations A(AnnotatedCode);
+  return runCompletion(A.code(), A.point());
 }
 
 std::vector
 collectPreferredTypes(StringRef AnnotatedCode,
   std::string *PtrDiffType = nullptr) {
-  ParsedAnnotations P = parseAnnotations(AnnotatedCode);
+  llvm::Annotations A(AnnotatedCode);
   std::vector Types;
-  for (size_t Point : P.Points) {
-auto Results = runCompletion(P.Code, Point);
+  for (size_t Point : A.points()) {
+auto Results = runCompletion(A.code(), Point);
 if (PtrDiffType) {
   assert(PtrDiffType->empty() || *PtrDiffType == Results.PtrDiffType);
   *PtrDiffType = Results.PtrDiffType;
Index: clang-tools-extra/trunk/unittests/clangd/Annotations.h
===
--- clang-tools-extra/trunk/unittests/clangd/Annotations.h
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.h
@@ -5,64 +5,32 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-// Annotations lets you mark points and ranges inside source code, for tests:
-//
-//Annotations Example(R"cpp(
-//   int complete() { x.pri^ }  // ^ indicates a point
-//   void err() { [["hello" == 42]]; }  // [[this is a range]]
-//   $definition^class Foo{};   // points can be named: "definition"
-//   $fail[[static_assert(false, "")]]  // ranges can be named too: "fail"
-//)cpp");
-//
-//StringRef Code = Example.code();  // annotations stripped.
-//std::vector PP = Example.points();  // all unnamed points
-//Position P = Example.point(); // there must be exactly one
-//Range R = Example.range("fail");  // find named ranges
-//
-// Points/ranges are coordinates into `code()` which is stripped of annotations.
-//
-// Ranges may be nested (and points can be inside ranges), but there's no way
-// to define general overlapping ranges.
-//
+// A clangd-specific version of llvm/Testing/Support/Annotations.h, replaces
+// offsets and offset-based ranges with types from the LSP protocol.
 //===-===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_ANNOTATIONS_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_ANNOTATIONS_H
 
 #include "Protocol.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/

[PATCH] D61120: [clangd] Optimize "don't include me" check.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM (see the comment about a typo, though)




Comment at: clangd/index/SymbolCollector.cpp:669
+return false;
+  return Line.contains_lower("includ");
+}

A typo? Should it be `include`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61120



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

[PATCH] D61122: [clangd] Don't build clangd or run its tests when LLVM_ENABLE_THREADS is off, unless specifically directed to do so

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

A better name could definitely be nice, e.g. `BUILD_CLANGD` would've been much 
nicer.
But the current one is also okayish, being consistent with existing names is a 
good reason to keep it that way.




Comment at: CMakeLists.txt:13
+# FIXME: move clangd to its own self-contained subtree and clean this up.
+option(CLANGD_BUILD "Build clangd" ${LLVM_ENABLE_THREADS})
 

NIT: maybe comment why we disable clangd with no threads?
Merely mentioning this configuration is not supported in clangd would be good 
enough.

Maybe it's just me, but I tend to find following cmake scripts really hard and 
even obvious comments make it much simpler.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61122



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

[PATCH] D61120: [clangd] Optimize "don't include me" check.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:669
+return false;
+  return Line.contains_lower("includ");
+}

ilya-biryukov wrote:
> A typo? Should it be `include`.
it's meant to match "include" or "including". I'll add a comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61120



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

[PATCH] D61120: [clangd] Optimize "don't include me" check.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:669
+return false;
+  return Line.contains_lower("includ");
+}

sammccall wrote:
> ilya-biryukov wrote:
> > A typo? Should it be `include`.
> it's meant to match "include" or "including". I'll add a comment.
Makes sense. Comment would work, thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61120



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

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

clangd currently prefers declarations from codegen files. This patch
implements that behavior for definition locations. If we have definiton
locations both coming from AST and index, clangd will perform a merging to show
the codegen file if that's the case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61126

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -186,7 +186,8 @@
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,27 @@
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Proto;
+void func() {}
+P$p^roto* create() {
+  fu$f^nc();
+  return nullptr;
+}
   )cpp");
 
   auto AST = TestTU::withCode(Test.code()).build();
-  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
-  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("p"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("p");
+EXPECT_THAT(Locs, ElementsAre(Sym("Proto", CodeGenLoc, CodeGenLoc)));
+llvm::errs() << Locs.front() << '\n';
+  }
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("f"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("f");
+EXPECT_THAT(Locs, ElementsAre(Sym("func", CodeGenLoc, CodeGenLoc)));
+llvm::errs() << Locs.front() << '\n';
+  }
 }
 
 TEST(LocateSymbol, All) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -346,28 +346,27 @@
 Index->lookup(QueryRequest, [&](const Symbol &Sym) {
   auto &R = Result[ResultIndex.lookup(Sym.ID)];
 
-  // Special case: if the AST yielded a definition, then it may not be
-  // the right *declaration*. Prefer the one from the index.
   if (R.Definition) { // from AST
-if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
-  R.PreferredDeclaration = *Loc;
+// In case of generated files we prefer to omit the definition in the
+// generated code.
 if (auto Loc = toLSPLocation(
 getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
 *MainFilePath))
   R.Definition = *Loc;
+
+// Special case: if the AST yielded a definition, then it may not be
+// the right *declaration*. Prefer the one from the index.
+if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
+  R.PreferredDeclaration = *Loc;
   } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
 
-if (Sym.CanonicalDeclaration) {
-  // Use merge logic to choose AST or index declaration.
-  // We only do this for declarations as definitions from AST
-  // is generally preferred (e.g. definitions in main file).
-  if (auto Loc = toLSPLocation(
-  getPreferredLocation(R.PreferredDeclaration,
-   Sym.CanonicalDeclaration, Scratch),
-  *MainFilePath))
-R.PreferredDeclaration = *Loc;
-}
+// Use merge logic to choose AST or index declaration.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration, Scratch),
+*MainFilePath))
+  R.PreferredDeclaration = *Loc;
   }
 });
   }


Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -186,7 +186,8 @@
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,27 @@
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Prot

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Javed Absar via Phabricator via cfe-commits]

javed.absar marked an inline comment as done.
javed.absar added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

t.p.northover wrote:
> I think this should be fixed now.  It looks like technical debt from the fact 
> that the instructions only fairly recently gained that feature after the 
> intrinsics were implemented internally. There's no good way to justify the 
> current semantics to someone unaware of that history.
Not quite that really.  So the instruction did gain the feature recently like 
you mentioned. But the ACLE/intrinsics were designed and agreed upon after it 
and it was decided in ACLE discussions that the exta feature added complexity 
that need not be exposed at ACLE level yet. No big use case to justify 
complicating the ACLE MTE spec yet. Directly assembly can use that instruction 
though.


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

https://reviews.llvm.org/D60485



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

Re: r358490 - [AArch64] Implement Vector Funtion ABI name mangling.

[Russell Gallop via cfe-commits]

Hi Alexey,

The new test "declare_simd_aarch64_sve.c" intermittently fails when the git
revision contains "a01".

.../llvm/tools/clang/test/OpenMP/declare_simd_aarch64_sve.c:38:15: error:
CHECK-NOT: excluded string found in input
// CHECK-NOT: a01
  ^
:75:102: note: found here
!1 = !{!"clang version 9.0.0 (.git
60d61fe3f64a01de5ac24f6c17cddb391ec3e02c)"}

 ^~~

I reckon this will fail about 1 in 100 times when built from a git
repository.

Please could you improve the test to avoid this issue?

Thanks
Russ

On Tue, 16 Apr 2019 at 14:54, Alexey Bataev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: abataev
> Date: Tue Apr 16 06:56:21 2019
> New Revision: 358490
>
> URL: http://llvm.org/viewvc/llvm-project?rev=358490&view=rev
> Log:
> [AArch64] Implement Vector Funtion ABI name mangling.
>
> Summary:
> The name mangling scheme is defined in section 3.5 of the "Vector function
> application binary interface specification for AArch64" [1].
>
> [1]
> https://developer.arm.com/products/software-development-tools/hpc/arm-compiler-for-hpc/vector-function-abi
>
> Reviewers: rengolin, ABataev
>
> Reviewed By: ABataev
>
> Subscribers: sdesmalen, javed.absar, kristof.beyls, jdoerfert, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D60583
>
> Added:
> cfe/trunk/test/OpenMP/Inputs/declare-simd-fix.h
> cfe/trunk/test/OpenMP/declare_simd_aarch64.c
> cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp
> cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c
> cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c
> cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c
> cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c
> cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c
> Modified:
> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=358490&r1=358489&r2=358490&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Apr 16 06:56:21 2019
> @@ -9648,6 +9648,307 @@ emitX86DeclareSimdFunction(const Functio
>}
>  }
>
> +// This are the Functions that are needed to mangle the name of the
> +// vector functions generated by the compiler, according to the rules
> +// defined in the "Vector Function ABI specifications for AArch64",
> +// available at
> +//
> https://developer.arm.com/products/software-development-tools/hpc/arm-compiler-for-hpc/vector-function-abi
> .
> +
> +/// Maps To Vector (MTV), as defined in 3.1.1 of the AAVFABI.
> +///
> +/// TODO: Need to implement the behavior for reference marked with a
> +/// var or no linear modifiers (1.b in the section). For this, we
> +/// need to extend ParamKindTy to support the linear modifiers.
> +static bool getAArch64MTV(QualType QT, ParamKindTy Kind) {
> +  QT = QT.getCanonicalType();
> +
> +  if (QT->isVoidType())
> +return false;
> +
> +  if (Kind == ParamKindTy::Uniform)
> +return false;
> +
> +  if (Kind == ParamKindTy::Linear)
> +return false;
> +
> +  // TODO: Handle linear references with modifiers
> +
> +  if (Kind == ParamKindTy::LinearWithVarStride)
> +return false;
> +
> +  return true;
> +}
> +
> +/// Pass By Value (PBV), as defined in 3.1.2 of the AAVFABI.
> +static bool getAArch64PBV(QualType QT, ASTContext &C) {
> +  QT = QT.getCanonicalType();
> +  unsigned Size = C.getTypeSize(QT);
> +
> +  // Only scalars and complex within 16 bytes wide set PVB to true.
> +  if (Size != 8 && Size != 16 && Size != 32 && Size != 64 && Size != 128)
> +return false;
> +
> +  if (QT->isFloatingType())
> +return true;
> +
> +  if (QT->isIntegerType())
> +return true;
> +
> +  if (QT->isPointerType())
> +return true;
> +
> +  // TODO: Add support for complex types (section 3.1.2, item 2).
> +
> +  return false;
> +}
> +
> +/// Computes the lane size (LS) of a return type or of an input parameter,
> +/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
> +/// TODO: Add support for references, section 3.2.1, item 1.
> +static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext
> &C) {
> +  if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
> +QualType PTy = QT.getCanonicalType()->getPointeeType();
> +if (getAArch64PBV(PTy, C))
> +  return C.getTypeSize(PTy);
> +  }
> +  if (getAArch64PBV(QT, C))
> +return C.getTypeSize(QT);
> +
> +  return C.getTypeSize(C.getUIntPtrType());
> +}
> +
> +// Get Narrowest Data Size (NDS) and Widest Data Size (WDS) from the
> +// signature of the scalar function, as defined in 3.2.2 of the
> +// AAVFABI.
> +static std::tuple
> +getNDSWDS(const FunctionDecl *FD, ArrayRef ParamAttrs) {
> +  QualType RetType = FD->getReturnType().getCanonicalType();
> +
> +  ASTContext &C = FD->getAST

[PATCH] D60925: [analyzer] Don't display implementation checkers under -analyzer-checker-help, but do under the new flag -analyzer-checker-help-hidden

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

Szelethus added a comment.

I plan to do the same for non-checker and checker options too.

I'll leave this up for just a little while to give people time to object.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60925



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

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Nicola Zaghen via Phabricator via cfe-commits]

Nicola added inline comments.



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:140-144
+class RoundingModes {
+  list Modes = !if(!or(_Ty1.HasRounding, _Ty2.HasRounding),
+  ["", "_rte", "_rtz", "_rtp", "_rtn"],
+  [""]);
+}

I think the CL standard (1.2, section 6.2.3.2) specifies that RoundingModes 
should be available on all convert_ functions, not only selectively based on 
types: "Conversions may have an optional rounding mode modifier described in 
table 6.6. "



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:211-212
+  def float_t   : Type<"float", QualType<"field", "FloatTy">>;
+  def double_t  : Type<"double", QualType<"field", "DoubleTy">>;
+  def half_t: Type<"half", QualType<"field", "HalfTy">>;
+}

Can half and double types and builtins be made dependent on extensions or 
configurable? The half datatype needs the cl_khr_fp16 extension, while double 
needs CL_DEVICE_DOUBLE_FP_CONFIG or cl_khr_fp64


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

https://reviews.llvm.org/D60763



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D60934#1475908 , @Tyker wrote:

> Fixed issues form feedback by @martong


Thank you for addressing the comments. `ASTImporter.cpp` and 
`ASTStructuralEquivalence.cpp` looks good to me now. However, about the 
equivalence check, do you think it would be possible to have a unit test for 
explicit(bool) in `StructuralEquivalenceTest.cpp`? That test would exercise the 
static `EquivalentExplicit()` function too.


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

https://reviews.llvm.org/D60934



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

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/SourceCode.cpp:503
+case tok::l_brace:
+  if (State == NamespaceName) {
+// Parsed: namespace  {

I believe it is safe to ignore(just mark the opening brace) anonymous 
namespaces here. Since there were no comments(and no test cases) just wanted to 
make sure you did not miss that case.



Comment at: clangd/SourceCode.cpp:595
+  });
+  Found.erase(std::unique(Found.begin(), Found.end()), Found.end());
+  return Found;

`scopesForIndexQuery` already de-duplicates. Do you plan to have any other 
users for the results of this function?



Comment at: clangd/SourceCode.h:169
+/// The returned vector is always non-empty.
+/// - The first element is the namespace that encloses the point: a declaration
+///   near the point would be within this namespace.

Does the code ever make use of it?



Comment at: unittests/clangd/SourceCodeTests.cpp:325
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {

NIT: maybe switch to TEST_P ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61077



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

Re: r358490 - [AArch64] Implement Vector Funtion ABI name mangling.

[Alexey Bataev via cfe-commits]

Hi Russel, thanks for the report, will fix this problem.

Best regards,
Alexey Bataev

25 апр. 2019 г., в 7:24, Russell Gallop 
mailto:russell.gal...@gmail.com>> написал(а):

Hi Alexey,

The new test "declare_simd_aarch64_sve.c" intermittently fails when the git 
revision contains "a01".

.../llvm/tools/clang/test/OpenMP/declare_simd_aarch64_sve.c:38:15: error: 
CHECK-NOT: excluded string found in input
// CHECK-NOT: a01
  ^
:75:102: note: found here
!1 = !{!"clang version 9.0.0 (.git 
60d61fe3f64a01de5ac24f6c17cddb391ec3e02c)"}

 ^~~

I reckon this will fail about 1 in 100 times when built from a git repository.

Please could you improve the test to avoid this issue?

Thanks
Russ

On Tue, 16 Apr 2019 at 14:54, Alexey Bataev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: abataev
Date: Tue Apr 16 06:56:21 2019
New Revision: 358490

URL: http://llvm.org/viewvc/llvm-project?rev=358490&view=rev
Log:
[AArch64] Implement Vector Funtion ABI name mangling.

Summary:
The name mangling scheme is defined in section 3.5 of the "Vector function 
application binary interface specification for AArch64" [1].

[1] 
https://developer.arm.com/products/software-development-tools/hpc/arm-compiler-for-hpc/vector-function-abi

Reviewers: rengolin, ABataev

Reviewed By: ABataev

Subscribers: sdesmalen, javed.absar, kristof.beyls, jdoerfert, llvm-commits

Tags: #llvm

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

Added:
cfe/trunk/test/OpenMP/Inputs/declare-simd-fix.h
cfe/trunk/test/OpenMP/declare_simd_aarch64.c
cfe/trunk/test/OpenMP/declare_simd_aarch64.cpp
cfe/trunk/test/OpenMP/declare_simd_aarch64_complex.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_fix.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_advsimd.c
cfe/trunk/test/OpenMP/declare_simd_aarch64_warning_sve.c
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=358490&r1=358489&r2=358490&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Apr 16 06:56:21 2019
@@ -9648,6 +9648,307 @@ emitX86DeclareSimdFunction(const Functio
   }
 }

+// This are the Functions that are needed to mangle the name of the
+// vector functions generated by the compiler, according to the rules
+// defined in the "Vector Function ABI specifications for AArch64",
+// available at
+// 
https://developer.arm.com/products/software-development-tools/hpc/arm-compiler-for-hpc/vector-function-abi.
+
+/// Maps To Vector (MTV), as defined in 3.1.1 of the AAVFABI.
+///
+/// TODO: Need to implement the behavior for reference marked with a
+/// var or no linear modifiers (1.b in the section). For this, we
+/// need to extend ParamKindTy to support the linear modifiers.
+static bool getAArch64MTV(QualType QT, ParamKindTy Kind) {
+  QT = QT.getCanonicalType();
+
+  if (QT->isVoidType())
+return false;
+
+  if (Kind == ParamKindTy::Uniform)
+return false;
+
+  if (Kind == ParamKindTy::Linear)
+return false;
+
+  // TODO: Handle linear references with modifiers
+
+  if (Kind == ParamKindTy::LinearWithVarStride)
+return false;
+
+  return true;
+}
+
+/// Pass By Value (PBV), as defined in 3.1.2 of the AAVFABI.
+static bool getAArch64PBV(QualType QT, ASTContext &C) {
+  QT = QT.getCanonicalType();
+  unsigned Size = C.getTypeSize(QT);
+
+  // Only scalars and complex within 16 bytes wide set PVB to true.
+  if (Size != 8 && Size != 16 && Size != 32 && Size != 64 && Size != 128)
+return false;
+
+  if (QT->isFloatingType())
+return true;
+
+  if (QT->isIntegerType())
+return true;
+
+  if (QT->isPointerType())
+return true;
+
+  // TODO: Add support for complex types (section 3.1.2, item 2).
+
+  return false;
+}
+
+/// Computes the lane size (LS) of a return type or of an input parameter,
+/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
+/// TODO: Add support for references, section 3.2.1, item 1.
+static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext &C) {
+  if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+QualType PTy = QT.getCanonicalType()->getPointeeType();
+if (getAArch64PBV(PTy, C))
+  return C.getTypeSize(PTy);
+  }
+  if (getAArch64PBV(QT, C))
+return C.getTypeSize(QT);
+
+  return C.getTypeSize(C.getUIntPtrType());
+}
+
+// Get Narrowest Data Size (NDS) and Widest Data Size (WDS) from the
+// signature of the scalar function, as defined in 3.2.2 of the
+// AAVFABI.
+static std::tuple
+getNDSWDS(const FunctionDecl *FD, ArrayRef ParamAttrs) {
+  QualType RetType = FD->getReturnType().getCanonicalType();
+
+  ASTConte

[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Why would we consider this a legitimate failure, rather than a programming 
error?
Same argument could be made about any form of format-string-like functions, 
e.g. `llvm::formatv` or `sprintf`.
Yet, they return strings and not `Expected` or their equivalent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1074
+  if (!RD->hasTrivialCopyAssignment())
+return true;
+  return false;

Should this function also check for user-provided constructors?


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

https://reviews.llvm.org/D60349



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.h:113
 
+  // Note that FileSymbols counts References by incrementing it per each file
+  // mentioning the symbol, including headers. This contradicts with the

We should agree on and stick to a single behavior in both cases.
I suggest keeping the current behavior for now (translation units). Why can't 
we implement that?



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:52
+  continue;
+// If this is the first time we see references to a symbol, reset its
+// Reference count, since filesymbols re-count number of references

The comment suggests there's something cheesy going on.
Why would FileSymbols recompute the number of references? Can we avoid this 
complicated logic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

You may be interested in D60605 , which is a 
related idea (adding incremental format-and-indent-on-type to clangd).

These are opposite extremes in some sense: this patch integrates deeply into 
clang-format and that patch entirely layers on top of it, by transforming the 
source code (in hacky ways, in some cases).

I think the cleanest approach is probably some combination: in particular a 
"preserve line break at point" primitive does seem useful, and even if source 
code transforms are used, putting a nice API on this in clang-format would be 
nice. (I'm not sure a separate command-line-tool is justified, though).

If you haven't run into these yet, here are some fun issues I ran into:

- clang-format bails out when brackets aren't sufficiently matched, incremental 
formatting needs to work in such cases
- incremental formatting sometimes wants to make edits both immediately before 
and after the cursor. `tooling::Replacements` can't represent this, they will 
be merged and destroy precise cursor position information.
- user expectations when breaking between `()` are fairly clear. `{}` is the 
same when it acts as a list, but not when it acts as a block!


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

https://reviews.llvm.org/D53072



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:78
+// offsets when displaying that information to users.
+Position toOneBased(Position P) {
+  ++P.line;

Could we avoid introducing a function that breaks the invariant of a type?
Having a function to print `Position` differently would be a much better fit.



Comment at: clangd/Diagnostics.cpp:225
+const auto &Inc = D.IncludeStack[I];
+OS << "\nIn file included from: " << Inc.first << ':'
+   << toOneBased(Inc.second);

WDYT about changing this to a shorter form?
```
include stack:

./project/foo.h:312
/usr/include/vector:344
```
Note that it does not mention column numbers as they aren't useful and is 
shorter.
Since we're already not 100% aligned with clang, why not have a more concise 
representation?



Comment at: unittests/clangd/DiagnosticsTests.cpp:739
+   {"/clangd-test/a.h", pos(0, 9)},
+   {"/clangd-test/c.h", pos(3, 6)}})));
+}

So the last location in the include stack is actually the original location of 
an error.
This is non-obvious design. Maybe move this into a separate field?



Comment at: unittests/clangd/TestTU.h:51
 
+  // Absolute path and contents of each file.
+  std::vector> AdditionalFiles;

Maybe use relative paths?
This would be consistent with `Filename` and `HeaderFilename`



Comment at: unittests/clangd/TestTU.h:52
+  // Absolute path and contents of each file.
+  std::vector> AdditionalFiles;
+

Why not `StringMap`?
This would be consistent with `buildTestFS` and disallow adding duplicates.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D60907: [OpenMP][WIP] Add math functions support in OpenMP offloading

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 196619.
gtbercea added a comment.

- Use macros.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_openmp_math.h

Index: lib/Headers/__clang_openmp_math.h
===
--- /dev/null
+++ lib/Headers/__clang_openmp_math.h
@@ -0,0 +1,95 @@
+/*=== __clang_openmp_math.h - Target OpenMP math support ---===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_OPENMP_MATH_H__
+#define __CLANG_OPENMP_MATH_H__
+
+#pragma omp declare target
+
+// Declarations of function in libomptarget
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+// POW
+float __kmpc_powf(float, float);
+double __kmpc_pow(double, double);
+long double __kmpc_powl(long double, long double);
+
+// LOG
+double __kmpc_log(double);
+float __kmpc_logf(float);
+double __kmpc_log10(double);
+float __kmpc_log10f(float);
+double __kmpc_log1p(double);
+float __kmpc_log1pf(float);
+double __kmpc_log2(double);
+float __kmpc_log2f(float);
+double __kmpc_logb(double);
+float __kmpc_logbf(float);
+
+// SIN
+float __kmpc_sinf(float);
+double __kmpc_sin(double);
+long double __kmpc_sinl(long double);
+
+// COS
+float __kmpc_cosf(float);
+double __kmpc_cos(double);
+long double __kmpc_cosl(long double);
+
+#if defined(__cplusplus)
+}
+#endif
+
+// Single argument functions
+#define __OPENMP_MATH_FUNC_1(__ty, __fn, __kmpc_fn)\
+  __attribute__((always_inline, used)) static __ty \
+  __fn(__ty __x) { \
+return __kmpc_fn(__x); \
+  }
+
+// Double argument functions
+#define __OPENMP_MATH_FUNC_2(__ty, __fn, __kmpc_fn)\
+  __attribute__((always_inline, used)) static __ty \
+  __fn(__ty __x, __ty __y) {   \
+return __kmpc_fn(__x, __y);\
+  }
+
+// POW
+__OPENMP_MATH_FUNC_2(float, powf, __kmpc_powf);
+__OPENMP_MATH_FUNC_2(double, pow, __kmpc_pow);
+__OPENMP_MATH_FUNC_2(long double, powl, __kmpc_powl);
+
+// LOG
+__OPENMP_MATH_FUNC_1(double, log, __kmpc_log);
+__OPENMP_MATH_FUNC_1(float, logf, __kmpc_logf);
+__OPENMP_MATH_FUNC_1(double, log10, __kmpc_log10);
+__OPENMP_MATH_FUNC_1(float, log10f, __kmpc_log10f);
+__OPENMP_MATH_FUNC_1(double, log1p, __kmpc_log1p);
+__OPENMP_MATH_FUNC_1(float, log1pf, __kmpc_log1pf);
+__OPENMP_MATH_FUNC_1(double, log2, __kmpc_log2);
+__OPENMP_MATH_FUNC_1(float, log2f, __kmpc_log2f);
+__OPENMP_MATH_FUNC_1(double, logb, __kmpc_logb);
+__OPENMP_MATH_FUNC_1(float, logbf, __kmpc_logbf);
+
+// SIN
+__OPENMP_MATH_FUNC_1(float, sinf, __kmpc_sinf);
+__OPENMP_MATH_FUNC_1(double, sin, __kmpc_sin);
+__OPENMP_MATH_FUNC_1(long double, sinl, __kmpc_sinl);
+
+// COS
+__OPENMP_MATH_FUNC_1(float, cosf, __kmpc_cosf);
+__OPENMP_MATH_FUNC_1(double, cos, __kmpc_cos);
+__OPENMP_MATH_FUNC_1(long double, cosl, __kmpc_cosl);
+
+#pragma omp end declare target
+
+#endif
+
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -31,6 +31,7 @@
   avxintrin.h
   bmi2intrin.h
   bmiintrin.h
+  __clang_openmp_math.h
   __clang_cuda_builtin_vars.h
   __clang_cuda_cmath.h
   __clang_cuda_complex_builtins.h
Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -48,6 +48,9 @@
   void AddCudaIncludeArgs(const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const;
 
+  void AddMathDeviceFunctions(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const;
+
   /// Emit an error if Version does not support the given Arch.
   ///
   /// If either Version or Arch is unknown, does not emit an error.  Emits at
@@ -165,6 +168,9 @@
   void AddCudaIncludeArgs(const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
+  void AddMathDeviceFunctions(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
+
   void addClangWarningOptions(llvm::opt::ArgStringList &CC1Args) const override;
   CXXStdli

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@sammccall

Having a separate tool is nice because it allows the client to make it 
plugable. clang-format sometimes changes options quite significantly and it can 
be nice if you have a choice which version to pick, otherwise it might be 
unable to read the configuration you have.


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

https://reviews.llvm.org/D53072



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

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Another example:

  int test() {
  
^
  }

Expected: a newline was added.
Actual: newline does not allow to be added.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61015#1478539 , @ilya-biryukov 
wrote:

> Why would we consider this a legitimate failure, rather than a programming 
> error?
>  Same argument could be made about any form of format-string-like functions, 
> e.g. `llvm::formatv` or `sprintf`.
>  Yet, they return strings and not `Expected` or their equivalent.


It's not that it's a legitimate failure so much as an invalid argument -- that 
is, the caller caused it and therefore should handle the failure.  For a 
standalone tool making the call, I'd argue they should treat it as a 
programming error and assert to crash.  However, a server that takes the input 
from, say, a user will want to propagate that back to the user.

As for `formatv` and `sprintf` -- I think the difference is that this has a 
more dynamic design.  Both of those take explicit arguments at the callsite, 
vs. TextGenerator which takes a `MatchResult`.  But, that's just a detail -- 
the key thing is that we're trying to support a usecase where the input may be 
flawed and we want to fail gracefully.  We do so at the cost of the added 
complexity of `Expected<>`.

Alternatives:

1. add a separate validation function.  But, this will complicate the design 
since we'd need to pass two functions rather than one.  That is, 
`TextGenerator` would need to be a pair of functions.
2. return an empty string on failure.  This has the typical tradeoffs of using 
a sentinel value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Input:

  int test() {
  }^

Expected:

  int test() {
  }
  ^

Actual:

  int test() {}
  ^


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I'd argue it's the server's job to validate the inputs in that case.

The code that landed so far clearly looks like a C++ DSL to describe 
transformations of the source. While it **can** be used a dependency in the 
server-side, I don't see why doing user-input checking should be done by the 
library, rather than the server itself.
User input would have to be transformed into the calls of the C++ API somehow, 
that looks like a proper layer to do the validation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D61112: AMDGPU: Enable _Float16

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Looks good to me.


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

https://reviews.llvm.org/D61112



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

r359193 - [OPENMP][AARCH64]Fix the test for declare simd, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Apr 25 07:04:37 2019
New Revision: 359193

URL: http://llvm.org/viewvc/llvm-project?rev=359193&view=rev
Log:
[OPENMP][AARCH64]Fix the test for declare simd, NFC.

Renamed function a01 in the test to fix possible problems with the git
hash match during testing.

Modified:
cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c

Modified: cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c?rev=359193&r1=359192&r2=359193&view=diff
==
--- cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c (original)
+++ cfe/trunk/test/OpenMP/declare_simd_aarch64_sve.c Thu Apr 25 07:04:37 2019
@@ -33,12 +33,12 @@ void foo_loop(double *x, float *y, int N
   // test integers
 
 #pragma omp declare simd notinbranch
-char a01(int x);
-// CHECK-DAG: _ZGVsMxv_a01
-// CHECK-NOT: a01
+char a01_fun(int x);
+// CHECK-DAG: _ZGVsMxv_a01_fun
+// CHECK-NOT: a01_fun
 
 static int *in;
 static char *out;
 void do_something() {
-  *out = a01(*in);
+  *out = a01_fun(*in);
 }


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

[PATCH] D48292: use modern type trait implementations when available

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think this broke the C++03 bots: 
http://green.lab.llvm.org/green/view/Libcxx/job/libc++%20and%20libc++abi%20trunk/CI_ARCH=64,CI_EXCEPTIONS=ON,CI_STD=c++03/103/consoleFull


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48292



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

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

> The FIXME in tests is fixed now.

... so instead of deleting the test, could you change it to show the current, 
better diagnostic?




Comment at: clang/include/clang/AST/ASTContext.h:818
+  /// For every comment not attached to any decl check if it should be attached
+  /// to any of \param Decls
+  ///

Please add a period.



Comment at: clang/include/clang/AST/ASTContext.h:818
+  /// For every comment not attached to any decl check if it should be attached
+  /// to any of \param Decls
+  ///

gribozavr wrote:
> Please add a period.
Please add a period.



Comment at: clang/lib/AST/ASTContext.cpp:494
+  // Explicitly not calling ExternalSource->ReadComments() as we're not
+  // interested in those.
+  ArrayRef RawComments = Comments.getComments();

Would be great to explain why (because we assume that the decls and their 
comments were parsed just now).  Otherwise the comment could enumerate a lot of 
other things that we are not calling here either...



Comment at: clang/lib/AST/ASTContext.cpp:564
+  for (const Decl *D : Decls) {
+D = adjustDeclToTemplate(D);
+if (!CanDeclHaveDocComment(D))

`getCommentForDecl` checks `D->isInvalidDecl()` first.



Comment at: clang/lib/AST/ASTContext.cpp:584
+// terminating early.
+for (auto CIt = RawComments.begin(); CIt != RawComments.end(); ++CIt) {
+  RawComment *C = *CIt;

Scanning all comments for every decl?  Isn't that O(n^2)?

Also logic duplication below.  I was expecting a call to 
`getRawCommentForDeclNoCache`, with an appropriate flag to disable loading 
external comments (it is a low-level API, users generally don't call it).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61103



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

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:2619
+  // Visibility attributes have no effect on symbols with internal linkage.
+  if (auto ND = dyn_cast(D)) {
+if (!ND->isExternallyVisible()) {

`const auto *ND` please.



Comment at: lib/Sema/SemaDeclAttr.cpp:2621
+if (!ND->isExternallyVisible()) {
+  S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+  return;

I think it would be more helpful for users if we introduced a new diagnostic 
here that explained why the attribute is being ignored, otherwise the 
diagnostic is somewhat indecipherable.



Comment at: lib/Sema/SemaDeclAttr.cpp:2622
+  S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+  return;
+}

We shouldn't early return here (it's not an error, just a warning), so that the 
other diagnostics can also trigger if needed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61097



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

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I'm okay with the PS4-specific bits.


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

https://reviews.llvm.org/D60274



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

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre added a comment.

I also think we could reduce the size of the tables.
To sum up how this is working right now:

1. The isOpenCLBuiltin(char* functionName) funcion is called to determine if a 
function is part of OpenCL builtin functions. If so, it returns its associated 
pair (index, number of signatures) in the OpenCLBuiltinDecl, otherwise it 
returns 0.
2. The OpenCLBuiltinDecl table is storing, for each OpenCL builtin function, 
the list of the possible signature associated for this function (e.g. cos can 
be "float cos(float)", "double cos(double)", ...). The available signatures are 
stored in the OpenCLSignature table. In the OpenCLBuiltinDecl are stored the 
indexes corresponding to the possible signature for each function.
3. The OpenCLSignature is storing the possible signatures.

E.g.: For the prototype float cos(float):

1. isOpenCLBuiltin("cos") is returning the pair (345, 18)
2. OpenCLBuiltinDecl[345] to  OpenCLBuiltinDecl[345+18] are the available 
signatures of the builtin function "cos". Let say OpenCLBuiltinDecl[346] is 
containing our "float cos(float)" prototype.  OpenCLBuiltinDecl[346] is 
containing the pair (123, 2), indexing the OpenCLSignature table and how many 
entries we have to read.
3. OpenCLSignature[123] is storing the return type of the function, so the 
"float" type. OpenCLSignature[124] is containing the type of the first 
argument, so the float type again. We are not looking further in the table 
because we are only looking for 2 types.

---

In the "float cos(float)" prototype, the information about the "float" type is 
duplicated. Plus, this "float" type is also the same as in the "float 
sin(float)" function. A third table, storing the different types, would discard 
duplicated definitions of types. The OpenCLSignature would store indexes of the 
required types, and the third table the type itself. This third table would be 
called OpenCLTypes, and would be as:

  struct OpenCLTypes {
// A type (e.g.: float, int, ...)
OpenCLTypeID ID;
// Size of the vector (if applicable)
unsigned VectorWidth;
// 0 if the type is not a pointer
unsigned isPointer;
// Address space of the pointer (if applicable)
clang::LangAS AS;
  }

and OpenCLSignature:

  struct OpenCLSignature {
  unsigned OpenCLTypesIndex
  }

---

Another way to save space would be to group functions. The "sin" and "cos" 
functions are similar (identical, we could say), regarding their use/ 
signature. However, they have two distinct lists of signatures in the 
OpenCLBuiltinDecl table. The consequence would be that isOpenCLBuiltin("cos") 
and isOpenCLBuiltin("sin") would give the same output. Such group of functions 
could be created manually by adding an attribute in the OpenCLBuiltins.td file, 
or automatically generated in the ClangOpenCLBuiltinEmitter.cpp file. The first 
solution would however make the feature potentially less understandable/ more 
complex to add new functions. The second solution is complex to implement/ 
could require a lot of time to process.


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

https://reviews.llvm.org/D60763



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1074
+  if (!RD->hasTrivialCopyAssignment())
+return true;
+  return false;

richard.townsend.arm wrote:
> Should this function also check for user-provided constructors?
I think it should: I speculatively added these two lines

  if (RD->hasUserDeclaredConstructor())
return true;

and it resolved the problem with `std::setw` I mentioned in the bug tracker 
(which means Electron could start). 


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

https://reviews.llvm.org/D60349



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

[PATCH] D61112: AMDGPU: Enable _Float16

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D61112



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

[PATCH] D61134: [Analyzer] Iterator Checkers - Do an early return after handling calls

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, gamesh411, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.

This patch is more of a fix than a real improvement: in `checkPostCall()` we 
should return immediately after finding the right call and handling it. This 
both saves unnecessary processing and double-handling calls by mistake.


Repository:
  rC Clang

https://reviews.llvm.org/D61134

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -566,9 +566,11 @@
   if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
 handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
  Call.getArgSVal(0));
-  } else {
-handleAssign(C, InstCall->getCXXThisVal());
+return;
   }
+
+  handleAssign(C, InstCall->getCXXThisVal());
+  return;
 } else if (isSimpleComparisonOperator(Op)) {
   const auto *OrigExpr = Call.getOriginExpr();
   if (!OrigExpr)
@@ -577,68 +579,107 @@
   if (const auto *InstCall = dyn_cast(&Call)) {
 handleComparison(C, OrigExpr, Call.getReturnValue(),
  InstCall->getCXXThisVal(), Call.getArgSVal(0), Op);
-  } else {
-handleComparison(C, OrigExpr, Call.getReturnValue(), Call.getArgSVal(0),
- Call.getArgSVal(1), Op);
+return;
   }
+
+  handleComparison(C, OrigExpr, Call.getReturnValue(), Call.getArgSVal(0),
+ Call.getArgSVal(1), Op);
+  return;
 } else if (isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) {
   if (const auto *InstCall = dyn_cast(&Call)) {
 if (Call.getNumArgs() >= 1) {
   handleRandomIncrOrDecr(C, Func->getOverloadedOperator(),
  Call.getReturnValue(),
  InstCall->getCXXThisVal(), Call.getArgSVal(0));
+  return;
 }
   } else {
 if (Call.getNumArgs() >= 2) {
   handleRandomIncrOrDecr(C, Func->getOverloadedOperator(),
  Call.getReturnValue(), Call.getArgSVal(0),
  Call.getArgSVal(1));
+  return;
 }
   }
 } else if (isIncrementOperator(Func->getOverloadedOperator())) {
   if (const auto *InstCall = dyn_cast(&Call)) {
 handleIncrement(C, Call.getReturnValue(), InstCall->getCXXThisVal(),
 Call.getNumArgs());
-  } else {
-handleIncrement(C, Call.getReturnValue(), Call.getArgSVal(0),
-Call.getNumArgs());
+return;
   }
+
+  handleIncrement(C, Call.getReturnValue(), Call.getArgSVal(0),
+  Call.getNumArgs());
+  return;
 } else if (isDecrementOperator(Func->getOverloadedOperator())) {
   if (const auto *InstCall = dyn_cast(&Call)) {
 handleDecrement(C, Call.getReturnValue(), InstCall->getCXXThisVal(),
 Call.getNumArgs());
-  } else {
-handleDecrement(C, Call.getReturnValue(), Call.getArgSVal(0),
-Call.getNumArgs());
+return;
   }
+
+  handleDecrement(C, Call.getReturnValue(), Call.getArgSVal(0),
+Call.getNumArgs());
+  return;
 }
   } else {
 if (const auto *InstCall = dyn_cast(&Call)) {
   if (isAssignCall(Func)) {
 handleAssign(C, InstCall->getCXXThisVal());
-  } else if (isClearCall(Func)) {
+return;
+  }
+
+  if (isClearCall(Func)) {
 handleClear(C, InstCall->getCXXThisVal());
-  } else if (isPushBackCall(Func) || isEmplaceBackCall(Func)) {
+return;
+  }
+
+  if (isPushBackCall(Func) || isEmplaceBackCall(Func)) {
 handlePushBack(C, InstCall->getCXXThisVal());
-  } else if (isPopBackCall(Func)) {
+return;
+  }
+
+  if (isPopBackCall(Func)) {
 handlePopBack(C, InstCall->getCXXThisVal());
-  } else if (isPushFrontCall(Func) || isEmplaceFrontCall(Func)) {
+return;
+  }
+
+  if (isPushFrontCall(Func) || isEmplaceFrontCall(Func)) {
 handlePushFront(C, InstCall->getCXXThisVal());
-  } else if (isPopFrontCall(Func)) {
+return;
+  }
+
+  if (isPopFrontCall(Func)) {
 handlePopFront(C, InstCall->getCXXThisVal());
-  } else if (isInsertCall(Func) || isEmplaceCall(Func)) {
+return;
+  }
+
+  if (isInsertCall(Func) || isEmplaceCall(Func)) {
 handleInsert(C, Call.getArgSVal(0));
-  } else if (isEraseCall(Func)) {
+return;
+  }
+
+  if (isEraseCall(Func)) {
 if (Call.getNumArgs()

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Sorry, coming back to this - line-filters are _inclusive_, so how do you 
indicate a 0-length range from the command line?


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

https://reviews.llvm.org/D54881



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

[PATCH] D59407: [clangd] Add RelationSlab

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: hokein.
sammccall added a comment.

Hi Nathan, sorry for the stall here, and for repeatedly going over the same 
issues.
The design space here is pretty complicated.

I think the conclusion of recent offline discussions is:

- refs and relations can be the same thing-ish when seen through a certain lens.
- Unifying them probably isn't space-prohibitive *if* we implement callgraph: 
it's something like +15% to overall memory usage, and storing callgraph in 
another way is likely to be similar.
- however unifying the concepts seems likely to be awkward in practice. LSP 
features seem to want one or the other but not both - symbolID is generally 
better than location if it's precise enough, and useless if not. We're storing 
redundant information (the location for the method override ref is the same as 
the declaration of the related symbol). We risk tying ourselves in knots 
maintaining a model that doesn't map well onto our problems.
- In terms of storage size, relation-major (`map>>` or so) seems like a quick win. But it's a 
small enough one that we should try to live without it first.

So if you can stomach it, I think

> **Approach 2: Add a RelationSlab storing (subject, predicate, object) 
> triples, intended for sparse relations***

is certainly fine with us (having spoken with @kadircet @ilya-biryukov 
@sammccall @gribozavr - @hokein is on vacation but not nearly as stubborn as I 
am!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:800
+  /// The result doesn't contain decls that don't have any comment attached.
+  std::unordered_map getRawCommentsForDeclsNoCache(
+  const std::unordered_map>

Why not DenseMap?



Comment at: clang/include/clang/AST/ASTContext.h:808
+  std::unordered_map
+  getRawCommentsForAnyRedecls(const std::vector &NDs) const;
+

Use ArrayRef.



Comment at: clang/lib/AST/ASTContext.cpp:303
+
+  return Result;
+}

I'm really worried about all the logic duplication here vs. existing code.



Comment at: clang/lib/AST/RawCommentList.cpp:366
+}
+  }
 }

Why is merging in `RawCommentList::addComment` not sufficient?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3180
   if (IncludeBriefComments) {
+// Try to get the comment if it wasn't provided
+if (!Comment)

There are only a couple of callers of this function, can we change them all to 
provide a comment if it exists?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3398
+  for (const auto *const ND : NDs) {
+if (const ObjCMethodDecl *M = dyn_cast(ND)) {
+  if (const ObjCPropertyDecl *PDecl = M->findPropertyDecl()) {

This method decl logic looks out of place here.  It should be pushed down into 
the core logic for attaching comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61104



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 196645.
aaron.ballman added a comment.
Herald added a subscriber: jdoerfert.

Rebased to master + small amount of additional functionality.

Since it seems this is non-controversial, I will probably commit after next 
week unless reviewers bring up concerns in the meantime.


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

https://reviews.llvm.org/D60910

Files:
  include/clang/AST/ASTDumperUtils.h
  include/clang/AST/DeclBase.h
  include/clang/AST/JSONNodeDumper.h
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/ASTConsumers.h
  include/clang/Frontend/FrontendOptions.h
  lib/AST/ASTDumper.cpp
  lib/AST/CMakeLists.txt
  lib/AST/JSONNodeDumper.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  test/AST/ast-dump-enum-json.cpp
  test/AST/ast-dump-if-json.cpp
  tools/clang-check/ClangCheck.cpp
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -316,8 +316,9 @@
   auto &CG = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
-ASTConsumers.push_back(CreateASTDumper(nullptr /*Dump to stdout.*/,
-   "", true, false, false));
+ASTConsumers.push_back(
+CreateASTDumper(nullptr /*Dump to stdout.*/, "", true, false, false,
+clang::FrontendOptions::AOF_Default));
 
   CI.getDiagnosticClient().BeginSourceFile(
   CI.getCompilerInstance().getLangOpts(),
Index: tools/clang-check/ClangCheck.cpp
===
--- tools/clang-check/ClangCheck.cpp
+++ tools/clang-check/ClangCheck.cpp
@@ -134,11 +134,11 @@
 if (ASTList)
   return clang::CreateASTDeclNodeLister();
 if (ASTDump)
-  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/,
-ASTDumpFilter,
+  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/, ASTDumpFilter,
 /*DumpDecls=*/true,
 /*Deserialize=*/false,
-/*DumpLookups=*/false);
+/*DumpLookups=*/false,
+clang::FrontendOptions::AOF_Default);
 if (ASTPrint)
   return clang::CreateASTPrinter(nullptr, ASTDumpFilter);
 return llvm::make_unique();
Index: test/AST/ast-dump-if-json.cpp
===
--- test/AST/ast-dump-if-json.cpp
+++ test/AST/ast-dump-if-json.cpp
@@ -0,0 +1,360 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++17 -ast-dump=json %s | FileCheck %s
+
+void func(int val) {
+  if (val)
+;
+
+// CHECK: "kind": "IfStmt",
+// CHECK-NEXT: "range": {"begin":{"col":3,"file":"{{.*}}","line":4},"end":{"col":5,"file":"{{.*}}","line":5}},
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"bool"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"int"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "DeclRefExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"int"},
+// CHECK-NEXT: "valueCategory": "lvalue"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "NullStmt",
+// CHECK-NEXT: "range": {"begin":{"col":5,"file":"{{.*}}","line":5},"end":{"col":5,"file":"{{.*}}","line":5}}
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: },
+
+  if (val)
+;
+  else
+;
+
+// CHECK: "kind": "IfStmt",
+// CHECK-NEXT: "range": {"begin":{"col":3,"file":"{{.*}}","line":43},"end":{"col":5,"file":"{{.*}}","line":46}},
+// CHECK-NEXT: "hasElse": true,
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":43},"end":{"col":7,"file":"{{.*}}","line":43}},
+// CHECK-NEXT: "type": {"qualType":"bool"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: 

[PATCH] D61136: [Analyzer] IteratorChecker - Ensure end()>=begin() and refactor begin and end symbol creation

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, gamesh411, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.

A container's `end()` is never less than its `begin(). This patch ensures it. 
Furthermore it refactors `begin` and `end` symbol creation by moving symbol 
conjuration into the `create...` functions. This way the functions' 
responsibilities are clearer and makes possible to add more functions handling 
these symbols (e.g. functions for handling the container's size) without code 
multiplication.


Repository:
  rC Clang

https://reviews.llvm.org/D61136

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -300,10 +300,13 @@
 SymbolRef getContainerBegin(ProgramStateRef State, const MemRegion *Cont);
 SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont);
 ProgramStateRef createContainerBegin(ProgramStateRef State,
- const MemRegion *Cont,
- const SymbolRef Sym);
+ const MemRegion *Cont, const Expr *E,
+ QualType T, const LocationContext *LCtx,
+ unsigned BlockCount);
 ProgramStateRef createContainerEnd(ProgramStateRef State, const MemRegion *Cont,
-   const SymbolRef Sym);
+   const Expr *E, QualType T,
+   const LocationContext *LCtx,
+   unsigned BlockCount);
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 const SVal &Val);
 ProgramStateRef setIteratorPosition(ProgramStateRef State, const SVal &Val,
@@ -1100,11 +1103,9 @@
   auto State = C.getState();
   auto BeginSym = getContainerBegin(State, ContReg);
   if (!BeginSym) {
-auto &SymMgr = C.getSymbolManager();
-BeginSym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
-C.getASTContext().LongTy, C.blockCount());
-State = assumeNoOverflow(State, BeginSym, 4);
-State = createContainerBegin(State, ContReg, BeginSym);
+State = createContainerBegin(State, ContReg, CE, C.getASTContext().LongTy,
+ C.getLocationContext(), C.blockCount());
+BeginSym = getContainerBegin(State, ContReg);
   }
   State = setIteratorPosition(State, RetVal,
   IteratorPosition::getPosition(ContReg, BeginSym));
@@ -1124,11 +1125,9 @@
   auto State = C.getState();
   auto EndSym = getContainerEnd(State, ContReg);
   if (!EndSym) {
-auto &SymMgr = C.getSymbolManager();
-EndSym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
-  C.getASTContext().LongTy, C.blockCount());
-State = assumeNoOverflow(State, EndSym, 4);
-State = createContainerEnd(State, ContReg, EndSym);
+State = createContainerEnd(State, ContReg, CE, C.getASTContext().LongTy,
+   C.getLocationContext(), C.blockCount());
+EndSym = getContainerEnd(State, ContReg);
   }
   State = setIteratorPosition(State, RetVal,
   IteratorPosition::getPosition(ContReg, EndSym));
@@ -1592,6 +1591,8 @@
   const MemRegion *Reg);
 SymbolRef rebaseSymbol(ProgramStateRef State, SValBuilder &SVB, SymbolRef Expr,
 SymbolRef OldSym, SymbolRef NewSym);
+ProgramStateRef ensureNonNegativeDiff(ProgramStateRef State, SymbolRef Sym1,
+  SymbolRef Sym2);
 
 bool isIteratorType(const QualType &Type) {
   if (Type->isPointerType())
@@ -1892,32 +1893,51 @@
 }
 
 ProgramStateRef createContainerBegin(ProgramStateRef State,
- const MemRegion *Cont,
- const SymbolRef Sym) {
+ const MemRegion *Cont, const Expr *E,
+ QualType T, const LocationContext *LCtx,
+ unsigned BlockCount) {
   // Only create if it does not exist
   const auto *CDataPtr = getContainerData(State, Cont);
+  if (CDataPtr && CDataPtr->getBegin())
+return State;
+
+  auto &SymMgr = State->getSymbolManager();
+  auto Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount, "begin");
+  State = assumeNoOverflow(State, Sym, 4);
+
   if (CDataPtr) {
-if (CDataPtr->getBegin()) {
-  return State;
-}
 const auto CData = CDataPtr->newBegin(Sym);
+if (auto EndSym = CDataPtr->getEnd()) {
+  State = ensureNonNegativeDiff(State, En

[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61015#1478586 , @ilya-biryukov 
wrote:

> I'd argue it's the server's job to validate the inputs in that case.
>
> The code that landed so far clearly looks like a C++ DSL to describe 
> transformations of the source. While it **can** be used a dependency in the 
> server-side, I don't see why doing user-input checking should be done by the 
> library, rather than the server itself.
>  User input would have to be transformed into the calls of the C++ API 
> somehow, that looks like a proper layer to do the validation.


The problem is that validation can't* be done in the abstract.  It has to be 
done with respect to a specific match result. Unfortunately, the server won't 
be layered directly on top of the call to the TextGenerator -- the rewrite rule 
is interposed between them.  That is, the client of the TG is the rewriterule 
and that's where the validation has to happen, but the TG is opaque to the 
rewrite rule, so it can't hardcode that validation logic. So, we'd need to 
change `TextGenerator` to bundle a validation function and string generator.  
Is it still worth it in that case?

*"can't" is a bit strong. I could imagine a design which allows full analysis 
and validation of rewrite rules before they are executed, but it would be far 
more sophisticated than the current design.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

r359200 - [OPENMP] Improved check for the linear dependency in the non-rectangular

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Apr 25 09:21:13 2019
New Revision: 359200

URL: http://llvm.org/viewvc/llvm-project?rev=359200&view=rev
Log:
[OPENMP] Improved check for the linear dependency in the non-rectangular
loop nests.

Added a checks that the initializer/condition expressions depend only
only of the single previous loop iteration variable.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/for_loop_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359200&r1=359199&r2=359200&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Apr 25 09:21:13 
2019
@@ -9172,6 +9172,8 @@ def err_omp_expected_private_copy_for_al
   "the referenced item is not found in any private clause on the same 
directive">;
 def err_omp_stmt_depends_on_loop_counter : Error<
   "the loop %select{initializer|condition}0 expression depends on the current 
loop control variable">;
+def err_omp_invariant_or_linear_dependancy : Error<
+  "expected loop invariant expression or ' * %0 + ' 
kind of expression">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=359200&r1=359199&r2=359200&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Apr 25 09:21:13 2019
@@ -4715,6 +4715,7 @@ class LoopCounterRefChecker final
   Sema &SemaRef;
   DSAStackTy &Stack;
   const ValueDecl *CurLCDecl = nullptr;
+  const ValueDecl *DepDecl = nullptr;
   bool IsInitializer = true;
 
 public:
@@ -4728,6 +4729,18 @@ public:
 return false;
   }
   const auto &&Data = Stack.isLoopControlVariable(VD);
+  if (DepDecl && Data.first) {
+SmallString<128> Name;
+llvm::raw_svector_ostream OS(Name);
+DepDecl->getNameForDiagnostic(OS, SemaRef.getPrintingPolicy(),
+  /*Qualified=*/true);
+SemaRef.Diag(E->getExprLoc(),
+ diag::err_omp_invariant_or_linear_dependancy)
+<< OS.str();
+return false;
+  }
+  if (Data.first)
+DepDecl = VD;
   return Data.first;
 }
 return false;
@@ -4742,16 +4755,27 @@ public:
 return false;
   }
   const auto &&Data = Stack.isLoopControlVariable(VD);
+  if (DepDecl && Data.first) {
+SmallString<128> Name;
+llvm::raw_svector_ostream OS(Name);
+DepDecl->getNameForDiagnostic(OS, SemaRef.getPrintingPolicy(),
+  /*Qualified=*/true);
+SemaRef.Diag(E->getExprLoc(),
+ diag::err_omp_invariant_or_linear_dependancy)
+<< OS.str();
+return false;
+  }
+  if (Data.first)
+DepDecl = VD;
   return Data.first;
 }
 return false;
   }
   bool VisitStmt(const Stmt *S) {
-for (const Stmt *Child : S->children()) {
-  if (Child && Visit(Child))
-return true;
-}
-return false;
+bool Res = true;
+for (const Stmt *Child : S->children())
+  Res = Child && Visit(Child) && Res;
+return Res;
   }
   explicit LoopCounterRefChecker(Sema &SemaRef, DSAStackTy &Stack,
  const ValueDecl *CurLCDecl, bool 
IsInitializer)

Modified: cfe/trunk/test/OpenMP/for_loop_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_loop_messages.cpp?rev=359200&r1=359199&r2=359200&view=diff
==
--- cfe/trunk/test/OpenMP/for_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/for_loop_messages.cpp Thu Apr 25 09:21:13 2019
@@ -293,6 +293,12 @@ int test_iteration_spaces() {
   for (ii = ii * 10 + 25; ii < ii / ii - 23; ii += 1)
 c[ii] = a[ii];
 
+// expected-error@+3 {{expected loop invariant expression or ' * 
ii + ' kind of expression}}
+#pragma omp for collapse(2)
+for (ii = 10 + 25; ii < 1000; ii += 1)
+  for (kk = ii * 10 + 25; kk < ii / ii - 23; kk += 1)
+;
+
 #pragma omp parallel
 // expected-note@+2  {{defined as firstprivate}}
 // expected-error@+2 {{loop iteration variable in the associated loop of 'omp 
for' directive may not be firstprivate, predetermined as private}}
@@ -603,7 +609,7 @@ int test_with_random_access_iterator() {
 
 template 
 class TC {
-  int ii;
+  int ii, iii;
 public:
   int dotest_lt(IT begin, IT end) {
 #pragma omp parallel
@@ -614,6 +620,14 @@ public:
 ;
 
 #pragma omp parallel
+// expected-error@+4 2 {{expected loop invarian

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 196654.
kadircet added a comment.

- Get rid of debug comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61126

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -186,7 +186,8 @@
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,25 @@
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Proto;
+void func() {}
+P$p^roto* create() {
+  fu$f^nc();
+  return nullptr;
+}
   )cpp");
 
   auto AST = TestTU::withCode(Test.code()).build();
-  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
-  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("p"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("p");
+EXPECT_THAT(Locs, ElementsAre(Sym("Proto", CodeGenLoc, CodeGenLoc)));
+  }
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("f"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("f");
+EXPECT_THAT(Locs, ElementsAre(Sym("func", CodeGenLoc, CodeGenLoc)));
+  }
 }
 
 TEST(LocateSymbol, All) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -346,28 +346,27 @@
 Index->lookup(QueryRequest, [&](const Symbol &Sym) {
   auto &R = Result[ResultIndex.lookup(Sym.ID)];
 
-  // Special case: if the AST yielded a definition, then it may not be
-  // the right *declaration*. Prefer the one from the index.
   if (R.Definition) { // from AST
-if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
-  R.PreferredDeclaration = *Loc;
+// In case of generated files we prefer to omit the definition in the
+// generated code.
 if (auto Loc = toLSPLocation(
 getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
 *MainFilePath))
   R.Definition = *Loc;
+
+// Special case: if the AST yielded a definition, then it may not be
+// the right *declaration*. Prefer the one from the index.
+if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
+  R.PreferredDeclaration = *Loc;
   } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
 
-if (Sym.CanonicalDeclaration) {
-  // Use merge logic to choose AST or index declaration.
-  // We only do this for declarations as definitions from AST
-  // is generally preferred (e.g. definitions in main file).
-  if (auto Loc = toLSPLocation(
-  getPreferredLocation(R.PreferredDeclaration,
-   Sym.CanonicalDeclaration, Scratch),
-  *MainFilePath))
-R.PreferredDeclaration = *Loc;
-}
+// Use merge logic to choose AST or index declaration.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration, Scratch),
+*MainFilePath))
+  R.PreferredDeclaration = *Loc;
   }
 });
   }


Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -186,7 +186,8 @@
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,25 @@
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Proto;
+void func() {}
+P$p^roto* create() {
+  fu$f^nc();
+  return nullptr;
+}
   )cpp");
 
   auto AST = TestTU::withCode(Test.code()).build();
-  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
-  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+  {
+auto Locs = clangd::lo

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

Ok, is there anything else should be included in this patch?
I saw more ideas about how to extend the functionality (custom pointers, 
fix-it, etc). I interpreted these ideas as nice-to-have things for the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 2 inline comments as done.
kadircet added a comment.

It has been a long time since I've proposed that change and even I forgot some 
of the high level details. Therefore, I wanted to sum up the state again so 
that we can decide on how to move forward.

Currently we have two different on-disk formats for index in clangd:

- Static index:
  - Merged single file, since merging is done before persistence, the data 
on-disk contains correct reference counts.
  - Produced by running SymbolCollector on each TU. Collector doesn't keep 
state between different TUs, merging is done on the caller-site.
  - No need to count references when loading, doesn't interact with 
`FileSymbols`.

- Background index:
  - Sharded multiple files.
  - Produced by running SymbolCollector on each TU, but instead of merging we 
separate those symbols into distinct files. Merging is done later on within 
`FileSymbols` which has no information about TUs.

FileSymbols is currently being used by BackgroundIndex and FileIndex(Dynamic 
Idx):

- In the case of BackgroundIndex, a symbol occurs at most twice(once in the 
header declaring it and once in the implementation file, if they are distinct), 
therefore when merging is done symbol references doesn't go above that.
- FileIndex doesn't perform de-duplication before-hand therefore it can perform 
reference counting while performing the merge. But currently it doesn't perform 
any merging.

As a result, changing `FileSymbols` would only effect `BackgroundIndex` and 
nothing else. Since merging occurs using the information in `RefSlabs` this can 
also be extended to benefit `FileIndex` and will most definitely be used
by any sort of indexing that performs merging over multiple files.

For unification purposes I would rather delete the `References` counting logic 
in `SymbolCollector` and perform it only at mergers(clangd-indexer and 
FileSymbols)




Comment at: clang-tools-extra/clangd/index/Background.h:113
 
+  // Note that FileSymbols counts References by incrementing it per each file
+  // mentioning the symbol, including headers. This contradicts with the

ilya-biryukov wrote:
> We should agree on and stick to a single behavior in both cases.
> I suggest keeping the current behavior for now (translation units). Why can't 
> we implement that?
Because `FileSymbols` only knows about file names and has no idea about whether 
a given file is TU or not. We might add some heuristics on the file extensions, 
or change SymbolCollector to count number of files(this would require 
maintaining a cache to de-duplicate references coming from the same files but 
originating from a different TU).



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:52
+  continue;
+// If this is the first time we see references to a symbol, reset its
+// Reference count, since filesymbols re-count number of references

ilya-biryukov wrote:
> The comment suggests there's something cheesy going on.
> Why would FileSymbols recompute the number of references? Can we avoid this 
> complicated logic?
This was the idea behind https://github.com/clangd/clangd/issues/23. Please see 
the main comment for a higher level discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> aganea wrote:
> > tstellar wrote:
> > > aganea wrote:
> > > > tstellar wrote:
> > > > > Same thing here too.
> > > > Not sure I understand, there're plenty of compiler-specific examples in 
> > > > the .cmake files. Is there an alternate way to fix this?
> > > I know there are some, but I don't think a warning like this is important 
> > > enough to fix with a compiler specific work-around.
> > You mean more like
> > ```
> > # Workaround for the gcc 6.1 bug 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
> > if (NOT MSVC)
> >   set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES 
> > COMPILE_FLAGS -Wno-unused-function)
> > endif()
> > ```
> > Or leave the warning? There's already an #ifdef below in 
> > LoopPassManagerTest.cpp but it's not at the right place, this simply 
> > corrects it.
> Ok, I see in this case you are just moving the ifdef out of the code and into 
> CMake.  I really don't like either approach, but I guess this is fine.  I 
> would limit the work-around to only known broken compilers though.  It looks 
> like this might be fixed in gcc 9.
Scoping this issue under GCC version range [6.1, 9.0)


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

https://reviews.llvm.org/D61046



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

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 196655.
aganea marked 3 inline comments as done.
aganea added a comment.

Please let me know if other changes are needed.


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

https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0 AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -459,6 +459,8 @@
   }
 }
 
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)HaveInterrupt3;
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3
+//warning: array subscript is above array bounds [-Warray-bounds]
+//if (NewBldVec[i] == NewBldVec[j]) {
+//~~~^
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: clang/trunk/unittests/Tooling/LookupTest.cpp
===
--- clang/trunk/unittests/Tooling/LookupTest.cpp
+++ clang/trunk/unittests/Tooling/LookupTest.cpp
@@ -217,8 +217,9 @@
   // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
   // it's not visible at [0].
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
-if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old") {
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };
   Visitor.runOver(R"(
 // a.h
Index: clang/trunk/unittests/AST/ASTImporterTest.cpp
===
--- clang/trunk/unittests/AST/ASTImporterTest.cpp
+++ clang/trunk/unit

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 196656.
sammccall added a comment.

Avoid removing lines when formatting after \n.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Format.cpp
  clangd/Format.h
  test/clangd/formatting.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/FormatTests.cpp

Index: unittests/clangd/FormatTests.cpp
===
--- /dev/null
+++ unittests/clangd/FormatTests.cpp
@@ -0,0 +1,264 @@
+//===-- FormatTests.cpp - Automatic code formatting tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Format.h"
+#include "Annotations.h"
+#include "SourceCode.h"
+#include "TestFS.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string afterTyped(llvm::StringRef CodeWithCursor,
+   llvm::StringRef Typed) {
+  Annotations Code(CodeWithCursor);
+  unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
+  auto Changes =
+  formatIncremental(Code.code(), Cursor, Typed,
+format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+  tooling::Replacements Merged;
+  for (const auto& R : Changes)
+if (llvm::Error E = Merged.add(R))
+  ADD_FAILURE() << llvm::toString(std::move(E));
+  auto NewCode = tooling::applyAllReplacements(Code.code(), Merged);
+  EXPECT_TRUE(bool(NewCode))
+  << "Bad replacements: " << llvm::toString(NewCode.takeError());
+  NewCode->insert(transformCursorPosition(Cursor, Changes), "^");
+  return *NewCode;
+}
+
+// We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
+void expectAfterNewline(const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+}
+void expectAfter(const char *Typed, const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+}
+
+TEST(FormatIncremental, SplitComment) {
+  expectAfterNewline(R"cpp(
+// this comment was
+^split
+)cpp",
+   R"cpp(
+// this comment was
+// ^split
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// trailing whitespace is not a split
+^   
+)cpp",
+   R"cpp(
+// trailing whitespace is not a split
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// extra   
+^ whitespace
+)cpp",
+   R"cpp(
+// extra
+// ^whitespace
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// triple
+^slash
+)cpp",
+   R"cpp(
+/// triple
+/// ^slash
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// editor continuation
+//^
+)cpp",
+   R"cpp(
+/// editor continuation
+/// ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// break before
+^ // slashes
+)cpp",
+   R"cpp(
+// break before
+^// slashes
+)cpp");
+
+
+  expectAfterNewline(R"cpp(
+int x;  // aligned
+^comment
+)cpp",
+   R"cpp(
+int x;  // aligned
+// ^comment
+)cpp");
+}
+
+TEST(FormatIncremental, Indentation) {
+  expectAfterNewline(R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp",
+   R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+  bar(baz(
+^
+)cpp",
+   R"cpp(
+void foo() {
+  bar(baz(
+  ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+^}
+)cpp",
+   R"cpp(
+void foo() {
+  ^
+}
+)cpp");
+
+  expectAfterNewline(R"cpp(
+class X {
+protected:
+^
+)cpp",
+   R"cpp(
+class X {
+ protected:
+  ^
+)cpp");
+
+// Mismatched brackets (1)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^}
+}
+)cpp");
+// Mismatched brackets (2)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^text}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^text}
+}
+)cpp");
+// Matched brackets
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^)
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^)
+}
+)cpp");
+}
+
+TEST(FormatIncremental, FormatPreviousLine) {
+  expectAfterNewline(R"cpp(
+void foo() {
+   untouched( );
+int x=2;
+^
+)cpp",
+ R"cpp(
+void foo() {
+   untouched( );
+   int x = 2;
+   ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+int x=untouched( );
+auto L = []{return;return;};
+^
+)cpp",
+   R"cpp(
+int x=untouched( );
+auto L = [] {
+  return;
+  return;
+};
+^
+)cpp");
+}
+
+TEST(FormatIncremental, Annoyances) {
+  // Don't remove newlines the user typed!
+  expectAfterNewline(R"cpp(
+int x(){
+
+
+^
+}
+

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D60605#1478579 , @ilya-biryukov 
wrote:

> Another example:
>
>   int test() {
>  
> ^
>   }
>
>
> Expected: a newline was added.
>  Actual: newline does not allow to be added.


Fixed.

In D60605#1478581 , @ilya-biryukov 
wrote:

> Input:
>
>   int test() {
>   }^
>
>
> Actual:
>
>   int test() {}
>   ^
>


I really do think this is the right thing in particular if you typed the brace.
It's visually confusing that hitting \n results in the cursor staying on the 
same line and the } moving.
And this interacts badly with the previous bug.
But can you see if this is something you could live with, and if not, explain 
why you don't want the {} formatted?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[clang-tools-extra] r359202 - [clangd] Use JSON streaming API for Trace rather than pasting strings. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Apr 25 09:37:07 2019
New Revision: 359202

URL: http://llvm.org/viewvc/llvm-project?rev=359202&view=rev
Log:
[clangd] Use JSON streaming API for Trace rather than pasting strings. NFC

Modified:
clang-tools-extra/trunk/clangd/Trace.cpp
clang-tools-extra/trunk/test/clangd/trace.test

Modified: clang-tools-extra/trunk/clangd/Trace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Trace.cpp?rev=359202&r1=359201&r2=359202&view=diff
==
--- clang-tools-extra/trunk/clangd/Trace.cpp (original)
+++ clang-tools-extra/trunk/clangd/Trace.cpp Thu Apr 25 09:37:07 2019
@@ -27,13 +27,14 @@ namespace {
 // Perhaps we should replace this by something that disturbs performance less.
 class JSONTracer : public EventTracer {
 public:
-  JSONTracer(llvm::raw_ostream &Out, bool Pretty)
-  : Out(Out), Sep(""), Start(std::chrono::system_clock::now()),
-JSONFormat(Pretty ? "{0:2}" : "{0}") {
+  JSONTracer(llvm::raw_ostream &OS, bool Pretty)
+  : Out(OS, Pretty ? 2 : 0), Start(std::chrono::system_clock::now()) {
 // The displayTimeUnit must be ns to avoid low-precision overlap
 // calculations!
-Out << R"({"displayTimeUnit":"ns","traceEvents":[)"
-<< "\n";
+Out.objectBegin();
+Out.attribute("displayTimeUnit", "ns");
+Out.attributeBegin("traceEvents");
+Out.arrayBegin();
 rawEvent("M", llvm::json::Object{
   {"name", "process_name"},
   {"args", llvm::json::Object{{"name", "clangd"}}},
@@ -41,7 +42,9 @@ public:
   }
 
   ~JSONTracer() {
-Out << "\n]}";
+Out.arrayEnd();
+Out.attributeEnd();
+Out.objectEnd();
 Out.flush();
   }
 
@@ -73,7 +76,7 @@ public:
 Contents["ts"] = Timestamp ? Timestamp : timestamp();
 Contents["tid"] = int64_t(TID);
 std::lock_guard Lock(Mu);
-rawEvent(Phase, std::move(Contents));
+rawEvent(Phase, Contents);
   }
 
 private:
@@ -145,13 +148,14 @@ private:
   // Record an event. ph and pid are set.
   // Contents must be a list of the other JSON key/values.
   void rawEvent(llvm::StringRef Phase,
-llvm::json::Object &&Event) /*REQUIRES(Mu)*/ {
+const llvm::json::Object &Event) /*REQUIRES(Mu)*/ {
 // PID 0 represents the clangd process.
-Event["pid"] = 0;
-Event["ph"] = Phase;
-Out << Sep
-<< llvm::formatv(JSONFormat, llvm::json::Value(std::move(Event)));
-Sep = ",\n";
+Out.object([&]{
+  Out.attribute("pid", 0);
+  Out.attribute("ph", Phase);
+  for (const auto& KV : Event)
+Out.attribute(KV.first, KV.second);
+});
   }
 
   // If we haven't already, emit metadata describing this thread.
@@ -177,11 +181,9 @@ private:
   }
 
   std::mutex Mu;
-  llvm::raw_ostream &Out /*GUARDED_BY(Mu)*/;
-  const char *Sep /*GUARDED_BY(Mu)*/;
+  llvm::json::OStream Out /*GUARDED_BY(Mu)*/;
   llvm::DenseSet ThreadsWithMD /*GUARDED_BY(Mu)*/;
   const llvm::sys::TimePoint<> Start;
-  const char *JSONFormat;
 };
 
 Key> JSONTracer::SpanKey;

Modified: clang-tools-extra/trunk/test/clangd/trace.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=359202&r1=359201&r2=359202&view=diff
==
--- clang-tools-extra/trunk/test/clangd/trace.test (original)
+++ clang-tools-extra/trunk/test/clangd/trace.test Thu Apr 25 09:37:07 2019
@@ -3,22 +3,25 @@
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
 main() {}"}}}
 # These assertions are a bit loose, to avoid brittleness.
-# CHECK: {"displayTimeUnit":"ns","traceEvents":[
 # CHECK: {
-# CHECK:   "args": {
-# CHECK: "File": "{{.*(/|\\)}}foo.c"
-# CHECK:   },
-# CHECK:   "name": "BuildPreamble",
-# CHECK:   "ph": "X",
+# CHECK:   "displayTimeUnit": "ns",
+# CHECK:   "traceEvents": [
+# CHECK: {
+# CHECK:   "ph": "X",
+# CHECK:   "name": "BuildPreamble",
+# CHECK:   "args": {
+# CHECK: "File": "{{.*(/|\\)}}foo.c"
+# CHECK:   },
+# CHECK: }
+# CHECK: {
+# CHECK:   "ph": "X",
+# CHECK:   "name": "BuildAST",
+# CHECK:   "args": {
+# CHECK: "File": "{{.*(/|\\)}}foo.c"
+# CHECK:   },
+# CHECK: }
+# CHECK:   ]
 # CHECK: }
-# CHECK: {
-# CHECK:   "args": {
-# CHECK: "File": "{{.*(/|\\)}}foo.c"
-# CHECK:   },
-# CHECK:   "name": "BuildAST",
-# CHECK:   "ph": "X",
-# CHECK: }
-# CHECK: },
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---


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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Looks good, but just a couple of questions.




Comment at: include/clang/AST/JSONNodeDumper.h:73
+  Indentation.clear();
+  OS << "\n" << Indentation << "}\n";
+  TopLevel = true;

Just curious, since you clear Indentation on the previous line, do you need it 
here?



Comment at: include/clang/Frontend/FrontendOptions.h:311
+  /// Specifies the output format of the AST.
+  enum ASTOutputFormat {
+AOF_Default,

Why can't you use the enum defined in `clang/AST/ASTDumperUtils.h`?  Seems to 
make the `dump()` call below unnecessarily ugly.


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

https://reviews.llvm.org/D60910



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/JSONNodeDumper.h:73
+  Indentation.clear();
+  OS << "\n" << Indentation << "}\n";
+  TopLevel = true;

hintonda wrote:
> Just curious, since you clear Indentation on the previous line, do you need 
> it here?
Definitely not required -- I had it there for consistency with other streaming 
output, but it's spurious. I'll remove.



Comment at: include/clang/Frontend/FrontendOptions.h:311
+  /// Specifies the output format of the AST.
+  enum ASTOutputFormat {
+AOF_Default,

hintonda wrote:
> Why can't you use the enum defined in `clang/AST/ASTDumperUtils.h`?  Seems to 
> make the `dump()` call below unnecessarily ugly.
I thought there would be a layering violation if AST and Frontend colluded on 
that, but I see now that Frontend already links against AST, so perhaps there 
isn't a layering violation after all?

I'll investigate to see if this can be cleaned up.


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

https://reviews.llvm.org/D60910



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: include/clang/Frontend/FrontendOptions.h:311
+  /// Specifies the output format of the AST.
+  enum ASTOutputFormat {
+AOF_Default,

aaron.ballman wrote:
> hintonda wrote:
> > Why can't you use the enum defined in `clang/AST/ASTDumperUtils.h`?  Seems 
> > to make the `dump()` call below unnecessarily ugly.
> I thought there would be a layering violation if AST and Frontend colluded on 
> that, but I see now that Frontend already links against AST, so perhaps there 
> isn't a layering violation after all?
> 
> I'll investigate to see if this can be cleaned up.
I think that Frontend uses AST, e.g., ASTContext, but not the other way around.


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

https://reviews.llvm.org/D60910



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

[PATCH] D61138: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

[Rong Xu via Phabricator via cfe-commits]

xur created this revision.
xur added reviewers: vsk, davidxl.
Herald added a subscriber: jfb.

Currently InstrProf lowering is not enabled for Clang PGO instrumentation in 
the new pass manager.
The following command "-fprofile-instr-generate -fexperimental-new-pass-manager 
..." is broken.
This CL enables InstrProf lowering pass for Clang PGO instrumentation in the 
new pass manager.


https://reviews.llvm.org/D61138

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/pgo-instrumentation.c

Index: clang/test/CodeGen/pgo-instrumentation.c
===
--- clang/test/CodeGen/pgo-instrumentation.c
+++ clang/test/CodeGen/pgo-instrumentation.c
@@ -1,20 +1,36 @@
 // Test if PGO instrumentation and use pass are invoked.
 //
 // Ensure Pass PGOInstrumentationGenPass is invoked.
-// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN
+// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN --check-prefix=CHECK-INSTRPROF
+// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s  -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-NEWPM --check-prefix=CHECK-INSTRPROF-NEWPM
 // CHECK-PGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass
+// CHECK-INSTRPROF: Frontend instrumentation-based coverage lowering
+// CHECK-PGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: PGOInstrumentationGen on
+// CHECK-INSTRPROF-NEWPM: Running pass: InstrProfiling on
 //
 // Ensure Pass PGOInstrumentationGenPass is not invoked.
 // RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NEWPM
 // CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NOT: PGOInstrumentationGenPass
+// CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NEWPM-NOT: Running pass: PGOInstrumentationGen on
+
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF-NEWPM
+// RUN: %clang_cc1 -O0 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
+// RUN: %clang_cc1 -O0 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF-NEWPM
+// CHECK-CLANG-INSTRPROF: Frontend instrumentation-based coverage lowering
+// CHECK-CLANG-INSTRPROF-NEWPM: Running pass: InstrProfiling on
 
 // Ensure Pass PGOInstrumentationUsePass is invoked.
 // RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotestir.profraw
 // RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t.profdata %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-INSTR-USE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t.profdata %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-INSTR-USE-NEWPM
 // CHECK-PGOUSEPASS-INVOKED-INSTR-USE: PGOInstrumentationUsePass
+// CHECK-PGOUSEPASS-INVOKED-INSTR-USE-NEWPM: Running pass: PGOInstrumentationUse on
 //
 // Ensure Pass PGOInstrumentationUsePass is not invoked.
 // RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotestclang.profraw
 // RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t.profdata %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-CLANG
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t.profdata %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-CLANG-NEWPM
 // CHECK-PGOUSEPASS-INVOKED-USE-CLANG-NOT: PGOInstrumentationUsePass
-
+// CHECK-PGOUSEPASS-INVOKED-USE-CLANG-NEWPM-NOT: Running pass: PGOInstrumentationUse on
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -57,6 +57,7 @@
 #include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/BoundsChecking.h"
 #include "llvm/Transforms/Instrumentation/GCOVProfiler.h"
+#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm

[PATCH] D61138: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

[Chandler Carruth via Phabricator via cfe-commits]

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

Awesome, LGTM

You might also update one of the instr prof tests to have two `RUN` lines, one 
for each pass manager. Feel free to do that (or not) and submit.


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

https://reviews.llvm.org/D61138



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

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 196671.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Add comments, add anon-namespacce test, tighten parsing rules slightly 
(`namespace ::...` is illegal)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61077

Files:
  clangd/CodeComplete.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -322,6 +322,74 @@
   EXPECT_EQ(IDs["foo"], 2u);
 }
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {
+  {
+  R"cpp(
+// Using directive resolved against enclosing namespaces.
+using namespace foo;
+namespace ns {
+using namespace bar;
+  )cpp",
+  {"ns", "", "bar", "foo", "ns::bar"},
+  },
+  {
+  R"cpp(
+// Don't include namespaces we've closed, ignore namespace aliases.
+using namespace clang;
+using std::swap;
+namespace clang {
+namespace clangd {}
+namespace ll = ::llvm;
+}
+namespace clang {
+  )cpp",
+  {"clang", ""},
+  },
+  {
+  R"cpp(
+// Using directives visible even if a namespace is reopened.
+// Ignore anonymous namespaces.
+namespace foo{ using namespace bar; }
+namespace foo{ namespace {
+  )cpp",
+  {"foo", "", "bar", "foo::bar"},
+  },
+  {
+  R"cpp(
+// Mismatched braces
+namespace foo{}
+}}}
+namespace bar{
+  )cpp",
+  {"bar", ""},
+  },
+  {
+  R"cpp(
+// Namespaces with multiple chunks.
+namespace a::b {
+  using namespace c::d;
+  namespace e::f {
+  )cpp",
+  {
+  "a::b::e::f",
+  "",
+  "a",
+  "a::b",
+  "a::b::c::d",
+  "a::b::e",
+  "a::c::d",
+  "c::d",
+  },
+  },
+  };
+  for (const auto& Case : Cases) {
+EXPECT_EQ(Case.second,
+  visibleNamespaces(Case.first, format::getLLVMStyle()))
+<< Case.first;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -18,6 +18,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "index/Index.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -2452,6 +2453,58 @@
   UnorderedElementsAre(Named("sym1"), Named("sym2")));
 }
 
+TEST(NoCompileCompletionTest, WithIndex) {
+  std::vector Syms = {func("xxx"), func("a::xxx"), func("ns::b::xxx"),
+  func("c::xxx"), func("ns::d::xxx")};
+  auto Results = completionsNoCompile(
+  R"cpp(
+// Current-scopes, unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::";
+  CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  Results = completionsNoCompile(
+  R"cpp(
+// All-scopes unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::")),
+   AllOf(Qualifier("c::"), Scope("c::")),
+   AllOf(Qualifier("d::"), Scope("ns::d::";
+  Results = completionsNoCompile(
+  R"cpp(
+// Qualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+b::xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  ElementsAre(AllOf(Qualifier(""), Scope("ns::b::";
+}
+
 } // namespace
 } // namespace clangd
 } // namespac

[PATCH] D60912: MS ABI: handle inline static data member and inline variable as template static data member

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: hans.
rnk added a comment.

+@hans, per https://bugs.llvm.org/show_bug.cgi?id=37903#c16


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

https://reviews.llvm.org/D60912



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D56571#1477992 , @jyu2 wrote:

> Please let me know if that is clang problem.


It's an orthogonal issue with Clang's integrated assembler. @compudj confirmed 
that setting `-no-integrated-as` solves the issue (which is what we do 
throughout the kernel, except in a few places mostly by accident).  I'm 
following up with @compudj off thread.

If we can get help w/ code review of:

- https://reviews.llvm.org/D58260
- https://reviews.llvm.org/D60887
- https://reviews.llvm.org/D60224

Then I think we'll have everything we need to land this patch, for the Linux 
kernel's consumption.


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

https://reviews.llvm.org/D56571



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

[PATCH] D42642: [CUDA] Detect installation in PATH

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.
Herald added a project: LLVM.

Just a quick heads-up - the `cuda-detect-path.cu` test fails on WSL/Ubuntu 
18.04:

  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ /usr/bin/python3.6 bin/llvm-lit -vv 
../clang/test/Driver/cuda-detect-path.cu
  llvm-lit: /mnt/f/svn/llvm/utils/lit/lit/llvm/config.py:341: note: using 
clang: /mnt/f/svn/buildWSL/bin/clang
  -- Testing: 1 tests, single process --
  FAIL: Clang :: Driver/cuda-detect-path.cu (1 of 1)
   TEST 'Clang :: Driver/cuda-detect-path.cu' FAILED 

  
  (output chopped)
  
  + env PATH=/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin 
/mnt/f/svn/buildWSL/bin/clang -v --target=i386-unknown-linux 
--sysroot=/mnt/f/svn/clang/test/Driver/no-cuda-there
  + /mnt/f/svn/buildWSL/bin/FileCheck 
/mnt/f/svn/clang/test/Driver/cuda-detect-path.cu --check-prefix SYMLINKS
  /mnt/f/svn/clang/test/Driver/cuda-detect-path.cu:82:14: error: SYMLINKS: 
expected string not found in input
  // SYMLINKS: Found CUDA installation: {{.*}}/Inputs/CUDA-symlinks/opt/cuda
   ^
  :1:1: note: scanning from here
  clang version 9.0.0 (trunk)
  ^
  :4:1: note: possible intended match here
  InstalledDir: /mnt/f/svn/buildWSL/bin
  ^
  
  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ env 
PATH=/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin 
/mnt/f/svn/buildWSL/bin/clang -v --target=i386-unknown-linux 
--sysroot=/mnt/f/svn/clang/test/Driver/no-cuda-there
  clang version 9.0.0 (trunk)
  Target: i386-unknown-linux
  Thread model: posix
  InstalledDir: /mnt/f/svn/buildWSL/bin
  aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ ls -l 
/mnt/f/svn/clang/test/Driver/Inputs/CUDA-symlinks/usr/bin
  total 0
  -rwxrwxrwx 1 aganea aganea 29 Feb  1  2018 ptxas


Repository:
  rL LLVM

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

https://reviews.llvm.org/D42642



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

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I feel something like `llvm::sys::path::PathListSeparator` would be nicer, 
since AFAIK any path list should be separated with semicolons on Windows and 
colons everywhere else.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61121



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

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

> Alternatively, this could just be a local ifdef _WIN32 in this function, or 
> should we generalize the existing EnvPathSeparator to e.g. a 
> llvm::sys::path::PathListSeparator?

I think it's clear as is. We're just reusing the environment path separator, 
which is used for INCLUDE, LIBS, PATH, and other path list environment 
variables.

> Does this change need a test?

If we don't already have tests for the colon separator, I don't think we need 
new tests to verify we get `:` on posix and `;` on Windows.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61121



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

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

[Reid Kleckner via Phabricator via cfe-commits]

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

In D61121#1479031 , @smeenai wrote:

> I feel something like `llvm::sys::path::PathListSeparator` would be nicer, 
> since AFAIK any path list should be separated with semicolons on Windows and 
> colons everywhere else.


I think the dominant use case for `EnvPathSeparator` is separating a list of 
paths in environment variables. We have other use cases for it in lists of 
paths that aren't environment variables (-print-search-dirs), but renaming it 
just seems like a minor improvement, which seems like it should be separate 
from this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61121



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

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.



Comment at: clangd/SourceCode.cpp:503
+case tok::l_brace:
+  if (State == NamespaceName) {
+// Parsed: namespace  {

kadircet wrote:
> I believe it is safe to ignore(just mark the opening brace) anonymous 
> namespaces here. Since there were no comments(and no test cases) just wanted 
> to make sure you did not miss that case.
Right, this was intended. Added a comment and a test.



Comment at: clangd/SourceCode.cpp:595
+  });
+  Found.erase(std::unique(Found.begin(), Found.end()), Found.end());
+  return Found;

kadircet wrote:
> `scopesForIndexQuery` already de-duplicates. Do you plan to have any other 
> users for the results of this function?
Only unit tests.
Seems a bit neater to always return canonical results, though.



Comment at: clangd/SourceCode.h:169
+/// The returned vector is always non-empty.
+/// - The first element is the namespace that encloses the point: a declaration
+///   near the point would be within this namespace.

kadircet wrote:
> Does the code ever make use of it?
This is passed into the `ScopeDistance`, and the first scope gets a quality 
boost. Added a comment.

I just noticed that getQueryScopes (not used on this codepath) only sometimes 
returns the scopes in the right order. Will fix in another patch.



Comment at: unittests/clangd/SourceCodeTests.cpp:325
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {

kadircet wrote:
> NIT: maybe switch to TEST_P ?
I find TEST_P much less readable and prefer to avoid it unless absolutely 
necessary.
Does it buy anything here?(


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61077



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

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 196674.
sammccall marked an inline comment as done.
sammccall added a comment.

Correctly handle absolutely qualifier (::foo::bar).
Fix seed scopes for proximity to be consistent with Sema case.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61077

Files:
  clangd/CodeComplete.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -322,6 +322,74 @@
   EXPECT_EQ(IDs["foo"], 2u);
 }
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {
+  {
+  R"cpp(
+// Using directive resolved against enclosing namespaces.
+using namespace foo;
+namespace ns {
+using namespace bar;
+  )cpp",
+  {"ns", "", "bar", "foo", "ns::bar"},
+  },
+  {
+  R"cpp(
+// Don't include namespaces we've closed, ignore namespace aliases.
+using namespace clang;
+using std::swap;
+namespace clang {
+namespace clangd {}
+namespace ll = ::llvm;
+}
+namespace clang {
+  )cpp",
+  {"clang", ""},
+  },
+  {
+  R"cpp(
+// Using directives visible even if a namespace is reopened.
+// Ignore anonymous namespaces.
+namespace foo{ using namespace bar; }
+namespace foo{ namespace {
+  )cpp",
+  {"foo", "", "bar", "foo::bar"},
+  },
+  {
+  R"cpp(
+// Mismatched braces
+namespace foo{}
+}}}
+namespace bar{
+  )cpp",
+  {"bar", ""},
+  },
+  {
+  R"cpp(
+// Namespaces with multiple chunks.
+namespace a::b {
+  using namespace c::d;
+  namespace e::f {
+  )cpp",
+  {
+  "a::b::e::f",
+  "",
+  "a",
+  "a::b",
+  "a::b::c::d",
+  "a::b::e",
+  "a::c::d",
+  "c::d",
+  },
+  },
+  };
+  for (const auto& Case : Cases) {
+EXPECT_EQ(Case.second,
+  visibleNamespaces(Case.first, format::getLLVMStyle()))
+<< Case.first;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -18,6 +18,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "index/Index.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -2452,6 +2453,71 @@
   UnorderedElementsAre(Named("sym1"), Named("sym2")));
 }
 
+TEST(NoCompileCompletionTest, WithIndex) {
+  std::vector Syms = {func("xxx"), func("a::xxx"), func("ns::b::xxx"),
+  func("c::xxx"), func("ns::d::xxx")};
+  auto Results = completionsNoCompile(
+  R"cpp(
+// Current-scopes, unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::";
+  CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  Results = completionsNoCompile(
+  R"cpp(
+// All-scopes unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::")),
+   AllOf(Qualifier("c::"), Scope("c::")),
+   AllOf(Qualifier("d::"), Scope("ns::d::";
+  Results = completionsNoCompile(
+  R"cpp(
+// Qualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+b::xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  ElementsAre(AllOf(Qualifier(""), Scope("ns::b::";
+  Results = completionsNoCompile(
+  R"cp

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 196676.
sammccall added a comment.

Fix test.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61077

Files:
  clangd/CodeComplete.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -322,6 +322,74 @@
   EXPECT_EQ(IDs["foo"], 2u);
 }
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {
+  {
+  R"cpp(
+// Using directive resolved against enclosing namespaces.
+using namespace foo;
+namespace ns {
+using namespace bar;
+  )cpp",
+  {"ns", "", "bar", "foo", "ns::bar"},
+  },
+  {
+  R"cpp(
+// Don't include namespaces we've closed, ignore namespace aliases.
+using namespace clang;
+using std::swap;
+namespace clang {
+namespace clangd {}
+namespace ll = ::llvm;
+}
+namespace clang {
+  )cpp",
+  {"clang", ""},
+  },
+  {
+  R"cpp(
+// Using directives visible even if a namespace is reopened.
+// Ignore anonymous namespaces.
+namespace foo{ using namespace bar; }
+namespace foo{ namespace {
+  )cpp",
+  {"foo", "", "bar", "foo::bar"},
+  },
+  {
+  R"cpp(
+// Mismatched braces
+namespace foo{}
+}}}
+namespace bar{
+  )cpp",
+  {"bar", ""},
+  },
+  {
+  R"cpp(
+// Namespaces with multiple chunks.
+namespace a::b {
+  using namespace c::d;
+  namespace e::f {
+  )cpp",
+  {
+  "a::b::e::f",
+  "",
+  "a",
+  "a::b",
+  "a::b::c::d",
+  "a::b::e",
+  "a::c::d",
+  "c::d",
+  },
+  },
+  };
+  for (const auto& Case : Cases) {
+EXPECT_EQ(Case.second,
+  visibleNamespaces(Case.first, format::getLLVMStyle()))
+<< Case.first;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -18,6 +18,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "index/Index.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -2452,6 +2453,71 @@
   UnorderedElementsAre(Named("sym1"), Named("sym2")));
 }
 
+TEST(NoCompileCompletionTest, WithIndex) {
+  std::vector Syms = {func("xxx"), func("a::xxx"), func("ns::b::xxx"),
+  func("c::xxx"), func("ns::d::xxx")};
+  auto Results = completionsNoCompile(
+  R"cpp(
+// Current-scopes, unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::";
+  CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  Results = completionsNoCompile(
+  R"cpp(
+// All-scopes unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::")),
+   AllOf(Qualifier("c::"), Scope("c::")),
+   AllOf(Qualifier("d::"), Scope("ns::d::";
+  Results = completionsNoCompile(
+  R"cpp(
+// Qualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+b::xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  ElementsAre(AllOf(Qualifier(""), Scope("ns::b::";
+  Results = completionsNoCompile(
+  R"cpp(
+// Absolutely qualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {

r359212 - Fix bug 37903:MS ABI: handle inline static data member and inline variable as template static data member

[Jennifer Yu via cfe-commits]

Author: jyu2
Date: Thu Apr 25 10:45:45 2019
New Revision: 359212

URL: http://llvm.org/viewvc/llvm-project?rev=359212&view=rev
Log:
Fix bug 37903:MS ABI: handle inline static data member and inline variable as 
template static data member

Added:
cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/test/Modules/initializers.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=359212&r1=359211&r2=359212&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Thu Apr 25 10:45:45 2019
@@ -467,7 +467,8 @@ CodeGenModule::EmitCXXGlobalVarDeclInitF
   } else if (auto *IPA = D->getAttr()) {
 OrderGlobalInits Key(IPA->getPriority(), PrioritizedCXXGlobalInits.size());
 PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
-  } else if (isTemplateInstantiation(D->getTemplateSpecializationKind())) {
+  } else if (isTemplateInstantiation(D->getTemplateSpecializationKind()) ||
+ getContext().GetGVALinkageForVariable(D) == GVA_DiscardableODR) {
 // C++ [basic.start.init]p2:
 //   Definitions of explicitly specialized class template static data
 //   members have ordered initialization. Other class template static data
@@ -481,6 +482,11 @@ CodeGenModule::EmitCXXGlobalVarDeclInitF
 // minor startup time optimization.  In the MS C++ ABI, there are no guard
 // variables, so this COMDAT key is required for correctness.
 AddGlobalCtor(Fn, 65535, COMDATKey);
+if (getTarget().getCXXABI().isMicrosoft() && COMDATKey) {
+  // In The MS C++, MS add template static data member in the linker
+  // drective.
+  addUsedGlobal(COMDATKey);
+}
   } else if (D->hasAttr()) {
 // SelectAny globals will be comdat-folded. Put the initializer into a
 // COMDAT group associated with the global, so the initializers get folded

Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp?rev=359212&view=auto
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp Thu Apr 25 
10:45:45 2019
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-pc-win32 -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -fms-extensions -emit-llvm 
-o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-pc-windows-msvc  -fms-extensions 
-emit-llvm -o - | FileCheck %s
+
+struct S {
+  S();
+  ~S();
+};
+
+template  struct __declspec(dllexport) ExportedTemplate {
+  static S s;
+};
+template  S ExportedTemplate::s;
+void useExportedTemplate(ExportedTemplate x) {
+  (void)x.s;
+}
+int f();
+namespace selectany_init {
+// MS don't put selectany static var in the linker directive, init routine
+// f() is not getting called if x is not referenced.
+int __declspec(selectany) x = f();
+inline int __declspec(selectany) x1 = f();
+}
+
+namespace explicit_template_instantiation {
+template  struct A { static  int x; };
+template  int A::x = f();
+template struct A;
+}
+
+namespace implicit_template_instantiation {
+template  struct A { static  int x; };
+template   int A::x = f();
+int g() { return A::x; }
+}
+
+
+template 
+struct X_ {
+  static T ioo;
+  static T init();
+};
+template  T X_::ioo = X_::init();
+template struct X_;
+
+template 
+struct X {
+  static T ioo;
+  static T init();
+};
+// template specialized static data don't need in llvm.used,
+// the static init routine get call from _GLOBAL__sub_I_ routines.
+template <> int X::ioo = X::init();
+template struct X;
+class a {
+public:
+  a();
+};
+// For the static var inside unnamed namespace, the object is local to TU.
+// No need to put static var in the linker directive.
+// The static init routine is called before main.
+namespace {
+template  class aj {
+public:
+  static a al;
+};
+template  a aj::al;
+class b : aj<3> {
+  void c();
+};
+void b::c() { al; }
+}
+
+// C++17, inline static data member also need to use
+struct A
+{
+  A();
+  ~A();
+};
+
+struct S1
+{
+  inline static A aoo; // C++17 inline variable, thus also a definition
+};
+
+int foo();
+inline int zoo = foo();
+inline static int boo = foo();
+
+
+// CHECK: @llvm.used = appending global [7 x i8*] [i8* bitcast (i32* 
@"?x1@selectany_init@@3HA" to i8*), i8* bitcast (i32* 
@"?x@?$A@H@explicit_template_instantiation@@2HA" to i8*), i8* bitcast (i32* 
@"?ioo@?$X_@H@@2HA" to i8*), i8* getelementptr inbounds (%struct.A, %struct.A* 
@"?aoo@S1@@2UA@@A", i32 0, i32 0), i

[clang-tools-extra] r359214 - [clangd] Optimize "don't include me" check.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Apr 25 10:47:07 2019
New Revision: 359214

URL: http://llvm.org/viewvc/llvm-project?rev=359214&view=rev
Log:
[clangd] Optimize "don't include me" check.

Summary:
llvm::Regex is really slow, and regex evaluation during preamble indexing was
showing up as 25% on a profile of clangd in a codebase with large preambles.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=359214&r1=359213&r2=359214&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu Apr 25 
10:47:07 2019
@@ -639,7 +639,7 @@ bool SymbolCollector::isSelfContainedHea
   return false;
 // This pattern indicates that a header can't be used without
 // particular preprocessor state, usually set up by another header.
-if (DontIncludeMePattern.match(SM.getBufferData(FID)))
+if (isDontIncludeMeHeader(SM.getBufferData(FID)))
   return false;
 return true;
   };
@@ -650,5 +650,36 @@ bool SymbolCollector::isSelfContainedHea
   return R.first->second;
 }
 
+// Is Line an #if or #ifdef directive?
+static bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+// Is Line an #error directive mentioning includes?
+static bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  if (! Line.startswith("error"))
+return false;
+  return Line.contains_lower("includ"); // Matches "include" or "including".
+}
+
+bool SymbolCollector::isDontIncludeMeHeader(llvm::StringRef Content) {
+  llvm::StringRef Line;
+  // Only sniff up to 100 lines or 10KB.
+  Content = Content.take_front(100*100);
+  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
+std::tie(Line, Content) = Content.split('\n');
+if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
+  return true;
+  }
+  return false;
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=359214&r1=359213&r2=359214&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Thu Apr 25 10:47:07 
2019
@@ -120,12 +120,8 @@ private:
 
   llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID);
   bool isSelfContainedHeader(FileID);
-  // Heuristic to detect headers that aren't self-contained, usually because
-  // they need to be included via an umbrella header. e.g. GTK matches this.
-  llvm::Regex DontIncludeMePattern = {
-  "^[ \t]*#[ \t]*if.*\n" // An #if, #ifndef etc directive, then
-  "[ \t]*#[ \t]*error.*include", // an #error directive mentioning 
"include"
-  llvm::Regex::Newline};
+  // Heuristically headers that only want to be included via an umbrella.
+  static bool isDontIncludeMeHeader(llvm::StringRef);
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;


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

[PATCH] D61120: [clangd] Optimize "don't include me" check.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 3 inline comments as done.
Closed by commit rCTE359214: [clangd] Optimize "don't include 
me" check. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61120?vs=196589&id=196681#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61120

Files:
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -639,7 +639,7 @@
   return false;
 // This pattern indicates that a header can't be used without
 // particular preprocessor state, usually set up by another header.
-if (DontIncludeMePattern.match(SM.getBufferData(FID)))
+if (isDontIncludeMeHeader(SM.getBufferData(FID)))
   return false;
 return true;
   };
@@ -650,5 +650,36 @@
   return R.first->second;
 }
 
+// Is Line an #if or #ifdef directive?
+static bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+// Is Line an #error directive mentioning includes?
+static bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  if (! Line.startswith("error"))
+return false;
+  return Line.contains_lower("includ"); // Matches "include" or "including".
+}
+
+bool SymbolCollector::isDontIncludeMeHeader(llvm::StringRef Content) {
+  llvm::StringRef Line;
+  // Only sniff up to 100 lines or 10KB.
+  Content = Content.take_front(100*100);
+  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
+std::tie(Line, Content) = Content.split('\n');
+if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
+  return true;
+  }
+  return false;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -120,12 +120,8 @@
 
   llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID);
   bool isSelfContainedHeader(FileID);
-  // Heuristic to detect headers that aren't self-contained, usually because
-  // they need to be included via an umbrella header. e.g. GTK matches this.
-  llvm::Regex DontIncludeMePattern = {
-  "^[ \t]*#[ \t]*if.*\n" // An #if, #ifndef etc directive, then
-  "[ \t]*#[ \t]*error.*include", // an #error directive mentioning 
"include"
-  llvm::Regex::Newline};
+  // Heuristically headers that only want to be included via an umbrella.
+  static bool isDontIncludeMeHeader(llvm::StringRef);
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -639,7 +639,7 @@
   return false;
 // This pattern indicates that a header can't be used without
 // particular preprocessor state, usually set up by another header.
-if (DontIncludeMePattern.match(SM.getBufferData(FID)))
+if (isDontIncludeMeHeader(SM.getBufferData(FID)))
   return false;
 return true;
   };
@@ -650,5 +650,36 @@
   return R.first->second;
 }
 
+// Is Line an #if or #ifdef directive?
+static bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+// Is Line an #error directive mentioning includes?
+static bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+return false;
+  Line = Line.ltrim();
+  if (! Line.startswith("error"))
+return false;
+  return Line.contains_lower("includ"); // Matches "include" or "including".
+}
+
+bool SymbolCollector::isDontIncludeMeHeader(llvm::StringRef Content) {
+  llvm::StringRef Line;
+  // Only sniff up to 100 lines or 10KB.
+  Content = Content.take_front(100*100);
+  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
+std::tie(Line, Content) = Content.split('\n');
+if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
+  return true;
+  }
+  return false;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -120,12 +120,8 @@
 
   llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID);
   bool isSelfContainedHeader(FileID);
-  // Heuristic to detect headers that aren't self-contained, usually 

[PATCH] D60912: MS ABI: handle inline static data member and inline variable as template static data member

[Jennifer Yu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGDeclCXX.cpp:470-471
 PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
-  } else if (isTemplateInstantiation(D->getTemplateSpecializationKind())) {
+  } else if (isTemplateInstantiation(D->getTemplateSpecializationKind()) ||
+ getContext().GetGVALinkageForVariable(D) == GVA_DiscardableODR) {
 // C++ [basic.start.init]p2:

rnk wrote:
> I think this can be simplified by removing the `isTemplateInstantiation` 
> check and then checking for GVA_DiscardableODR or GVA_StrongODR. That will 
> handle the cases of explicit template instantiation that you have below.
> 
> It's up to you if you want to implement the simplification. The current code 
> is correct, I don't believe it's possible to create a GVA_StrongODR global 
> variable with a dynamic initializer without template instantiation (phew).
Thanks Reid!  I also think GVA_StrongODR which is set for template 
instantiation, but I am not too sure about it.  Since we already handle 
template by calling isTemplateInstantiation, and add GVA_DiscardableODR.  I'd 
like to with this.  If we see other problem.  We can always add it up.

Thank you so much for your review.




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

https://reviews.llvm.org/D60912



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

r359215 - [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

[Rong Xu via cfe-commits]

Author: xur
Date: Thu Apr 25 10:52:43 2019
New Revision: 359215

URL: http://llvm.org/viewvc/llvm-project?rev=359215&view=rev
Log:
[PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass 
manager
Currently InstrProf lowering is not enabled for Clang PGO instrumentation in
the new pass manager. The following command
"-fprofile-instr-generate -fexperimental-new-pass-manager ..." is broken.

This CL enables InstrProf lowering pass for Clang PGO instrumentation in the
new pass manager.

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

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/pgo-instrumentation.c
cfe/trunk/test/Profile/c-general.c

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=359215&r1=359214&r2=359215&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Apr 25 10:52:43 2019
@@ -57,6 +57,7 @@
 #include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/BoundsChecking.h"
 #include "llvm/Transforms/Instrumentation/GCOVProfiler.h"
+#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
@@ -505,6 +506,21 @@ static Optional getGCOVOpti
   return Options;
 }
 
+static Optional
+getInstrProfOptions(const CodeGenOptions &CodeGenOpts,
+const LangOptions &LangOpts) {
+  if (!CodeGenOpts.hasProfileClangInstr())
+return None;
+  InstrProfOptions Options;
+  Options.NoRedZone = CodeGenOpts.DisableRedZone;
+  Options.InstrProfileOutput = CodeGenOpts.InstrProfileOutput;
+
+  // TODO: Surface the option to emit atomic profile counter increments at
+  // the driver level.
+  Options.Atomic = LangOpts.Sanitize.has(SanitizerKind::Thread);
+  return Options;
+}
+
 void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
   legacy::FunctionPassManager &FPM) {
   // Handle disabling of all LLVM passes, where we want to preserve the
@@ -659,17 +675,10 @@ void EmitAssemblyHelper::CreatePasses(le
   MPM.add(createStripSymbolsPass(true));
   }
 
-  if (CodeGenOpts.hasProfileClangInstr()) {
-InstrProfOptions Options;
-Options.NoRedZone = CodeGenOpts.DisableRedZone;
-Options.InstrProfileOutput = CodeGenOpts.InstrProfileOutput;
-
-// TODO: Surface the option to emit atomic profile counter increments at
-// the driver level.
-Options.Atomic = LangOpts.Sanitize.has(SanitizerKind::Thread);
+  if (Optional Options =
+  getInstrProfOptions(CodeGenOpts, LangOpts))
+MPM.add(createInstrProfilingLegacyPass(*Options, false));
 
-MPM.add(createInstrProfilingLegacyPass(Options, false));
-  }
   bool hasIRInstr = false;
   if (CodeGenOpts.hasProfileIRInstr()) {
 PMBuilder.EnablePGOInstrGen = true;
@@ -1056,6 +1065,9 @@ void EmitAssemblyHelper::EmitAssemblyWit
 if (CodeGenOpts.OptimizationLevel == 0) {
   if (Optional Options = getGCOVOptions(CodeGenOpts))
 MPM.addPass(GCOVProfilerPass(*Options));
+  if (Optional Options =
+  getInstrProfOptions(CodeGenOpts, LangOpts))
+MPM.addPass(InstrProfiling(*Options, false));
 
   // Build a minimal pipeline based on the semantics required by Clang,
   // which is just that always inlining occurs.
@@ -1120,6 +1132,11 @@ void EmitAssemblyHelper::EmitAssemblyWit
 PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
   MPM.addPass(GCOVProfilerPass(*Options));
 });
+  if (Optional Options =
+  getInstrProfOptions(CodeGenOpts, LangOpts))
+PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
+  MPM.addPass(InstrProfiling(*Options, false));
+});
 
   if (IsThinLTO) {
 MPM = PB.buildThinLTOPreLinkDefaultPipeline(

Modified: cfe/trunk/test/CodeGen/pgo-instrumentation.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pgo-instrumentation.c?rev=359215&r1=359214&r2=359215&view=diff
==
--- cfe/trunk/test/CodeGen/pgo-instrumentation.c (original)
+++ cfe/trunk/test/CodeGen/pgo-instrumentation.c Thu Apr 25 10:52:43 2019
@@ -1,20 +1,36 @@
 // Test if PGO instrumentation and use pass are invoked.
 //
 // Ensure Pass PGOInstrumentationGenPass is invoked.
-// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm 
-debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN
+// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm 
-debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PGOGENPASS-INVOKED-

[PATCH] D60912: MS ABI: handle inline static data member and inline variable as template static data member

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 closed this revision.
jyu2 added a comment.

committed rGc19f4f806972 
: Fix bug 
37903:MS ABI: handle inline static data member and inline variable as… 
(authored by jyu2).


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

https://reviews.llvm.org/D60912



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

[PATCH] D61138: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

[Rong Xu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359215: [PGO] Enable InstrProf lowering for Clang PGO 
instrumentation in the new pass… (authored by xur, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D61138?vs=196663&id=196683#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61138

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/pgo-instrumentation.c
  test/Profile/c-general.c

Index: test/Profile/c-general.c
===
--- test/Profile/c-general.c
+++ test/Profile/c-general.c
@@ -1,10 +1,12 @@
 // Test instrumentation of general constructs in C.
 
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument=clang | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument=clang -fexperimental-new-pass-manager | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOGEN %s
 
 // RUN: llvm-profdata merge %S/Inputs/c-general.proftext -o %t.profdata
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v3 | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v3 -fexperimental-new-pass-manager | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
 // Also check compatibility with older profiles.
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v1 | FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
 
Index: test/CodeGen/pgo-instrumentation.c
===
--- test/CodeGen/pgo-instrumentation.c
+++ test/CodeGen/pgo-instrumentation.c
@@ -1,20 +1,36 @@
 // Test if PGO instrumentation and use pass are invoked.
 //
 // Ensure Pass PGOInstrumentationGenPass is invoked.
-// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN
+// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN --check-prefix=CHECK-INSTRPROF
+// RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s  -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-NEWPM --check-prefix=CHECK-INSTRPROF-NEWPM
 // CHECK-PGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass
+// CHECK-INSTRPROF: Frontend instrumentation-based coverage lowering
+// CHECK-PGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: PGOInstrumentationGen on
+// CHECK-INSTRPROF-NEWPM: Running pass: InstrProfiling on
 //
 // Ensure Pass PGOInstrumentationGenPass is not invoked.
 // RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NEWPM
 // CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NOT: PGOInstrumentationGenPass
+// CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NEWPM-NOT: Running pass: PGOInstrumentationGen on
+
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
+// RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF-NEWPM
+// RUN: %clang_cc1 -O0 -fprofile-instrument=clang %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
+// RUN: %clang_cc1 -O0 -fprofile-instrument=clang %s -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF-NEWPM
+// CHECK-CLANG-INSTRPROF: Frontend instrumentation-based coverage lowering
+// CHECK-CLANG-INSTRPROF-NEWPM: Running pass: InstrProfiling on
 
 // Ensure Pass PGOInstrumentationUsePass is invoked.
 // RUN: llvm-profdata merge -o %t.profdata %S/

r359216 - creduce-clang-crash: add -F flag to grep to avoid interpreting string as regex

[Amy Huang via cfe-commits]

Author: akhuang
Date: Thu Apr 25 11:00:25 2019
New Revision: 359216

URL: http://llvm.org/viewvc/llvm-project?rev=359216&view=rev
Log:
creduce-clang-crash: add -F flag to grep to avoid interpreting string as regex

Modified:
cfe/trunk/utils/creduce-clang-crash.py

Modified: cfe/trunk/utils/creduce-clang-crash.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/creduce-clang-crash.py?rev=359216&r1=359215&r2=359216&view=diff
==
--- cfe/trunk/utils/creduce-clang-crash.py (original)
+++ cfe/trunk/utils/creduce-clang-crash.py Thu Apr 25 11:00:25 2019
@@ -187,7 +187,7 @@ class Reduce(object):
 (pipes.quote(not_cmd), crash_flag, quote_cmd(self.get_crash_cmd()))
 
 for msg in self.expected_output:
-  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+  output += 'grep -F %s t.log || exit 1\n' % pipes.quote(msg)
 
 write_to_script(output, self.testfile)
 self.check_interestingness()


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

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: martong, teemperor, aprantl, a_sidorin.
Herald added a subscriber: rnkovacs.

For a `CXXRecordDecl` the `RecordDeclBits` are stored in the `DeclContext`. 
Currently when we import the definition of a `CXXRecordDecl` via the 
`ASTImporter` we do not copy over this data.

We had a LLDB expression parsing bug where we would set it to not pass in 
registers:

  setArgPassingRestrictions(clang::RecordDecl::APK_CannotPassInRegs);

but when imported this setting would be lost. So dumping the `CXXRecordDecl` 
before importing we would see:

  CXXRecordDecl 0x7faaba292e50 <>  struct Bounds 
definition
  |-DefinitionData standard_layout has_user_declared_ctor can_const_default_init
  ...

but after importing it would show the following:

  CXXRecordDecl 0x7ff286823c50 <>  struct Bounds 
definition
  |-DefinitionData pass_in_registers standard_layout has_user_declared_ctor 
can_const_default_init
 ^

There will be a separate LLDB PR that will have a test that covers this and 
introduces a related fix for LLDB.

Note, we did not copy over any other of the `RecordDeclBits` since we don't 
have tests for those. We know that copying over 
`LoadedFieldsFromExternalStorage` would be a error and that may be the case for 
others as well.


https://reviews.llvm.org/D61140

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1767,6 +1767,9 @@
 ToData.HasDeclaredCopyAssignmentWithConstParam
   = FromData.HasDeclaredCopyAssignmentWithConstParam;
 
+// Copy over the data stored in RecordDeclBits
+ToCXX->setArgPassingRestrictions(FromCXX->getArgPassingRestrictions());
+
 SmallVector Bases;
 for (const auto &Base1 : FromCXX->bases()) {
   ExpectedType TyOrErr = import(Base1.getType());


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1767,6 +1767,9 @@
 ToData.HasDeclaredCopyAssignmentWithConstParam
   = FromData.HasDeclaredCopyAssignmentWithConstParam;
 
+// Copy over the data stored in RecordDeclBits
+ToCXX->setArgPassingRestrictions(FromCXX->getArgPassingRestrictions());
+
 SmallVector Bases;
 for (const auto &Base1 : FromCXX->bases()) {
   ExpectedType TyOrErr = import(Base1.getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

[Adrian Prantl via Phabricator via cfe-commits]

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

Could we test this by doing -dump-ast of From and To and FileCheck-ing the 
output?


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

https://reviews.llvm.org/D61140



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

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+for msg in self.expected_output:
+  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+

lebedev.ri wrote:
> >>! In D59725#1477362, @arichardson wrote:
> >>>! In D59725#1477042, @lebedev.ri wrote:
> >> I've stumbled into an issue with the script:
> >> It got a line: `clang: 
> >> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> >> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> >> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' 
> >> failed.`
> >> And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, 
> >> *LI) && "LCSSA required to run indvars!"' t.log || exit 1`
> >> But that doesn't work, escapes should be applied. it should be
> >> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
> >> indvars!\"' t.log || exit 1`
> > 
> > In my script I use `grep -F " + shlex.quote(self.args.crash_message)` which 
> > should escape both the message and ensure that the grep argument is not 
> > interpreted as a regex.
> 
> Great to hear!
> It appears that the script is currently more primitive than that currently 
> though.
Thanks; I added a `-F` flag (r359216), and it seems like that fixes the issue. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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

r359217 - DebugInfo: Fix bitrotted test case

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Thu Apr 25 11:11:48 2019
New Revision: 359217

URL: http://llvm.org/viewvc/llvm-project?rev=359217&view=rev
Log:
DebugInfo: Fix bitrotted test case

This test was updated with some CHECK suffix variants, but dropped
checking for the unsuffixed 'CHECK'

Modified:
cfe/trunk/test/CodeGenCXX/debug-info-class.cpp

Modified: cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class.cpp?rev=359217&r1=359216&r2=359217&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-class.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-class.cpp Thu Apr 25 11:11:48 2019
@@ -83,12 +83,12 @@ int main(int argc, char **argv) {
   return 0;
 }
 
-// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++98 %s -o - | FileCheck 
-check-prefix=CHECK98 %s
-// RUN: %clang_cc1 -triple i686-cygwin -emit-llvm -debug-info-kind=limited 
-fexceptions -std=c++98 %s -o - | FileCheck -check-prefix=CHECK98 %s
-// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++98 %s -o - | FileCheck 
-check-prefix=CHECK98 %s
-// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++11 %s -o - | FileCheck 
-check-prefix=CHECK11 %s
-// RUN: %clang_cc1 -triple i686-cygwin -emit-llvm -debug-info-kind=limited 
-fexceptions -std=c++11 %s -o - | FileCheck -check-prefix=CHECK11 %s
-// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++11 %s -o - | FileCheck 
-check-prefix=CHECK11 %s
+// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++98 %s -o - | FileCheck 
-check-prefix=CHECK98 -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple i686-cygwin -emit-llvm -debug-info-kind=limited 
-fexceptions -std=c++98 %s -o - | FileCheck -check-prefix=CHECK98 
-check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++98 %s -o - | FileCheck 
-check-prefix=CHECK98 -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++11 %s -o - | FileCheck 
-check-prefix=CHECK11 -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple i686-cygwin -emit-llvm -debug-info-kind=limited 
-fexceptions -std=c++11 %s -o - | FileCheck -check-prefix=CHECK11 
-check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm 
-debug-info-kind=limited -fexceptions -std=c++11 %s -o - | FileCheck 
-check-prefix=CHECK11 -check-prefix=CHECK %s
 
 // CHECK98: invoke {{.+}} @_ZN1BD1Ev(%class.B* %b)
 // CHECK98-NEXT: unwind label %{{.+}}, !dbg ![[EXCEPTLOC:.*]]
@@ -135,10 +135,9 @@ int main(int argc, char **argv) {
 // CHECK-SAME: identifier: "_ZTS1E"
 
 // CHECK: !DISubprogram(name: "func",{{.*}} scope: [[D]]
-// CHECK-SAME:  isDefinition: true
+// CHECK-SAME:  DISPFlagDefinition
 // CHECK-SAME:  declaration: [[D_FUNC_DECL:![0-9]*]]
 // CHECK: [[D_FUNC_DECL]] = !DISubprogram(name: "func",{{.*}} scope: [[D]]
-// CHECK-SAME:isDefinition: false
 
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "inner",{{.*}} 
line: 50
 // CHECK-NOT: DIFlagFwdDecl


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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

Ping @hfinkel @tra


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D59725: Additions to creduce script

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+for msg in self.expected_output:
+  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+

akhuang wrote:
> lebedev.ri wrote:
> > >>! In D59725#1477362, @arichardson wrote:
> > >>>! In D59725#1477042, @lebedev.ri wrote:
> > >> I've stumbled into an issue with the script:
> > >> It got a line: `clang: 
> > >> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> > >> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> > >> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run 
> > >> indvars!"' failed.`
> > >> And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, 
> > >> *LI) && "LCSSA required to run indvars!"' t.log || exit 1`
> > >> But that doesn't work, escapes should be applied. it should be
> > >> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
> > >> indvars!\"' t.log || exit 1`
> > > 
> > > In my script I use `grep -F " + shlex.quote(self.args.crash_message)` 
> > > which should escape both the message and ensure that the grep argument is 
> > > not interpreted as a regex.
> > 
> > Great to hear!
> > It appears that the script is currently more primitive than that currently 
> > though.
> Thanks; I added a `-F` flag (r359216), and it seems like that fixes the 
> issue. 
Thanks!
What about `shlex`? How do you know bash won't expand that as regex already?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

In D61140#1479111 , @aprantl wrote:

> Could we test this by doing -dump-ast of From and To and FileCheck-ing the 
> output?


+1 for this. Importing a simple record should test this? E.g. something like 
this:

  diff --git a/clang/test/Import/cxx-record-flags/Inputs/F.cpp 
b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  new file mode 100644
  index 000..8d1d88c632d
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  @@ -0,0 +1,11 @@
  +class FTrivial {
  +  int i;
  +};
  +
  +void foo();
  +
  +struct FNonTrivial {
  +  FNonTrivial(const FNonTrivial &F) { foo(); }
  +  int i;
  +};
  +
  diff --git a/clang/test/Import/cxx-record-flags/test.cpp 
b/clang/test/Import/cxx-record-flags/test.cpp
  new file mode 100644
  index 000..c51558b4793
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/test.cpp
  @@ -0,0 +1,15 @@
  +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
  +
  +// CHECK: FTrivial
  +// CHECK-NEXT: DefinitionData
  +// CHECK-SAME: pass_in_registers
  +
  +// CHECK: FNonTrivial
  +// CHECK-NEXT: DefinitionData
  +// CHECK-NOT: pass_in_registers
  +// CHECK: DefaultConstructor
  +
  +void expr() {
  +  FTrivial f1;
  +  FNonTrivial f2;
  +}


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

https://reviews.llvm.org/D61140



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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D60907#1479118 , @gtbercea wrote:

> Ping @hfinkel @tra


The last two comments in D47849  indicated 
exploration of a different approach, and one which still seems superior to this 
one. Can you please comment on why you're now pursuing this approach instead?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

In D47358#1438929 , @Quuxplusone wrote:

> Rebased. Added `_NOEXCEPT` to `upstream_resource()` and `options()` (this is 
> OK per [res.on.exception.handling]/5).


That's fine, but then we should have a test for that.
We have the macro `LIBCPP_ASSERT_NOEXCEPT` specifically for this purpose 
(libc++ - specific noexcept markings).


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

https://reviews.llvm.org/D47358



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

javed.absar wrote:
> t.p.northover wrote:
> > I think this should be fixed now.  It looks like technical debt from the 
> > fact that the instructions only fairly recently gained that feature after 
> > the intrinsics were implemented internally. There's no good way to justify 
> > the current semantics to someone unaware of that history.
> Not quite that really.  So the instruction did gain the feature recently like 
> you mentioned. But the ACLE/intrinsics were designed and agreed upon after it 
> and it was decided in ACLE discussions that the exta feature added complexity 
> that need not be exposed at ACLE level yet. No big use case to justify 
> complicating the ACLE MTE spec yet. Directly assembly can use that 
> instruction though.
I think the ACLE decision was a mistake, but since it happened I withdraw this 
request. I expect (and hope) far more people will use these through ACLE than 
as compiler-specific builtins.


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

https://reviews.llvm.org/D60485



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