[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

> clang-cl isn't supposed to do (explicit) registry accesses when you hold it 
> right (pass in -fms-compatibility-version etc). Have you seen registry access 
> costs, or is that speculation?

Please see this log: F7268226: clang-cl-log.zip 
 - the child `clang-cl -cc1` takes about 
~117ms until it gets into the global initializers. This is on my Haswell PC. On 
the Skylake, this takes "only" ~60ms.
This probably explains why Ninja is slower on the Skylake when using `clang-cl` 
as a compiler. There should be a shorter codepath maybe when only a single .cpp 
is being compiled, and avoid running the child process.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166351.
nickdesaulniers added a comment.

- warn only if -pedantic, add gcc bug test cases


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,24 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,14 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode &&
+!S.Diags.getDiagnosticOptions().Pedantic &&
+DS.getTypeSpecType() == DeclSpec::TST_typeofExpr))  {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,24 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,14 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode &&
+!S.Diags.getDiagnosticOptions().Pedantic &&
+DS.getTypeSpecType() == DeclSpec::TST_typeofExpr))  {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

Closed by r342708.


Repository:
  rC Clang

https://reviews.llvm.org/D52021



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

r342708 - [clang-format] Do not merge short case labels if followed by a block.

[Owen Pan via cfe-commits]

Author: owenpan
Date: Thu Sep 20 20:46:36 2018
New Revision: 342708

URL: http://llvm.org/viewvc/llvm-project?rev=342708&view=rev
Log:
[clang-format] Do not merge short case labels if followed by a block.

Do not allow short case labels on a single line if the label is followed by a
left brace.

Fixes PR38926.

Modified:
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=342708&r1=342707&r2=342708&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Thu Sep 20 20:46:36 2018
@@ -428,6 +428,8 @@ private:
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
+  return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=342708&r1=342707&r2=342708&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Sep 20 20:46:36 2018
@@ -1241,6 +1241,30 @@ TEST_F(FormatTest, ShortCaseLabels) {
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {


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

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, rjmccall.
Herald added a subscriber: dexonsmith.

A significant number of internal C users have been complaining about the lack 
of support for fixed enums. We already support this in Objective-C mode, as 
well as in C mode with -fms-extensions. Supporting this in C makes us more 
consistent, and provides a useful feature to C users!

If there is some doubt as to whether we want this, I can start a thread on 
cfe-dev for the wider audience.

rdar://43831380

Thanks for taking a look!


Repository:
  rC Clang

https://reviews.llvm.org/D52339

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Sema/fixed-enum.c
  clang/test/SemaObjC/enum-fixed-type.m

Index: clang/test/SemaObjC/enum-fixed-type.m
===
--- clang/test/SemaObjC/enum-fixed-type.m
+++ clang/test/SemaObjC/enum-fixed-type.m
@@ -1,9 +1,14 @@
 // RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc %s
 
 #if !__has_feature(objc_fixed_enum)
 #  error Enumerations with a fixed underlying type are not supported
 #endif
 
+#if !__has_extension(cxx_fixed_enum)
+#  error Enumerations with a fixed underlying type are not supported
+#endif
+
 typedef long Integer;
 
 typedef enum : Integer { Enumerator1, Enumerator2 } Enumeration;
Index: clang/test/Sema/fixed-enum.c
===
--- /dev/null
+++ clang/test/Sema/fixed-enum.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -Weverything -xc++ -std=c++11 -DCXX11 -verify %s
+// RUN: %clang_cc1 -Weverything -xc++ -std=c++03 -DCXX03 -verify %s
+// RUN: %clang_cc1 -Weverything -xobjective-c -DOBJC -verify %s
+// RUN: %clang_cc1 -Weverything -std=c11 -xc -DC11 -verify %s
+// RUN: %clang_cc1 -Weverything -std=c11 -xc -fms-extensions -DMS -verify %s
+
+enum X : int {e};
+#if defined(CXX11)
+// expected-warning@-2{{enumeration types with a fixed underlying type are incompatible with C++98}}
+#elif defined(CXX03)
+// expected-warning@-4{{enumeration types with a fixed underlying type are a C++11 extension}}
+#elif defined(OBJC)
+// expected-no-diagnostics
+#elif defined(C11)
+// expected-warning@-8{{enumeration types with a fixed underlying type are a Clang extension}}
+#elif defined(MS)
+// expected-warning@-10{{enumeration types with a fixed underlying type are a Microsoft extension}}
+#endif
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -4153,15 +4153,11 @@
   // Enum definitions should not be parsed in a trailing-return-type.
   bool AllowDeclaration = DSC != DeclSpecContext::DSC_trailing;
 
-  bool AllowFixedUnderlyingType = AllowDeclaration &&
-(getLangOpts().CPlusPlus11 || getLangOpts().MicrosoftExt ||
- getLangOpts().ObjC2);
-
   CXXScopeSpec &SS = DS.getTypeSpecScope();
   if (getLangOpts().CPlusPlus) {
 // "enum foo : bar;" is not a potential typo for "enum foo::bar;"
 // if a fixed underlying type is allowed.
-ColonProtectionRAIIObject X(*this, AllowFixedUnderlyingType);
+ColonProtectionRAIIObject X(*this, AllowDeclaration);
 
 CXXScopeSpec Spec;
 if (ParseOptionalCXXScopeSpecifier(Spec, nullptr,
@@ -4183,7 +4179,7 @@
 
   // Must have either 'enum name' or 'enum {...}'.
   if (Tok.isNot(tok::identifier) && Tok.isNot(tok::l_brace) &&
-  !(AllowFixedUnderlyingType && Tok.is(tok::colon))) {
+  !(AllowDeclaration && Tok.is(tok::colon))) {
 Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_brace;
 
 // Skip the rest of this declarator, up until the comma or semicolon.
@@ -4216,7 +4212,7 @@
 
   // Parse the fixed underlying type.
   bool CanBeBitfield = getCurScope()->getFlags() & Scope::ClassScope;
-  if (AllowFixedUnderlyingType && Tok.is(tok::colon)) {
+  if (AllowDeclaration && Tok.is(tok::colon)) {
 bool PossibleBitfield = false;
 if (CanBeBitfield) {
   // If we're in class scope, this can either be an enum declaration with
@@ -4276,13 +4272,15 @@
   SourceRange Range;
   BaseType = ParseTypeName(&Range);
 
-  if (getLangOpts().CPlusPlus11) {
-Diag(StartLoc, diag::warn_cxx98_compat_enum_fixed_underlying_type);
-  } else if (!getLangOpts().ObjC2) {
-if (getLangOpts().CPlusPlus)
-  Diag(StartLoc, diag::ext_cxx11_enum_fixed_underlying_type) << Range;
+  if (!getLangOpts().ObjC2) {
+if (getLangOpts().CPlusPlus11)
+  Diag(StartLoc, diag::warn_cxx98_compat_enum_fixed_underlying_type);
+else if (getLangOpts().CPlusPlus)
+  Diag(StartLoc, diag::ext_cxx11_enum_fixed_underlying_type);
+else if (getLangOpts().MicrosoftExt)
+  Diag(StartLoc, diag::ext_ms_c_enum_fixed_underlying_type);
 else
- 

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

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: JonasToth, george.karpenkov, shuaiwang, aaron.ballman.
Herald added subscribers: cfe-commits, Szelethus, a.sidorin, mgorny.

Conditionally compile the parts of clang-tidy which depend on the static
analyzer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334

Files:
  CMakeLists.txt
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidy.cpp
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp

Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -534,10 +534,12 @@
 static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
 ModernizeModuleAnchorSource;
 
+#if CLANG_ENABLE_STATIC_ANALYZER
 // This anchor is used to force the linker to link the MPIModule.
 extern volatile int MPIModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
 MPIModuleAnchorSource;
+#endif
 
 // This anchor is used to force the linker to link the PerformanceModule.
 extern volatile int PerformanceModuleAnchorSource;
Index: clang-tidy/tool/CMakeLists.txt
===
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -29,7 +29,6 @@
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
-  clangTidyMPIModule
   clangTidyObjCModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
@@ -39,6 +38,12 @@
   clangToolingCore
   )
 
+if(CLANG_ENABLE_STATIC_ANALYZER)
+  target_link_libraries(clang-tidy PRIVATE
+clangTidyMPIModule
+  )
+endif()
+
 install(PROGRAMS clang-tidy-diff.py
   DESTINATION share/clang
   COMPONENT clang-tidy)
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -34,8 +34,10 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Rewrite/Frontend/FixItRewriter.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
+#if CLANG_ENABLE_STATIC_ANALYZER
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#endif
 #include "clang/Tooling/DiagnosticsYaml.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/ReplacementsYaml.h"
@@ -56,6 +58,7 @@
 namespace tidy {
 
 namespace {
+#if CLANG_ENABLE_STATIC_ANALYZER
 static const char *AnalyzerCheckNamePrefix = "clang-analyzer-";
 
 class AnalyzerDiagnosticConsumer : public ento::PathDiagnosticConsumer {
@@ -87,6 +90,7 @@
 private:
   ClangTidyContext &Context;
 };
+#endif
 
 class ErrorReporter {
 public:
@@ -296,6 +300,7 @@
   }
 }
 
+#if CLANG_ENABLE_STATIC_ANALYZER
 static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts,
  AnalyzerOptionsRef AnalyzerOptions) {
   StringRef AnalyzerPrefix(AnalyzerCheckNamePrefix);
@@ -339,6 +344,7 @@
   }
   return List;
 }
+#endif
 
 std::unique_ptr
 ClangTidyASTConsumerFactory::CreateASTConsumer(
@@ -380,6 +386,7 @@
   if (!Checks.empty())
 Consumers.push_back(Finder->newASTConsumer());
 
+#if CLANG_ENABLE_STATIC_ANALYZER
   AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();
   AnalyzerOptions->CheckersControlList =
   getCheckersControlList(Context, Context.canEnableAnalyzerAlphaCheckers());
@@ -395,6 +402,7 @@
 new AnalyzerDiagnosticConsumer(Context));
 Consumers.push_back(std::move(AnalysisConsumer));
   }
+#endif
   return llvm::make_unique(
   std::move(Consumers), std::move(Profiling), std::move(Finder),
   std::move(Checks));
@@ -407,9 +415,11 @@
   CheckNames.push_back(CheckFactory.first);
   }
 
+#if CLANG_ENABLE_STATIC_ANALYZER
   for (const auto &AnalyzerCheck : getCheckersControlList(
Context, Context.canEnableAnalyzerAlphaCheckers()))
 CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first);
+#endif
 
   std::sort(CheckNames.begin(), CheckNames.end());
   return CheckNames;
Index: clang-tidy/CMakeLists.txt
===
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -21,12 +21,17 @@
   clangLex
   clangRewrite
   clangSema
-  clangStaticAnalyzerCore
-  clangStaticAnalyzerFrontend
   clangTooling
   clangToolingCore
   )
 
+if(CLANG_ENABLE_STATIC_ANALYZER)
+target_link_libraries(clangTidy PRIVATE
+  clangStaticAnalyzerCore
+  clangStaticAnalyzerFrontend
+)
+endif()
+
 add_subdirectory(android)
 add_subdirectory(abseil)
 add_subdirectory(boost)
@@ -39,7 +44,9 @@
 add_subdirectory(llvm)
 add_subdirectory(misc)
 add_subdirectory(modernize)
+if(CLANG_ENABLE_STATIC_ANALYZER)
 add_subdirectory(mpi)
+endif()
 add_subdirectory(objc)
 add_subdirectory(performance)
 add_subdirectory(plugin)
Index: CMakeLists.txt
===
-

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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Thanks for pointing out my error! Ignoring the implementation for a moment, do 
you think this is a good idea or will it produce to many false positives? 
Releasable/relockable scoped capabilities that I have seen keep track of the 
status, so it makes sense, but maybe there are others out there.




Comment at: lib/Analysis/ThreadSafety.cpp:929
+  return Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc());
+Locked = true;
+

delesley wrote:
> It's been a while since I last looked at this code, but I don't think you can 
> use mutable fields in a FactEntry.  The analysis creates a FactSet for each 
> program point, but each FactSet simply has pointers (FactIDs) for the 
> underlying FactEntries.  If you change the definition of a FactEntry, it will 
> change that definition for every program point.  So if you do an unlock on 
> one side of a branch, and change the underlying FactEntry to reflect that, 
> then analysis will then think you have also done the unlock on the other side 
> of the branch.  
> 
> Any changes should always be done by adding or removing entries from the 
> FactSet, not by mutating the underlying FactEntries.
> 
That explains why there are problems with back edges. Now that you say it I'm 
actually aware of how the `FactManager` and `FactSet` work, but apparently I 
didn't consider that here.

Would you mind if I make a separate change that returns only `const FactEntry*` 
from the `FactManager`? Maybe that makes it harder to write wrong code in the 
future.


Repository:
  rC Clang

https://reviews.llvm.org/D51187



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+  // template
+  // void f(T func) {
+  //   func();
+  //   ^~~
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);

lebedev.ri wrote:
> Bikeshedding: i do not understand the idea behind `std::invoke`.
> Why is the new version better/preferred?
It generically handles all the various kinds of "callable" objects (lambdas, 
functors, function pointers, pointer to member functions, etc).



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:97-99
+  std::ostringstream Replace;
+  Replace << "::std::invoke(" << Param
+  << (Functor->getNumArgs() == 0 ? "" : ", ") << OriginalParams;

This should use a `Twine` instead.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h:19
+
+/// Use std::invoke to call callable objects in generic code
+///

Missing full stop at the end of the comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



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

[PATCH] D52331: [Index] Report specialization bases as references when IndexImplicitInstantiation is true

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: akyrtzi, arphaman.
Herald added a subscriber: cfe-commits.

  template  struct B {};
  template  struct D : B {}; // `B` was not reported as a 
reference

This patch fixes this.


Repository:
  rC Clang

https://reviews.llvm.org/D52331

Files:
  lib/Index/IndexTypeSourceInfo.cpp
  test/Index/index-template-specialization.cpp


Index: test/Index/index-template-specialization.cpp
===
--- test/Index/index-template-specialization.cpp
+++ test/Index/index-template-specialization.cpp
@@ -9,11 +9,21 @@
   foo.f(0);
 }
 
+template 
+struct B {};
+
+template 
+struct D : B {};
+
 // FIXME: if c-index-test uses OrigD for symbol info, refererences below should
 // refer to template specialization decls.
 // RUN: env CINDEXTEST_INDEXIMPLICITTEMPLATEINSTANTIATIONS=1 c-index-test 
-index-file %s | FileCheck %s
 // CHECK: [indexDeclaration]: kind: c++-class-template | name: Foo
 // CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: f
 // CHECK-NEXT: [indexDeclaration]: kind: function | name: g
 // CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: Foo | 
USR: c:@ST>1#T@Foo
 // CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: f | 
USR: c:@ST>1#T@Foo@F@f#t0.0#
+
+// CHECK: [indexDeclaration]: kind: c++-class-template | name: D
+// CHECK-NEXT: : kind: c++-class-template | name: B
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: B
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -130,14 +130,15 @@
   bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) {
 if (const auto *T = TL.getTypePtr()) {
   if (IndexCtx.shouldIndexImplicitInstantiation()) {
-if (CXXRecordDecl *RD = T->getAsCXXRecordDecl())
+if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) {
   IndexCtx.handleReference(RD, TL.getTemplateNameLoc(),
Parent, ParentDC, SymbolRoleSet(), 
Relations);
-  } else {
-if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
-  IndexCtx.handleReference(D, TL.getTemplateNameLoc(),
-   Parent, ParentDC, SymbolRoleSet(), 
Relations);
+  return true;
+}
   }
+  if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
+IndexCtx.handleReference(D, TL.getTemplateNameLoc(), Parent, ParentDC,
+ SymbolRoleSet(), Relations);
 }
 return true;
   }


Index: test/Index/index-template-specialization.cpp
===
--- test/Index/index-template-specialization.cpp
+++ test/Index/index-template-specialization.cpp
@@ -9,11 +9,21 @@
   foo.f(0);
 }
 
+template 
+struct B {};
+
+template 
+struct D : B {};
+
 // FIXME: if c-index-test uses OrigD for symbol info, refererences below should
 // refer to template specialization decls.
 // RUN: env CINDEXTEST_INDEXIMPLICITTEMPLATEINSTANTIATIONS=1 c-index-test -index-file %s | FileCheck %s
 // CHECK: [indexDeclaration]: kind: c++-class-template | name: Foo
 // CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: f
 // CHECK-NEXT: [indexDeclaration]: kind: function | name: g
 // CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: Foo | USR: c:@ST>1#T@Foo
 // CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: f | USR: c:@ST>1#T@Foo@F@f#t0.0#
+
+// CHECK: [indexDeclaration]: kind: c++-class-template | name: D
+// CHECK-NEXT: : kind: c++-class-template | name: B
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: B
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -130,14 +130,15 @@
   bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) {
 if (const auto *T = TL.getTypePtr()) {
   if (IndexCtx.shouldIndexImplicitInstantiation()) {
-if (CXXRecordDecl *RD = T->getAsCXXRecordDecl())
+if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) {
   IndexCtx.handleReference(RD, TL.getTemplateNameLoc(),
Parent, ParentDC, SymbolRoleSet(), Relations);
-  } else {
-if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
-  IndexCtx.handleReference(D, TL.getTemplateNameLoc(),
-   Parent, ParentDC, SymbolRoleSet(), Relations);
+  return true;
+}
   }
+  if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
+IndexCtx.handleReference(D, TL.getTemplateNameLoc(), Parent, ParentDC,
+ SymbolRoleSet(), Relations);
   

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: include/clang/Driver/CC1Options.td:236
+def coverage_exclude_EQ : Joined<["-"], "coverage-exclude=">,
+  Alias;
 def coverage_exit_block_before_body : Flag<["-"], 
"coverage-exit-block-before-body">,

marco-c wrote:
> calixte wrote:
> > vsk wrote:
> > > Have you checked whether gcc supports similar options? If so, it would be 
> > > great if we could match their name & behavior.
> > The only one I found -finstrument-functions-exclude-file-list 
> > (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html).
> > But no regex and no way to include one file only.
> > I took the names from gcovr: 
> > https://manpages.debian.org/jessie/gcovr/gcovr.1.en.html
> We could file a bug in GCC's Bugzilla and agree with them about the options.
+ 1, I think it's a great idea to loop in some gcc developers.


Repository:
  rC Clang

https://reviews.llvm.org/D52034



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

[PATCH] D52273: [clangd] Initial implementation of expected types

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This seems very clever, but extremely complicated - you've implemented much of 
C++'s conversion logic, it's not clear to me which parts are actually necessary 
to completion quality.
(Honestly this applies to expected types overall - it seems intuitively likely 
that it's a good signal, it seems less obvious that it pulls its weight if it 
can't be made simple).

From the outside it seems much of it is YAGNI, and if we do then we need to 
build it up slowly with an eye for maintainability.
Can we start with expected type boosting (no conversions) as previously 
discussed, and later measure which other parts make a difference? (I think 
we'll need/want the simple model anyway, for this to work with Dex and other 
token-based indexes).




Comment at: clangd/ExpectedTypes.h:66
+using SHA1Array = std::array;
+SHA1Array computeSHA1(llvm::StringRef Input);
+

While a hash of a string might be a reasonable choice in the long term, I worry 
about debuggability. (With SymbolID we can just look up the symbol).

You could make the hashing an implementation detail of the index, and have the 
APIs speak in terms of opaque strings. But that forces the index to be able to 
report the full opaque string of each returned symbol (for scoring), so the 
index now has to have a lookup table... messy.

Another fun thing about this representation is that you're storing 20 bytes of 
data (+ overhead) for common types like "void" where we could get away with one.



Comment at: clangd/ExpectedTypes.h:66
+using SHA1Array = std::array;
+SHA1Array computeSHA1(llvm::StringRef Input);
+

sammccall wrote:
> While a hash of a string might be a reasonable choice in the long term, I 
> worry about debuggability. (With SymbolID we can just look up the symbol).
> 
> You could make the hashing an implementation detail of the index, and have 
> the APIs speak in terms of opaque strings. But that forces the index to be 
> able to report the full opaque string of each returned symbol (for scoring), 
> so the index now has to have a lookup table... messy.
> 
> Another fun thing about this representation is that you're storing 20 bytes 
> of data (+ overhead) for common types like "void" where we could get away 
> with one.
in the *short run* I'd suggest just printing the type name and using that as 
the representation.
I'm happy to (eventually) learn about the semantics of USRs in types, but not 
today :-)



Comment at: clangd/ExpectedTypes.h:68
+
+/// Represents a type of partially applied conversion. Should be treated as an
+/// opaque value and can only be used to check whether the types are converible

this represents a type (in the c++ sense), not a conversion, right?



Comment at: clangd/ExpectedTypes.h:69
+/// Represents a type of partially applied conversion. Should be treated as an
+/// opaque value and can only be used to check whether the types are converible
+/// between each other (by using the equality operator).

"convertible (using equality)" is confusing.

It sounds like "this is actually an equivalence class of types" but I think 
that's not true, because it's not symmetric.

Isn't the model here just "SType is a serializable token representing a type. 
They can be compared for equality."



Comment at: clangd/ExpectedTypes.h:72
+/// Representation is fixed-size, small and cheap to copy.
+class SType {
+public:

Is this a placeholder name? It's not clear what it means.

Suggest OpaqueType or ExpectedType



Comment at: clangd/ExpectedTypes.h:81
+  /// implementation attempts to store as little types as possible.
+  static llvm::SmallVector
+  fromCompletionResult(ASTContext &Ctx, const CodeCompletionResult &R);

can we separate "get the representative set of types for R" from "encode them 
as SType"?
Seems like the APIs would be easier to test and understand.

(I think at least the former should be a non-member function BTW, to keep clear 
that SType itself isn't aware of any clever folding or whatnot)



Comment at: clangd/ExpectedTypes.h:82
+  static llvm::SmallVector
+  fromCompletionResult(ASTContext &Ctx, const CodeCompletionResult &R);
+

coupling to CompletionResult seems premature here, can we stick to passing 
getExpectedType() until we know that abstraction needs to be broken?



Comment at: clangd/ExpectedTypes.h:91
+  ///
+  /// The result is a map from a type to a multiplier (>= 1) that denotes the
+  /// quality of conversion that had to be applied (better conversion receive

I don't understand the scale here. If better conversions get higher numbers, 
what number does "no conversion" get?
The code looks like worse conversions get higher numbers.
I'd suggest using an additive penalty to avoid confusion with scores, but 
really...

t

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: test/CodeGen/code-coverage-filter.c:4
+// RUN: %clang_cc1 -emit-llvm -femit-coverage-data -coverage-exclude=.*\\.h %s 
-o - \
+// RUN:| FileCheck -check-prefix=NOH %s
+// RUN: %clang_cc1 -emit-llvm -femit-coverage-data -coverage-filter=.*\\.c %s 
-o - \

Could you use more descriptive check prefixes here, like 'NO-HEADERS'? Also, if 
it's possible to use fewer '\' escape tokens by wrapping the regex list in 
single-quotes, that would make things easier to read.



Comment at: test/CodeGen/code-coverage-filter.c:32
+
+// ALL: define i32 @test1(i32 %x) #0 {{.*}}
+// ALL: {{.*}}__llvm_gcov_ctr{{.*}}

1. This test will break on bots which use 16-bit ints. For simplicity, consider 
sticking to `void` and getting rid of unnecessary control flow.
2. Is there any need to check '#0'?


Repository:
  rC Clang

https://reviews.llvm.org/D52034



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

Re: r342668 - Add testcases for r342667.

[Jorge Gorbe Moya via cfe-commits]

Filed https://bugs.llvm.org/show_bug.cgi?id=39029 for the incorrect
behavior when including a path with a leading slash on Windows.

On Thu, Sep 20, 2018 at 3:35 PM Eric Christopher  wrote:

> Thank you!
>
> On Thu, Sep 20, 2018, 3:34 PM Zachary Turner  wrote:
>
>> Test removed in r342693.
>>
>> On Thu, Sep 20, 2018 at 3:30 PM Jorge Gorbe Moya 
>> wrote:
>>
>>> Zach and I were able to find the cause.
>>>
>>> Clang on Windows manages to find "file.h" when you #include "/file.h"
>>> and that makes the expected diagnostic not appear. MSVC inteprets an
>>> #include with a leading slash as an absolute path so I think we have
>>> accidentally hit a different bug in Clang :)
>>>
>>> One option to fix the test would be replacing the slash with another
>>> random non-alphanumeric character that can't be interpreted as a directory
>>> separator, but at that point I think we can just delete the failing test
>>> and rely on the existing include-likely-typo.c that tests with both leading
>>> and trailing non-alphanumeric characters.
>>>
>>> The other test in r342668 works because it includes a file that doesn't
>>> exist even if you interpret the path as relative so it should be OK to keep
>>> while the bug is found.
>>>
>>> I'll go find a bug about the behavior on windows. Thanks!
>>>
>>> Jorge
>>>
>>> On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher 
>>> wrote:
>>>
 FWIW we're trying to reproduce here real fast and then will revert or
 fix real fast.

 Thanks!

 -eric

 On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher 
 wrote:

> Adding Jorge...
>
> On Thu, Sep 20, 2018 at 2:36 PM  wrote:
>
>> Hi Eric,
>>
>> The test that you added in this commit is failing on the PS4 Windows
>> bot. Can you please take a look?
>>
>>
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>>
>> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765
>> of 43992)
>>  TEST 'Clang ::
>> Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
>> 
>> Script:
>> --
>> : 'RUN: at line 1';
>>  
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
>> -cc1 -internal-isystem
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
>> -nostdsysteminc
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> -verify
>> --
>> Exit Code: 1
>>
>> Command Output (stdout):
>> --
>> $ ":" "RUN: at line 1"
>> $
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
>> "-cc1" "-internal-isystem"
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
>> "-nostdsysteminc"
>> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
>> "-verify"
>> # command stderr:
>> error: 'error' diagnostics expected but not seen:
>>
>>   File
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> Line 3: '/empty_file_to_include.h' file not found, did you mean
>> 'empty_file_to_include.h'?
>>
>> 1 error generated.
>>
>>
>> error: command failed with exit status: 1
>>
>>
> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>
> Thanks!
>
> -eric
>
>
>
>> Douglas Yung
>>
>> > -Original Message-
>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
>> Behalf
>> > Of Eric Christopher via cfe-commits
>> > Sent: Thursday, September 20, 2018 10:23
>> > To: cfe-commits@lists.llvm.org
>> > Subject: r342668 - Add testcases for r342667.
>> >
>> > Author: echristo
>> > Date: Thu Sep 20 10:22:43 2018
>> > New Revision: 342668
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
>> > Log:
>> > Add testcases for r342667.
>> >
>> > Added:
>> >
>>  cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> >
>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c?rev=342668&view=auto
>> >
>> ===
>> > ===
>> > ---
>> cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest

Re: r342668 - Add testcases for r342667.

[Eric Christopher via cfe-commits]

Thank you!

On Thu, Sep 20, 2018, 3:34 PM Zachary Turner  wrote:

> Test removed in r342693.
>
> On Thu, Sep 20, 2018 at 3:30 PM Jorge Gorbe Moya 
> wrote:
>
>> Zach and I were able to find the cause.
>>
>> Clang on Windows manages to find "file.h" when you #include "/file.h" and
>> that makes the expected diagnostic not appear. MSVC inteprets an #include
>> with a leading slash as an absolute path so I think we have accidentally
>> hit a different bug in Clang :)
>>
>> One option to fix the test would be replacing the slash with another
>> random non-alphanumeric character that can't be interpreted as a directory
>> separator, but at that point I think we can just delete the failing test
>> and rely on the existing include-likely-typo.c that tests with both leading
>> and trailing non-alphanumeric characters.
>>
>> The other test in r342668 works because it includes a file that doesn't
>> exist even if you interpret the path as relative so it should be OK to keep
>> while the bug is found.
>>
>> I'll go find a bug about the behavior on windows. Thanks!
>>
>> Jorge
>>
>> On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher 
>> wrote:
>>
>>> FWIW we're trying to reproduce here real fast and then will revert or
>>> fix real fast.
>>>
>>> Thanks!
>>>
>>> -eric
>>>
>>> On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher 
>>> wrote:
>>>
 Adding Jorge...

 On Thu, Sep 20, 2018 at 2:36 PM  wrote:

> Hi Eric,
>
> The test that you added in this commit is failing on the PS4 Windows
> bot. Can you please take a look?
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>
> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765
> of 43992)
>  TEST 'Clang ::
> Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
> 
> Script:
> --
> : 'RUN: at line 1';
>  
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
> -cc1 -internal-isystem
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
> -nostdsysteminc
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> -verify
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> $ ":" "RUN: at line 1"
> $
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
> "-cc1" "-internal-isystem"
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
> "-nostdsysteminc"
> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
> "-verify"
> # command stderr:
> error: 'error' diagnostics expected but not seen:
>
>   File
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> Line 3: '/empty_file_to_include.h' file not found, did you mean
> 'empty_file_to_include.h'?
>
> 1 error generated.
>
>
> error: command failed with exit status: 1
>
>
 Oof. Thanks. If I don't have something in 10 minutes I'll just revert.

 Thanks!

 -eric



> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
> Behalf
> > Of Eric Christopher via cfe-commits
> > Sent: Thursday, September 20, 2018 10:23
> > To: cfe-commits@lists.llvm.org
> > Subject: r342668 - Add testcases for r342667.
> >
> > Author: echristo
> > Date: Thu Sep 20 10:22:43 2018
> > New Revision: 342668
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> > Log:
> > Add testcases for r342667.
> >
> > Added:
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> >
> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c?rev=342668&view=auto
> >
> ===
> > ===
> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > (added)
> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > Thu Sep 20 10:22:43 2018
> > @@ -0,0 +1,3 @@
> > +// RUN: %clang_cc1 %s -verify
> > +
> > +#include "/non_existing_file_to_include.h" // expected-error
> > {{'/non_existing_file_to_include.h' file not found}}
> 

Re: r342668 - Add testcases for r342667.

[Zachary Turner via cfe-commits]

Test removed in r342693.

On Thu, Sep 20, 2018 at 3:30 PM Jorge Gorbe Moya  wrote:

> Zach and I were able to find the cause.
>
> Clang on Windows manages to find "file.h" when you #include "/file.h" and
> that makes the expected diagnostic not appear. MSVC inteprets an #include
> with a leading slash as an absolute path so I think we have accidentally
> hit a different bug in Clang :)
>
> One option to fix the test would be replacing the slash with another
> random non-alphanumeric character that can't be interpreted as a directory
> separator, but at that point I think we can just delete the failing test
> and rely on the existing include-likely-typo.c that tests with both leading
> and trailing non-alphanumeric characters.
>
> The other test in r342668 works because it includes a file that doesn't
> exist even if you interpret the path as relative so it should be OK to keep
> while the bug is found.
>
> I'll go find a bug about the behavior on windows. Thanks!
>
> Jorge
>
> On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher 
> wrote:
>
>> FWIW we're trying to reproduce here real fast and then will revert or fix
>> real fast.
>>
>> Thanks!
>>
>> -eric
>>
>> On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher 
>> wrote:
>>
>>> Adding Jorge...
>>>
>>> On Thu, Sep 20, 2018 at 2:36 PM  wrote:
>>>
 Hi Eric,

 The test that you added in this commit is failing on the PS4 Windows
 bot. Can you please take a look?


 http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052

 FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765
 of 43992)
  TEST 'Clang ::
 Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
 
 Script:
 --
 : 'RUN: at line 1';
  
 c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
 -cc1 -internal-isystem
 c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
 -nostdsysteminc
 C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
 -verify
 --
 Exit Code: 1

 Command Output (stdout):
 --
 $ ":" "RUN: at line 1"
 $
 "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
 "-cc1" "-internal-isystem"
 "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
 "-nostdsysteminc"
 "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
 "-verify"
 # command stderr:
 error: 'error' diagnostics expected but not seen:

   File
 C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
 Line 3: '/empty_file_to_include.h' file not found, did you mean
 'empty_file_to_include.h'?

 1 error generated.


 error: command failed with exit status: 1


>>> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>>>
>>> Thanks!
>>>
>>> -eric
>>>
>>>
>>>
 Douglas Yung

 > -Original Message-
 > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
 Behalf
 > Of Eric Christopher via cfe-commits
 > Sent: Thursday, September 20, 2018 10:23
 > To: cfe-commits@lists.llvm.org
 > Subject: r342668 - Add testcases for r342667.
 >
 > Author: echristo
 > Date: Thu Sep 20 10:22:43 2018
 > New Revision: 342668
 >
 > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
 > Log:
 > Add testcases for r342667.
 >
 > Added:
 > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
 > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
 >
 > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
 > suggest.c
 > URL: http://llvm.org/viewvc/llvm-
 > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
 > suggest.c?rev=342668&view=auto
 >
 ===
 > ===
 > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
 > (added)
 > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
 > Thu Sep 20 10:22:43 2018
 > @@ -0,0 +1,3 @@
 > +// RUN: %clang_cc1 %s -verify
 > +
 > +#include "/non_existing_file_to_include.h" // expected-error
 > {{'/non_existing_file_to_include.h' file not found}}
 >
 > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
 > URL: http://llvm.org/viewvc/llvm-
 > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
 > suggest.c?rev=

r342693 - Remove failing test.

[Zachary Turner via cfe-commits]

Author: zturner
Date: Thu Sep 20 15:32:51 2018
New Revision: 342693

URL: http://llvm.org/viewvc/llvm-project?rev=342693&view=rev
Log:
Remove failing test.

Removing on behalf of Jorge Moya.  This test is broken on
Windows due to it actually being able to resolve the path.  There
is an actual Windows-specific bug somewhere, but we already have
sufficient test coverage of this with a different test, so removing
this was the approach suggested by Jorge.

Removed:
cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c

Removed: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c?rev=342692&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (original)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (removed)
@@ -1,3 +0,0 @@
-// RUN: %clang_cc1 %s -verify
-
-#include "/empty_file_to_include.h" // expected-error 
{{'/empty_file_to_include.h' file not found, did you mean 
'empty_file_to_include.h'?}}


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

Re: r342668 - Add testcases for r342667.

[Jorge Gorbe Moya via cfe-commits]

Zach and I were able to find the cause.

Clang on Windows manages to find "file.h" when you #include "/file.h" and
that makes the expected diagnostic not appear. MSVC inteprets an #include
with a leading slash as an absolute path so I think we have accidentally
hit a different bug in Clang :)

One option to fix the test would be replacing the slash with another random
non-alphanumeric character that can't be interpreted as a directory
separator, but at that point I think we can just delete the failing test
and rely on the existing include-likely-typo.c that tests with both leading
and trailing non-alphanumeric characters.

The other test in r342668 works because it includes a file that doesn't
exist even if you interpret the path as relative so it should be OK to keep
while the bug is found.

I'll go find a bug about the behavior on windows. Thanks!

Jorge

On Thu, Sep 20, 2018 at 2:51 PM Eric Christopher  wrote:

> FWIW we're trying to reproduce here real fast and then will revert or fix
> real fast.
>
> Thanks!
>
> -eric
>
> On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher 
> wrote:
>
>> Adding Jorge...
>>
>> On Thu, Sep 20, 2018 at 2:36 PM  wrote:
>>
>>> Hi Eric,
>>>
>>> The test that you added in this commit is failing on the PS4 Windows
>>> bot. Can you please take a look?
>>>
>>>
>>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>>>
>>> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
>>> 43992)
>>>  TEST 'Clang ::
>>> Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
>>> Script:
>>> --
>>> : 'RUN: at line 1';
>>>  
>>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
>>> -cc1 -internal-isystem
>>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
>>> -nostdsysteminc
>>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>>> -verify
>>> --
>>> Exit Code: 1
>>>
>>> Command Output (stdout):
>>> --
>>> $ ":" "RUN: at line 1"
>>> $
>>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
>>> "-cc1" "-internal-isystem"
>>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
>>> "-nostdsysteminc"
>>> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
>>> "-verify"
>>> # command stderr:
>>> error: 'error' diagnostics expected but not seen:
>>>
>>>   File
>>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>>> Line 3: '/empty_file_to_include.h' file not found, did you mean
>>> 'empty_file_to_include.h'?
>>>
>>> 1 error generated.
>>>
>>>
>>> error: command failed with exit status: 1
>>>
>>>
>> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>>
>> Thanks!
>>
>> -eric
>>
>>
>>
>>> Douglas Yung
>>>
>>> > -Original Message-
>>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
>>> Behalf
>>> > Of Eric Christopher via cfe-commits
>>> > Sent: Thursday, September 20, 2018 10:23
>>> > To: cfe-commits@lists.llvm.org
>>> > Subject: r342668 - Add testcases for r342667.
>>> >
>>> > Author: echristo
>>> > Date: Thu Sep 20 10:22:43 2018
>>> > New Revision: 342668
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
>>> > Log:
>>> > Add testcases for r342667.
>>> >
>>> > Added:
>>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> >
>>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>>> > suggest.c
>>> > URL: http://llvm.org/viewvc/llvm-
>>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>>> > suggest.c?rev=342668&view=auto
>>> > ===
>>> > ===
>>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > (added)
>>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>>> > Thu Sep 20 10:22:43 2018
>>> > @@ -0,0 +1,3 @@
>>> > +// RUN: %clang_cc1 %s -verify
>>> > +
>>> > +#include "/non_existing_file_to_include.h" // expected-error
>>> > {{'/non_existing_file_to_include.h' file not found}}
>>> >
>>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> > URL: http://llvm.org/viewvc/llvm-
>>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
>>> > suggest.c?rev=342668&view=auto
>>> > ===
>>> > ===
>>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>>> > (added)
>>> > +++ cfe/trunk/test/Preprocessor/include-leading-

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

lebedev.ri wrote:
> This ideally needs positive tests. E.g.:
> * `-std=c89`
> * `-std=c89 -pedantic`
> * `-std=gnu99`
> * `-std=gnu99 -pedantic`
> * `-std=c99`
> * `-std=c99 -pedantic`
> 
Since `typeof` is a gnu extension, its use constitutes an error for all non gnu 
C standards, so it's moot to check for duplicate const specifiers from typeof 
exprs.

Since we're trying to match GCC's behavior here, GCC does not warn for 
`-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

nickdesaulniers wrote:
> lebedev.ri wrote:
> > This ideally needs positive tests. E.g.:
> > * `-std=c89`
> > * `-std=c89 -pedantic`
> > * `-std=gnu99`
> > * `-std=gnu99 -pedantic`
> > * `-std=c99`
> > * `-std=c99 -pedantic`
> > 
> Since `typeof` is a gnu extension, its use constitutes an error for all non 
> gnu C standards, so it's moot to check for duplicate const specifiers from 
> typeof exprs.
> 
> Since we're trying to match GCC's behavior here, GCC does not warn for 
> `-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases.
https://godbolt.org/z/3trZdl


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

Re: r342668 - Add testcases for r342667.

[Eric Christopher via cfe-commits]

FWIW we're trying to reproduce here real fast and then will revert or fix
real fast.

Thanks!

-eric

On Thu, Sep 20, 2018 at 2:46 PM Eric Christopher  wrote:

> Adding Jorge...
>
> On Thu, Sep 20, 2018 at 2:36 PM  wrote:
>
>> Hi Eric,
>>
>> The test that you added in this commit is failing on the PS4 Windows bot.
>> Can you please take a look?
>>
>>
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>>
>> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
>> 43992)
>>  TEST 'Clang ::
>> Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
>> Script:
>> --
>> : 'RUN: at line 1';
>>  
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
>> -cc1 -internal-isystem
>> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
>> -nostdsysteminc
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> -verify
>> --
>> Exit Code: 1
>>
>> Command Output (stdout):
>> --
>> $ ":" "RUN: at line 1"
>> $
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
>> "-cc1" "-internal-isystem"
>> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
>> "-nostdsysteminc"
>> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
>> "-verify"
>> # command stderr:
>> error: 'error' diagnostics expected but not seen:
>>
>>   File
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
>> Line 3: '/empty_file_to_include.h' file not found, did you mean
>> 'empty_file_to_include.h'?
>>
>> 1 error generated.
>>
>>
>> error: command failed with exit status: 1
>>
>>
> Oof. Thanks. If I don't have something in 10 minutes I'll just revert.
>
> Thanks!
>
> -eric
>
>
>
>> Douglas Yung
>>
>> > -Original Message-
>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
>> > Of Eric Christopher via cfe-commits
>> > Sent: Thursday, September 20, 2018 10:23
>> > To: cfe-commits@lists.llvm.org
>> > Subject: r342668 - Add testcases for r342667.
>> >
>> > Author: echristo
>> > Date: Thu Sep 20 10:22:43 2018
>> > New Revision: 342668
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
>> > Log:
>> > Add testcases for r342667.
>> >
>> > Added:
>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> >
>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
>> > suggest.c?rev=342668&view=auto
>> > ===
>> > ===
>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > (added)
>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
>> > Thu Sep 20 10:22:43 2018
>> > @@ -0,0 +1,3 @@
>> > +// RUN: %clang_cc1 %s -verify
>> > +
>> > +#include "/non_existing_file_to_include.h" // expected-error
>> > {{'/non_existing_file_to_include.h' file not found}}
>> >
>> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
>> > suggest.c?rev=342668&view=auto
>> > ===
>> > ===
>> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
>> > (added)
>> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
>> > Sep 20 10:22:43 2018
>> > @@ -0,0 +1,3 @@
>> > +// RUN: %clang_cc1 %s -verify
>> > +
>> > +#include "/empty_file_to_include.h" // expected-error
>> > {{'/empty_file_to_include.h' file not found, did you mean
>> > 'empty_file_to_include.h'?}}
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r342668 - Add testcases for r342667.

[Eric Christopher via cfe-commits]

Adding Jorge...

On Thu, Sep 20, 2018 at 2:36 PM  wrote:

> Hi Eric,
>
> The test that you added in this commit is failing on the PS4 Windows bot.
> Can you please take a look?
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052
>
> FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of
> 43992)
>  TEST 'Clang ::
> Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
> Script:
> --
> : 'RUN: at line 1';
>  
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
> -cc1 -internal-isystem
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
> -nostdsysteminc
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> -verify
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> $ ":" "RUN: at line 1"
> $
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
> "-cc1" "-internal-isystem"
> "c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
> "-nostdsysteminc"
> "C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
> "-verify"
> # command stderr:
> error: 'error' diagnostics expected but not seen:
>
>   File
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
> Line 3: '/empty_file_to_include.h' file not found, did you mean
> 'empty_file_to_include.h'?
>
> 1 error generated.
>
>
> error: command failed with exit status: 1
>
>
Oof. Thanks. If I don't have something in 10 minutes I'll just revert.

Thanks!

-eric



> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Eric Christopher via cfe-commits
> > Sent: Thursday, September 20, 2018 10:23
> > To: cfe-commits@lists.llvm.org
> > Subject: r342668 - Add testcases for r342667.
> >
> > Author: echristo
> > Date: Thu Sep 20 10:22:43 2018
> > New Revision: 342668
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> > Log:
> > Add testcases for r342667.
> >
> > Added:
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> >
> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> > suggest.c?rev=342668&view=auto
> > ===
> > ===
> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > (added)
> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> > Thu Sep 20 10:22:43 2018
> > @@ -0,0 +1,3 @@
> > +// RUN: %clang_cc1 %s -verify
> > +
> > +#include "/non_existing_file_to_include.h" // expected-error
> > {{'/non_existing_file_to_include.h' file not found}}
> >
> > Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
> > suggest.c?rev=342668&view=auto
> > ===
> > ===
> > --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> > (added)
> > +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
> > Sep 20 10:22:43 2018
> > @@ -0,0 +1,3 @@
> > +// RUN: %clang_cc1 %s -verify
> > +
> > +#include "/empty_file_to_include.h" // expected-error
> > {{'/empty_file_to_include.h' file not found, did you mean
> > 'empty_file_to_include.h'?}}
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r342668 - Add testcases for r342667.

[via cfe-commits]

Hi Eric,

The test that you added in this commit is failing on the PS4 Windows bot. Can 
you please take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20052

FAIL: Clang :: Preprocessor/include-leading-nonalpha-suggest.c (10765 of 43992)
 TEST 'Clang :: 
Preprocessor/include-leading-nonalpha-suggest.c' FAILED 
Script:
--
: 'RUN: at line 1';   
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE
 -cc1 -internal-isystem 
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include
 -nostdsysteminc 
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
 -verify
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ 
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.EXE"
 "-cc1" "-internal-isystem" 
"c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\8.0.0\include"
 "-nostdsysteminc" 
"C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c"
 "-verify"
# command stderr:
error: 'error' diagnostics expected but not seen: 

  File 
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Preprocessor\include-leading-nonalpha-suggest.c
 Line 3: '/empty_file_to_include.h' file not found, did you mean 
'empty_file_to_include.h'?

1 error generated.


error: command failed with exit status: 1

Douglas Yung

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of Eric Christopher via cfe-commits
> Sent: Thursday, September 20, 2018 10:23
> To: cfe-commits@lists.llvm.org
> Subject: r342668 - Add testcases for r342667.
> 
> Author: echristo
> Date: Thu Sep 20 10:22:43 2018
> New Revision: 342668
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
> Log:
> Add testcases for r342667.
> 
> Added:
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> 
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-
> suggest.c?rev=342668&view=auto
> ===
> ===
> --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> (added)
> +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
> Thu Sep 20 10:22:43 2018
> @@ -0,0 +1,3 @@
> +// RUN: %clang_cc1 %s -verify
> +
> +#include "/non_existing_file_to_include.h" // expected-error
> {{'/non_existing_file_to_include.h' file not found}}
> 
> Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-
> suggest.c?rev=342668&view=auto
> ===
> ===
> --- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
> (added)
> +++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu
> Sep 20 10:22:43 2018
> @@ -0,0 +1,3 @@
> +// RUN: %clang_cc1 %s -verify
> +
> +#include "/empty_file_to_include.h" // expected-error
> {{'/empty_file_to_include.h' file not found, did you mean
> 'empty_file_to_include.h'?}}
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52323: Add necessary support for storing code-model to module IR.

[Caroline Tice via Phabricator via cfe-commits]

cmtice created this revision.
cmtice added reviewers: tejohnson, pcc.
Herald added subscribers: cfe-commits, dexonsmith, mehdi_amini.

Currently the code-model does not get saved in the module IR, so if a code 
model is specified when compiling with LTO, it gets lost and is not propagated 
properly to LTO. This patch does what is necessary in the front end to pass the 
code-model to the module, so that the back end can store it in the Module (see 
https://reviews.llvm.org/D52322 for the back-end patch).

Fixes PR33306.


Repository:
  rC Clang

https://reviews.llvm.org/D52323

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGen/codemodels.c


Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm  %s -o - | FileCheck %s 
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s 
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s 
-check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s 
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s 
-check-prefix=CHECK-LARGE
+
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -35,6 +35,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
 
 namespace llvm {
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
 #include "clang/CodeGen/ConstantInitBuilder.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/CallSite.h"
@@ -556,6 +557,21 @@
   getModule().setPIELevel(static_cast(PLevel));
   }
 
+  if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+  .Case("tiny", llvm::CodeModel::Tiny)
+  .Case("small", llvm::CodeModel::Small)
+  .Case("kernel", llvm::CodeModel::Kernel)
+  .Case("medium", llvm::CodeModel::Medium)
+  .Case("large", llvm::CodeModel::Large)
+  .Case("default", ~1u)
+  .Default(~0u);
+if ((CM != ~0u) && (CM != ~1u)) {
+  llvm::CodeModel::Model codeModel = 
static_cast(CM);
+  getModule().setCodeModel(codeModel);
+}
+  }
+
   if (CodeGenOpts.NoPLT)
 getModule().setRtLibUseGOT();
 


Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm  %s -o - | FileCheck %s -check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s -check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s -check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s -check-prefix=CHECK-LARGE
+
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -35,6 +35,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
 
 namespace llvm {
Index

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



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

Can this be rewritten with `llvm::for_each()` and a `Twine` so that we don't 
have to use `ostringstream` (which is a big hammer for this).



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:73
+  diag(Namespaces.front()->getBeginLoc(),
+   "Nested namespaces can be concatenated", DiagnosticIDs::Warning)
+  << FixItHint::CreateReplacement(FrontReplacement,

Diagnostics should not start with a capital letter.



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+  using NamespaceContextVec = llvm::SmallVector;
+

Why 6?



Comment at: docs/clang-tidy/checks/list.rst:13
abseil-str-cat-append
+   abseil-string-find-startswith
android-cloexec-accept

This is an unrelated change -- feel free to make a separate commit that fixes 
this (no review needed).


https://reviews.llvm.org/D52136



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In https://reviews.llvm.org/D52193#1241056, @zturner wrote:

> The process stuff looks cool and useful independently of `/MP`.  Would it be 
> possible to break that into a separate patch, and add a unit test for the 
> behavior of the `WaitAll` flag?


Of course, will do.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

You can retrieve any node from the `ASTMatcher` by `.bind()`ing sub-matchers to 
string keys and later retrieving them from the `MatchResult` dictionary by 
those keys. It's like a regex capturing groups (and, well, `ASTMatchers` also 
support backreferences to such groups, i.e. `equalsBoundNode()`).


Repository:
  rC Clang

https://reviews.llvm.org/D51866



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: lib/Sema/SemaType.cpp:1682
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {

This is broken for C11 and C17 (even before you touch anything). As we just 
talked about, let's have a helper function to detect the oldest version (and 
maybe even that should get promoted as a LANGOPT).


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D50214: Add inherited attributes before parsed attributes.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D50214



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

The process stuff looks cool and useful independently of `/MP`.  Would it be 
possible to break that into a separate patch, and add a unit test for the 
behavior of the `WaitAll` flag?


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166357.
nickdesaulniers added a comment.

- add line numbers to match specific warning lines


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Volodymyr,

Thanks for working on this, really nice work with the tests! Comments below.

> - No support for 'fallthrough' in crash reproducer.

That'd be nice to have at some point to make sure we never escape into the real 
fs.

> - Current way of working with modules in VFS "root" is clunky and error-prone.

Why?

> Also I don't like that VFSFromYamlDirIterImpl is similar to overlay iterator 
> but doesn't share code. At the moment I think it's not worth it to rework 
> those abstractions to make more code reusable. If you've noticed problems 
> with those iterators before, maybe it makes sense to try to find a better 
> approach.

Maybe add FIXMEs for it?




Comment at: clang/lib/Basic/VirtualFileSystem.cpp:934
   RedirectingFileSystem &FS;
   RedirectingDirectoryEntry::iterator Current, End;
+  bool IsExternalFSCurrent;

Can you add comments to these new members? Specifically, what's the purpose of 
`IsExternalFSCurrent` and how it connects  to `ExternalFS`



Comment at: clang/lib/Basic/VirtualFileSystem.cpp:940
 
-  std::error_code incrementImpl();
+  std::error_code incrementExternal();
+  std::error_code incrementContent(bool IsFirstTime);

Same for these methods



Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1738
+ErrorOr RedirectingFileSystem::status(const Twine &Path,
+  bool ShouldCheckExternalFS) {
   ErrorOr Result = lookupPath(Path);

Is passing `ShouldCheckExternalFS ` really necessary? Why checking the status 
of the member isn't enough?



Comment at: clang/lib/Basic/VirtualFileSystem.cpp:2041
+  IsExternalFSCurrent(false), ExternalFS(ExternalFS) {
+  EC = incrementImpl(true);
 }

`incrementImpl(true/*IsFirstTime*/)`



Comment at: clang/lib/Basic/VirtualFileSystem.cpp:2045
 std::error_code VFSFromYamlDirIterImpl::increment() {
-  assert(Current != End && "cannot iterate past end");
-  ++Current;
-  return incrementImpl();
+  return incrementImpl(false);
+}

`incrementImpl(false/*IsFirstTime*/)`


https://reviews.llvm.org/D50539



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

`LinkageComputer` isn't actually persisted anywhere, right?  And there's maybe 
one computer active at once?  So this compression is theoretically saving one 
pointer of stack space but forcing a bunch of bit-manipulation every time these 
fields are accessed.


Repository:
  rC Clang

https://reviews.llvm.org/D52268



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166355.
nickdesaulniers added a comment.

- move test to new file, use check-prefix for both cases


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

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

It seems Reid's change has done good: `cmake` is not as significant as before 
in the build process. See below the improved timings:

The tests consist in a full cleanup (delete build folder), cmake to regenerate, 
then a full rebuild of LLVM + Clang + LLD (at r342552), Release target, 
optimized tablegen.
VS2017 15.8.3, Ninja 1.8.2, CMake 3.12.2
For the `clang-cl` tests, I'm not using any official LLVM release, only the 
binaries I built myself. `lld-link` is used in that case.

//(built with MSVC)// means the LLVM toolchain used to perfom this test was 
compiled in a previous run with MSVC cl 15.8.3
//(built with Clang)// means the LLVM toolchain used to perform this test was 
compiled in a previous run with Clang at r342552

I took the best figures from several runs (ran in a random order).

---

**Config 1 :** Intel Xeon Haswell 6 cores / 12 HW threads, 3.5 GHz, 15M cache, 
128 GB RAM, SSD 550 MB/s

__MSBuild :__

| MSVC cl /MP | (50min 26sec) | 2 parallel msbuild  
|
| MSVC cl /MP | (40min 23sec) | 8 parallel msbuild  
|
| MSVC cl /MP | (40min 5sec)  | 16 parallel msbuild 
|
| clang-cl //(built with MSVC)//  | (43min 36sec) | 16 parallel msbuild 
|
| clang-cl //(built with Clang//) | (43min 42sec) | 16 parallel msbuild 
|
| clang-cl **/MP** //(built with MSVC)//  | not tested| 
|
| clang-cl **/MP** //(built with Clang)// | (36min 13sec) | 8 parallel msbuild  
|
| clang-cl **/MP** //(built with Clang)// | (34min 57sec) | 16 parallel msbuild 
|
|

__Ninja:__

| MSVC cl | (33min 29sec) |
| clang-cl //(built with MSVC)//  | (30min 2sec)  |
| clang-cl //(built with Clang)// | (28min 29sec) |
|



-

**Config 2 :** Intel Xeon Skylake 18 cores / 36 HW threads, x2 (Dual CPU), 72 
HW threads total, 2.3 GHz, 24.75M cache, 128 GB RAM, NVMe 4.6 GB/s

__MSBuild :__

| MSVC cl /MP | (10min 3sec)  | 32 parallel msbuild 
|
| clang-cl //(built with MSVC)//  | (24min 15sec) | 32 parallel msbuild 
|
| clang-cl //(built with Clang)// | (21min 21sec) | 32 parallel msbuild 
|
| clang-cl **/MP** //(built with MSVC)//  | (7min 52sec)  | 32 parallel msbuild 
|
| clang-cl **/MP** //(built with Clang)// | (7min 30sec)  | 32 parallel msbuild 
|
|

__Ninja:__

| MSVC cl | (7min 25sec) |
| clang-cl //(built with MSVC)//  | (8min 23sec) |
| clang-cl //(built with Clang)// | (8min)   |
|




Comment at: llvm/trunk/lib/Support/Windows/Program.inc:424
 
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
-  bool WaitUntilChildTerminates, std::string *ErrMsg) {
-  assert(PI.Pid && "invalid pid to wait on, process not started?");
-  assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) &&
- "invalid process handle to wait on, process not started?");
+bool sys::WaitMany(MutableArrayRef PIArray, bool WaitOnAll,
+   unsigned SecondsToWait, bool WaitUntilProcessTerminates) {

rnk wrote:
> I guess no Posix implementation? It's kind of hard to know if we made the 
> right abstractions without doing it for Windows and *nix.
Yes, I currenly don't have an Unix box at hand. But I will implement it shortly 
as it needs to be atomically commited with the Windows part.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

This ideally needs positive tests. E.g.:
* `-std=c89`
* `-std=c89 -pedantic`
* `-std=gnu99`
* `-std=gnu99 -pedantic`
* `-std=c99`
* `-std=c99 -pedantic`




Comment at: test/Sema/gnu89.c:16
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+

There is no `otherwise` check prefix in the run lines.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

lgtm. But someone more familiar with these code paths should approve.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166352.
nickdesaulniers added a comment.

- git-clang-format HEAD~


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52320: AMDGPU: add __builtin_amdgcn_update_dpp

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: kzhuravl, b-sumner, arsenm.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely.

https://reviews.llvm.org/D52320

Files:
  include/clang/Basic/BuiltinsAMDGPU.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenOpenCL/builtins-amdgcn-vi.cl
  test/SemaOpenCL/builtins-amdgcn-error.cl


Index: test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- test/SemaOpenCL/builtins-amdgcn-error.cl
+++ test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -102,6 +102,15 @@
   *out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument 
to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
 }
 
+void test_update_dpp2(global int* out, int a, int b, int c, int d, int e, bool 
f)
+{
+  *out = __builtin_amdgcn_update_dpp(a, b, 0, 0, 0, false);
+  *out = __builtin_amdgcn_update_dpp(a, 0, c, 0, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_update_dpp(a, 0, 0, d, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, e, false); // expected-error 
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, 0, f); // expected-error 
{{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+}
+
 void test_ds_faddf(local float *out, float src, int a) {
   *out = __builtin_amdgcn_ds_faddf(out, src, a, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_faddf' must be a constant integer}}
   *out = __builtin_amdgcn_ds_faddf(out, src, 0, a, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_faddf' must be a constant integer}}
Index: test/CodeGenOpenCL/builtins-amdgcn-vi.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -96,6 +96,13 @@
   *out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
 }
 
+// CHECK-LABEL: @test_update_dpp
+// CHECK: call i32 @llvm.amdgcn.update.dpp.i32(i32 %arg1, i32 %arg2, i32 0, 
i32 0, i32 0, i1 false)
+void test_update_dpp(global int* out, int arg1, int arg2)
+{
+  *out = __builtin_amdgcn_update_dpp(arg1, arg2, 0, 0, 0, false);
+}
+
 // CHECK-LABEL: @test_ds_fadd
 // CHECK: call float @llvm.amdgcn.ds.fadd(float addrspace(3)* %out, float 
%src, i32 0, i32 0, i1 false)
 void test_ds_faddf(local float *out, float src) {
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -11310,6 +11310,14 @@
 Args[0]->getType());
 return Builder.CreateCall(F, Args);
   }
+  case AMDGPU::BI__builtin_amdgcn_update_dpp: {
+llvm::SmallVector Args;
+for (unsigned I = 0; I != 6; ++I)
+  Args.push_back(EmitScalarExpr(E->getArg(I)));
+Value *F =
+CGM.getIntrinsic(Intrinsic::amdgcn_update_dpp, Args[0]->getType());
+return Builder.CreateCall(F, Args);
+  }
   case AMDGPU::BI__builtin_amdgcn_div_fixup:
   case AMDGPU::BI__builtin_amdgcn_div_fixupf:
   case AMDGPU::BI__builtin_amdgcn_div_fixuph:
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -122,6 +122,7 @@
 TARGET_BUILTIN(__builtin_amdgcn_classh, "bhi", "nc", "16-bit-insts")
 TARGET_BUILTIN(__builtin_amdgcn_s_memrealtime, "LUi", "n", "s-memrealtime")
 TARGET_BUILTIN(__builtin_amdgcn_mov_dpp, "iiIiIiIiIb", "nc", "dpp")
+TARGET_BUILTIN(__builtin_amdgcn_update_dpp, "iiiIiIiIiIb", "nc", "dpp")
 TARGET_BUILTIN(__builtin_amdgcn_s_dcache_wb, "v", "n", "vi-insts")
 
 
//===--===//


Index: test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- test/SemaOpenCL/builtins-amdgcn-error.cl
+++ test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -102,6 +102,15 @@
   *out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
 }
 
+void test_update_dpp2(global int* out, int a, int b, int c, int d, int e, bool f)
+{
+  *out = __builtin_amdgcn_update_dpp(a, b, 0, 0, 0, false);
+  *out = __builtin_amdgcn_update_dpp(a, 0, c, 0, 0, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_update_dpp(a, 0, 0, d, 0, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_update_dpp(a, 0, 0, 0, e, false); // expected-error {{argument to '__builtin_amdgcn_update_dpp' must be a constant integer}}
+  *out = __builtin_amdgcn_updat

[PATCH] D52313: [clang-doc] Avoid parsing undefined base classes

[Roman Lebedev via Phabricator via cfe-commits]

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

If it **no longer** crashes, i guess you can test for that?

Other than that, SG.


https://reviews.llvm.org/D52313



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

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342679: Fix an assert in 
-Wquoted-include-in-framework-header (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52253?vs=166070&id=166344#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52253

Files:
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
  
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
  cfe/trunk/test/Modules/double-quotes.m


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void
Index: cfe/trunk/test/Modules/double-quotes.m
===
--- cfe/trunk/test/Modules/double-quotes.m
+++ cfe/trunk/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
 #import "A.h"
 #import 
 
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
 int bar() { return foo(); }
 
 // 
expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted 
include "A0.h" in framework header, expected angle-bracketed instead}}
Index: 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
--- 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
+++ 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index: 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
--- 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
+++ 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void
Index: cfe/trunk/test/Modules/double-quotes.m
===
--- cfe/trunk/test/Modules/double-quotes.m
+++ cfe/trunk/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
 #import "A.h"
 #import 
 
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
 int bar() { return foo(); }
 
 // expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted include "A0.h" in framework header, expected angle-bracketed instead}}
Index: cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index: cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
--- cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
+++ cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342679 - Fix an assert in -Wquoted-include-in-framework-header

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Sep 20 12:00:03 2018
New Revision: 342679

URL: http://llvm.org/viewvc/llvm-project?rev=342679&view=rev
Log:
Fix an assert in -Wquoted-include-in-framework-header

Fixes rdar://43692300

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

Added:
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/

cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h

cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Modules/double-quotes.m

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=342679&r1=342678&r2=342679&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Sep 20 12:00:03 2018
@@ -648,7 +648,7 @@ static bool isFrameworkStylePath(StringR
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void

Added: 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h?rev=342679&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
 Thu Sep 20 12:00:03 2018
@@ -0,0 +1 @@
+#include "Thing2.h"

Added: 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h?rev=342679&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
 Thu Sep 20 12:00:03 2018
@@ -0,0 +1 @@
+// Empty file!

Modified: cfe/trunk/test/Modules/double-quotes.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/double-quotes.m?rev=342679&r1=342678&r2=342679&view=diff
==
--- cfe/trunk/test/Modules/double-quotes.m (original)
+++ cfe/trunk/test/Modules/double-quotes.m Thu Sep 20 12:00:03 2018
@@ -32,6 +32,9 @@
 #import "A.h"
 #import 
 
+// Make sure we correctly handle paths that resemble frameworks, but aren't.
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
 int bar() { return foo(); }
 
 // 
expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted 
include "A0.h" in framework header, expected angle-bracketed instead}}


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

[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


https://reviews.llvm.org/D50539



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[John McCall via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/DeclarationName.h:46
 
-/// DeclarationName - The name of a declaration. In the common case,
-/// this just stores an IdentifierInfo pointer to a normal
-/// name. However, it also provides encodings for Objective-C
-/// selectors (optimizing zero- and one-argument selectors, which make
-/// up 78% percent of all selectors in Cocoa.h) and special C++ names
-/// for constructors, destructors, and conversion functions.
+namespace detail {
+

riccibruno wrote:
> rjmccall wrote:
> > Should `DeclarationNameExtra` be moved here?  I'm not sure why it's in 
> > `IdentifierTable.h` in the first place, and your refactor seems to make 
> > that even more pointless.
> `DeclarationNameExtra` is unfortunately needed by `MultiKeywordSelector`
> in `IdentifierInfo.cpp`.
Oh, and one is in Basic instead of AST.

I'm not sure there's really a good reason for Selector to exist at the Basic 
level instead of AST; it's probably more an artifact of old divisions than 
something that's really useful.  But changing that would probably be painful.



Comment at: include/clang/AST/DeclarationName.h:164
+CXXUsingDirective = 10,
+ObjCMultiArgSelector = 11
+  };

riccibruno wrote:
> rjmccall wrote:
> > Is the criterion for inclusion in the first seven really just frequency of 
> > use, or is it a combination of that and relative benefit?
> > 
> > The only one I would really quibble about is that multi-argument selectors 
> > are probably more common and important to Objective-C code than conversion 
> > operators are to C++.  But it's quite possible that the relative benefit is 
> > much higher for C++, since selectors only appear on specific kinds of 
> > declarations that know they're declared with selectors — relatively little 
> > code actually needs to be polymorphic about them — and since they have to 
> > be defined out-of-line.
> I did not do an in-depth analysis regarding the selection of the first seven 
> inline kinds.
> My thought process was that C++ constructors and operator names should 
> definitely be
> inline. C++ destructors seemed much more frequent than c++ literal operators 
> and
> deduction guides. This leaves one slot available and since C++ conversion 
> operators
> share the same class `CXXSpecialName` including it as an inline kind 
> simplifies the code.
> 
> Also a practical point is that I don't really have a representative sample of 
> ObjC code
> to benchmark this. For C++ I am using all of Boost which I hope is 
> representative
> enough. If you deem it necessary I will try to do some benchmarks with
> `ObjCMultiArgSelector` stored inline.
I think I can accept this as-is without extra investigation.



Comment at: include/clang/AST/DeclarationName.h:220
+   detail::DeclarationNameExtra::ObjCMultiArgSelector
   };
 

Thanks, this looks great.


Repository:
  rC Clang

https://reviews.llvm.org/D52267



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

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM with one more small change.




Comment at: clang/test/Modules/double-quotes.m:35
 
+// rdar://43692300
+#import "NotAFramework/Headers/Headers/Thing1.h"

Please write a meaningful message here instead, the radar number in the commit 
message should be enough to track this down later if needed.


https://reviews.llvm.org/D52253



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

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Do you/what's your particular use case for this scenario? I guess this looks a 
bit like Apple's format (where debug info stays in the object file and isn't 
linked into the final binary), but don't expect they'd be moving to this any 
time soon.


https://reviews.llvm.org/D52296



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

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342672: [Sema] Retain __restrict qualifiers when 
substituting a reference type. (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52271?vs=166224&id=166334#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52271

Files:
  lib/Sema/TreeTransform.h
  test/SemaCXX/subst-restrict.cpp


Index: test/SemaCXX/subst-restrict.cpp
===
--- test/SemaCXX/subst-restrict.cpp
+++ test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+  return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.


Index: test/SemaCXX/subst-restrict.cpp
===
--- test/SemaCXX/subst-restrict.cpp
+++ test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+  return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342672 - [Sema] Retain __restrict qualifiers when substituting a reference type.

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Sep 20 11:12:24 2018
New Revision: 342672

URL: http://llvm.org/viewvc/llvm-project?rev=342672&view=rev
Log:
[Sema] Retain __restrict qualifiers when substituting a reference type.

Fixes rdar://43760099

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

Added:
cfe/trunk/test/SemaCXX/subst-restrict.cpp
Modified:
cfe/trunk/lib/Sema/TreeTransform.h

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=342672&r1=342671&r2=342672&view=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Thu Sep 20 11:12:24 2018
@@ -4252,14 +4252,20 @@ QualType TreeTransform::Rebuild
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+  return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.

Added: cfe/trunk/test/SemaCXX/subst-restrict.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/subst-restrict.cpp?rev=342672&view=auto
==
--- cfe/trunk/test/SemaCXX/subst-restrict.cpp (added)
+++ cfe/trunk/test/SemaCXX/subst-restrict.cpp Thu Sep 20 11:12:24 2018
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");


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

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: include/clang/Driver/CLCompatOptions.td:121
+def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, HelpText<"Disable 
function inlining">;
+def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, HelpText<"Only inline 
functions which are (explicitly or implicitly) marked inline">;
+def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline 
functions as deemed beneficial by the compiler">;

Also, can we try to keep these at 80 columns?

The other flags in this file put HelpText before Alias and AliasArg (see my 
diff), but your order is fine. The 80 columns help trying to keep the help text 
concise, so it encourages better UI in this case :-)


https://reviews.llvm.org/D52266



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

[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.



Comment at: clang-tidy/utils/TypeTraits.cpp:49
+  if (Context.getTypeSize(Type) <= Context.getTypeSize(Context.VoidPtrTy))
+return false;
+

This early return now ignores the fact that the type has non-trivial copy 
constructors, virtual destructors etc which makes the type expensive to copy. 
Could we achieve the same desired outcome by adding a blacklist that allows 
users exclude types they deem to be not expensive?

This way they would get a warning the first time they run the check and then 
could disable such types. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52315



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Addressed some inline comments.




Comment at: include/clang/AST/DeclarationName.h:46
 
-/// DeclarationName - The name of a declaration. In the common case,
-/// this just stores an IdentifierInfo pointer to a normal
-/// name. However, it also provides encodings for Objective-C
-/// selectors (optimizing zero- and one-argument selectors, which make
-/// up 78% percent of all selectors in Cocoa.h) and special C++ names
-/// for constructors, destructors, and conversion functions.
+namespace detail {
+

rjmccall wrote:
> Should `DeclarationNameExtra` be moved here?  I'm not sure why it's in 
> `IdentifierTable.h` in the first place, and your refactor seems to make that 
> even more pointless.
`DeclarationNameExtra` is unfortunately needed by `MultiKeywordSelector`
in `IdentifierInfo.cpp`.



Comment at: include/clang/AST/DeclarationName.h:164
+CXXUsingDirective = 10,
+ObjCMultiArgSelector = 11
+  };

rjmccall wrote:
> Is the criterion for inclusion in the first seven really just frequency of 
> use, or is it a combination of that and relative benefit?
> 
> The only one I would really quibble about is that multi-argument selectors 
> are probably more common and important to Objective-C code than conversion 
> operators are to C++.  But it's quite possible that the relative benefit is 
> much higher for C++, since selectors only appear on specific kinds of 
> declarations that know they're declared with selectors — relatively little 
> code actually needs to be polymorphic about them — and since they have to be 
> defined out-of-line.
I did not do an in-depth analysis regarding the selection of the first seven 
inline kinds.
My thought process was that C++ constructors and operator names should 
definitely be
inline. C++ destructors seemed much more frequent than c++ literal operators and
deduction guides. This leaves one slot available and since C++ conversion 
operators
share the same class `CXXSpecialName` including it as an inline kind simplifies 
the code.

Also a practical point is that I don't really have a representative sample of 
ObjC code
to benchmark this. For C++ I am using all of Boost which I hope is 
representative
enough. If you deem it necessary I will try to do some benchmarks with
`ObjCMultiArgSelector` stored inline.


Repository:
  rC Clang

https://reviews.llvm.org/D52267



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 166324.
riccibruno marked 9 inline comments as done.
riccibruno added a comment.

Address rjmccall comments:

1. Renamed `CXXSpecialName` to `CXXSpecialNameExtra`
2. Introduced a constant `IdentifierInfoAlignment` for 
`alignas(IdentifierInfoAlignment)`
3. Updated some comments
4. Cleaned up the `static_assert` which checked the consistency of the 
numerical values of the enumerators.

Regarding using a `PointerIntPair` it would work if we pass a custom
`PointerLikeTypeTraits` to state that, yes, our `void *` are aligned to 8 bytes
instead of 4. However I am not sure this is a good idea since the 
`PointerIntPair`
do not really simplify the code. What we really want here is a pointer union 
but we
also need precise control over the low bits.


Repository:
  rC Clang

https://reviews.llvm.org/D52267

Files:
  include/clang/AST/DeclarationName.h
  include/clang/Basic/IdentifierTable.h
  lib/AST/DeclarationName.cpp
  lib/Basic/IdentifierTable.cpp
  lib/Sema/IdentifierResolver.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp

Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3574,7 +3574,7 @@
 II->isPoisoned() ||
 (IsModule ? II->hasRevertedBuiltin() : II->getObjCOrBuiltinID()) ||
 II->hasRevertedTokenIDToIdentifier() ||
-(NeedDecls && II->getFETokenInfo()))
+(NeedDecls && II->getFETokenInfo()))
   return true;
 
 return false;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -908,7 +908,7 @@
  (IsModule ? II.hasRevertedBuiltin() : II.getObjCOrBuiltinID()) ||
  II.hasRevertedTokenIDToIdentifier() ||
  (!(IsModule && Reader.getPreprocessor().getLangOpts().CPlusPlus) &&
-  II.getFETokenInfo());
+  II.getFETokenInfo());
 }
 
 static bool readBit(unsigned &Bits) {
Index: lib/Sema/IdentifierResolver.cpp
===
--- lib/Sema/IdentifierResolver.cpp
+++ lib/Sema/IdentifierResolver.cpp
@@ -147,7 +147,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (!Ptr) {
 Name.setFETokenInfo(D);
@@ -172,7 +172,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (!Ptr) {
 AddDecl(D);
@@ -213,7 +213,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   assert(Ptr && "Didn't find this decl on its identifier's chain!");
 
@@ -232,7 +232,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 readingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
   if (!Ptr) return end();
 
   if (isDeclPtr(Ptr))
@@ -304,7 +304,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 readingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (!Ptr) {
 Name.setFETokenInfo(D);
@@ -397,7 +397,7 @@
 /// It creates a new IdDeclInfo if one was not created before for this id.
 IdentifierResolver::IdDeclInfo &
 IdentifierResolver::IdDeclInfoMap::operator[](DeclarationName Name) {
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (Ptr) return *toIdDeclInfo(Ptr);
 
@@ -415,7 +415,7 @@
 
 void IdentifierResolver::iterator::incrementSlowCase() {
   NamedDecl *D = **this;
-  void *InfoPtr = D->getDeclName().getFETokenInfo();
+  void *InfoPtr = D->getDeclName().getFETokenInfo();
   assert(!isDeclPtr(InfoPtr) && "Decl with wrong id ?");
   IdDeclInfo *Info = toIdDeclInfo(InfoPtr);
 
Index: lib/Basic/IdentifierTable.cpp
===
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -382,50 +382,49 @@
 
 namespace clang {
 
-/// MultiKeywordSelector - One of these variable length records is kept for each
+/// One of these variable length records is kept for each
 /// selector containing more than one keyword. We use a folding set
 /// to unique aggregate names (keyword selectors in ObjC parlance). Access to
 /// this class is provided strictly through Selector.
-class MultiKeywordSelector
-  : public DeclarationNameExtra, public llvm::FoldingSetNode {
-  MultiKeywordSelector(unsigned nKeys) {
-ExtraKindOrNumArgs = NUM_EXTRA_KINDS + nKeys;
-  }
+class alignas(IdentifierInfoAlignment) MultiKeywordSelector
+: public detail::DeclarationNameExtra,
+  public llvm::FoldingSetNode {
+  MultiKeywor

[PATCH] D46140: [coroutines] std::task type (WIP)

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.



Comment at: test/std/experimental/task/sync_wait.hpp:36-37
+  __isSet_ = true;
+}
+__cv_.notify_all();
+  }

The call to `notify_all()` needs to be inside the lock.
Otherwise, it's possible the waiting thread may see the write to `__isSet_` 
inside `wait()` below and return, destroying the `condition_variable` before 
`__cv_.notify_all()` call completes.


https://reviews.llvm.org/D46140



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

Re: r229575 - clang-cl: Disable frame pointer elimination at -O0

[Reid Kleckner via cfe-commits]

It looks like we don't do anything special if you run clang-cl -O0 or /O0,
but it's not an error. I don't have my computer and can't run a test, but
from the outside, it looks like clang-cl -O0 does generate unoptimized code
without warning about an unrecognized flag, but it doesn't disable FP
elimination. It's also a GCC style flag, and usually if a core option is
accepted, it has the same behavior in both driver modes. I'd be fine
removing this extra handling so long as the driver explicitly tells the
user that /O0 isn't recognized. While we're at it, maybe we should warn
about other unrecognized /O flags.

On Wed, Sep 19, 2018 at 6:10 PM Nico Weber  wrote:

> The generic O flag handling doesn't support 0 either. Would you be ok with
> removing this?
>
> Does /Od do what you want?
>
> On Wed, Sep 19, 2018 at 4:52 PM Reid Kleckner 
> wrote:
>
>> I was probably using it myself, and was surprised that /O0 and -O0 had
>> different behavior, because -O0 will hit the special O0 flag that disables
>> FPO, but /O0 will hit the generic 'O' flag handling.
>>
>> On Wed, Sep 19, 2018 at 7:10 AM Nico Weber  wrote:
>>
>>> Do you remember why you landed this? cl.exe doesn't support /O0; why
>>> does clang-cl?
>>>
>>> On Tue, Feb 17, 2015 at 5:53 PM Reid Kleckner  wrote:
>>>
 Author: rnk
 Date: Tue Feb 17 16:40:42 2015
 New Revision: 229575

 URL: http://llvm.org/viewvc/llvm-project?rev=229575&view=rev
 Log:
 clang-cl: Disable frame pointer elimination at -O0

 This wasn't kicking in because the _SLASH_O flag didn't match our check
 for OPT_O0. Add an alias that does to keep the logic simple.

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

 Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=229575&r1=229574&r2=229575&view=diff

 ==
 --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
 +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Tue Feb 17
 16:40:42 2015
 @@ -80,6 +80,7 @@ def _SLASH_I : CLJoinedOrSeparate<"I">,
Alias;
  def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
Alias;
 +def _SLASH_O0 : CLFlag<"O0">, Alias;
  def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">,
MetaVarName<"">, Alias;
  def _SLASH_Ob0 : CLFlag<"Ob0">, HelpText<"Disable inlining">,


 ___
 cfe-commits mailing list
 cfe-comm...@cs.uiuc.edu
 http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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

[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This looks questionable to me.
I don't disagree with the reasons stated about llvm types.
But is that *always* true?
I would personally be very surprized, and consider this a false-positive.

This should at least be optional.
Not sure about the default, but setting the `StrictMode` should certainly 
disable this relaxation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52315



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

r342668 - Add testcases for r342667.

[Eric Christopher via cfe-commits]

Author: echristo
Date: Thu Sep 20 10:22:43 2018
New Revision: 342668

URL: http://llvm.org/viewvc/llvm-project?rev=342668&view=rev
Log:
Add testcases for r342667.

Added:
cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c

Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c?rev=342668&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c (added)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-no-suggest.c Thu Sep 
20 10:22:43 2018
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error 
{{'/non_existing_file_to_include.h' file not found}}

Added: cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c?rev=342668&view=auto
==
--- cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c (added)
+++ cfe/trunk/test/Preprocessor/include-leading-nonalpha-suggest.c Thu Sep 20 
10:22:43 2018
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error 
{{'/empty_file_to_include.h' file not found, did you mean 
'empty_file_to_include.h'?}}


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

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Eric Christopher via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342667: r342177 introduced a hint in cases where an 
#included file is not found. It… (authored by echristo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52280?vs=166184&id=166326#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52280

Files:
  cfe/trunk/lib/Lex/PPDirectives.cpp


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342667 - r342177 introduced a hint in cases where an #included file is not found. It tries to find a suggestion by removing leading or trailing non-alphanumeric characters and checking if a matching

[Eric Christopher via cfe-commits]

Author: echristo
Date: Thu Sep 20 10:21:56 2018
New Revision: 342667

URL: http://llvm.org/viewvc/llvm-project?rev=342667&view=rev
Log:
r342177 introduced a hint in cases where an #included file is not found. It 
tries to find a suggestion by removing leading or trailing non-alphanumeric 
characters and checking if a matching file exists, then it reports an error 
like:

include-likely-typo.c:3:10: error: '' file not found, 
did you mean 'empty_file_to_include.h'?
 ^~~
 "empty_file_to_include.h"
1 error generated.
However, if a hint is not found, the error message will show only the trimmed 
name we use to look for a hint, so:

will result in:

include-leading-nonalpha-no-suggest.c:3:10: fatal error: 
'non_existing_file_to_include.h' file not found
 ^~~~
1 error generated.
where the name reported after "fatal error:" doesn't match what the user wrote.

Patch by Jorge Gorbe!

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

This change reports the original file name instead of the trimmed one when a 
suggestion is not found.

Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=342667&r1=342666&r2=342667&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Sep 20 10:21:56 2018
@@ -1887,8 +1887,8 @@ void Preprocessor::HandleIncludeDirectiv
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@ void Preprocessor::HandleIncludeDirectiv
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


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

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Eric Christopher via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342667: r342177 introduced a hint in cases where an 
#included file is not found. It… (authored by echristo, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52280?vs=166184&id=166327#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52280

Files:
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings

[George Karpenkov via Phabricator via cfe-commits]

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

If that helps you, then sure.

I'm not sure  I understand why having warnings causes the collection process to 
fail, but I guess ultimately it's not important.


Repository:
  rC Clang

https://reviews.llvm.org/D51926



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

[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/utils/TypeTraits.cpp:47
+
+  // Do not consider "expensive to copy" types not greater than a pointer
+  if (Context.getTypeSize(Type) <= Context.getTypeSize(Context.VoidPtrTy))

Please make that comment a sentence, and maybe don't use `not` twice.

What do you think about making this parameter configurable and/or set it in 
relation to the size of a cache-line.
This is of course target dependent, but given the CPU effects more accurate.


https://reviews.llvm.org/D52315



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

r342666 - [OPENMP] Fix spelling of getLoopCounter (NFC)

[Mike Rice via cfe-commits]

Author: mikerice
Date: Thu Sep 20 10:19:41 2018
New Revision: 342666

URL: http://llvm.org/viewvc/llvm-project?rev=342666&view=rev
Log:
[OPENMP] Fix spelling of getLoopCounter (NFC)

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Sep 20 10:19:41 2018
@@ -990,8 +990,8 @@ public:
   /// Set loop counter for the specified loop.
   void setLoopCounter(unsigned NumLoop, Expr *Counter);
   /// Get loops counter for the specified loop.
-  Expr *getLoopCunter(unsigned NumLoop);
-  const Expr *getLoopCunter(unsigned NumLoop) const;
+  Expr *getLoopCounter(unsigned NumLoop);
+  const Expr *getLoopCounter(unsigned NumLoop) const;
 
   child_range children() { return child_range(&NumForLoops, &NumForLoops + 1); 
}
 

Modified: cfe/trunk/lib/AST/OpenMPClause.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/OpenMPClause.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/AST/OpenMPClause.cpp (original)
+++ cfe/trunk/lib/AST/OpenMPClause.cpp Thu Sep 20 10:19:41 2018
@@ -222,12 +222,12 @@ void OMPOrderedClause::setLoopCounter(un
   getTrailingObjects()[NumberOfLoops + NumLoop] = Counter;
 }
 
-Expr *OMPOrderedClause::getLoopCunter(unsigned NumLoop) {
+Expr *OMPOrderedClause::getLoopCounter(unsigned NumLoop) {
   assert(NumLoop < NumberOfLoops && "out of loops number.");
   return getTrailingObjects()[NumberOfLoops + NumLoop];
 }
 
-const Expr *OMPOrderedClause::getLoopCunter(unsigned NumLoop) const {
+const Expr *OMPOrderedClause::getLoopCounter(unsigned NumLoop) const {
   assert(NumLoop < NumberOfLoops && "out of loops number.");
   return getTrailingObjects()[NumberOfLoops + NumLoop];
 }

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Thu Sep 20 10:19:41 2018
@@ -1516,7 +1516,7 @@ void CodeGenFunction::EmitOMPPrivateLoop
 for (unsigned I = S.getCollapsedNumber(),
   E = C->getLoopNumIterations().size();
  I < E; ++I) {
-  const auto *DRE = cast(C->getLoopCunter(I));
+  const auto *DRE = cast(C->getLoopCounter(I));
   const auto *VD = cast(DRE->getDecl());
   // Override only those variables that are really emitted already.
   if (LocalDeclMap.count(VD)) {
@@ -4966,7 +4966,7 @@ void CodeGenFunction::EmitSimpleOMPExecu
 E = C->getLoopNumIterations().size();
I < E; ++I) {
 if (const auto *VD = dyn_cast(
-cast(C->getLoopCunter(I))->getDecl())) {
+cast(C->getLoopCounter(I))->getDecl())) {
   // Emit only those that were not explicitly referenced in 
clauses.
   if (!CGF.LocalDeclMap.count(VD))
 CGF.EmitVarDecl(*VD);

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=342666&r1=342665&r2=342666&view=diff
==
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Sep 20 10:19:41 2018
@@ -6555,7 +6555,7 @@ void OMPClauseWriter::VisitOMPOrderedCla
   for (Expr *NumIter : C->getLoopNumIterations())
 Record.AddStmt(NumIter);
   for (unsigned I = 0, E = C->getLoopNumIterations().size(); I getLoopCunter(I));
+Record.AddStmt(C->getLoopCounter(I));
   Record.AddSourceLocation(C->getLParenLoc());
 }
 


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

[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, flx.

The three checks mentioned in the Title are two noisy if the code uses 
intrusive smart (reference counting) pointers. LLVM/Clang is such a code, it 
has lots of such types, e.g. StringRef, SymbolRef or ProgramStateRef, all of 
them based llvm::IntrusiveRefCntPtr. Every time such a type is passed as 
parameter, used for initialization or in a range-based for loop a false 
positive warning is issued together with an (unnecessary) fix.

This patch changes the term "expensive to copy" to also regard the size of the 
type so it disables warnings and fixes if the type is not greater than a 
pointer. This removes false positives for intrusive smart pointers. False 
positives for non-intrusive smart pointers are not addressed in this patch.


https://reviews.llvm.org/D52315

Files:
  clang-tidy/utils/TypeTraits.cpp
  docs/clang-tidy/checks/performance-for-range-copy.rst
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  test/clang-tidy/Inputs/performance-unnecessary-value-param/header-fixed.h
  test/clang-tidy/Inputs/performance-unnecessary-value-param/header.h
  test/clang-tidy/performance-for-range-copy-warn-on-all-auto-copies.cpp
  test/clang-tidy/performance-for-range-copy.cpp
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp
  test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
  test/clang-tidy/performance-unnecessary-value-param.cpp

Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -8,6 +8,9 @@
   }
   void nonConstMethod();
   virtual ~ExpensiveToCopyType();
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 void mutate(ExpensiveToCopyType &);
@@ -27,6 +30,9 @@
   iterator end();
   const_iterator begin() const;
   const_iterator end() const;
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 // This class simulates std::pair<>. It is trivially copy constructible
@@ -37,13 +43,19 @@
   SomewhatTrivial(const SomewhatTrivial&) = default;
   ~SomewhatTrivial() = default;
   SomewhatTrivial& operator=(const SomewhatTrivial&);
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 struct MoveOnlyType {
   MoveOnlyType(const MoveOnlyType &) = delete;
   MoveOnlyType(MoveOnlyType &&) = default;
   ~MoveOnlyType();
   void constMethod() const;
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 struct ExpensiveMovableType {
@@ -53,6 +65,33 @@
   ExpensiveMovableType &operator=(const ExpensiveMovableType &) = default;
   ExpensiveMovableType &operator=(ExpensiveMovableType &&);
   ~ExpensiveMovableType();
+private:
+  // Greater than a pointer
+  long Data1, Data2;
+};
+
+// Intrusive smart pointers are usually passed by value
+template struct IntrusiveSmartPointerType {
+  IntrusiveSmartPointerType(const T* data);
+  IntrusiveSmartPointerType(const IntrusiveSmartPointerType& rhs);
+  const IntrusiveSmartPointerType& operator=(const T* data);
+  const IntrusiveSmartPointerType&
+  operator=(const IntrusiveSmartPointerType& data);
+
+  operator T*();
+private:
+  T *Data;
+};
+
+// Wrapper for builtin types used by intrusive smart pointers
+template
+class RefCntType {
+  unsigned count;
+  T value;
+public:
+  RefCntType(T val) : value(val) {}
+  operator T() { return value; }
+  friend class IntrusiveSmartPoitnerType;
 };
 
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
@@ -381,3 +420,10 @@
   // CHECK-FIXES: void templateFunction(ExpensiveToCopyType E) {
   E.constReference();
 }
+
+// Do not warn for intrusive smart pointers which are usually passed by value
+void negativeIntrusiveSmartPointer(const IntrusiveSmartPointerType> Obj);
+
+void negativeIntrusiveSmartPointer(const IntrusiveSmartPointerType> Obj) {
+}
+
Index: test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
+++ test/clang-tidy/performance-unnecessary-value-param-delayed.cpp
@@ -6,6 +6,9 @@
   }
   void nonConstMethod();
   virtual ~ExpensiveToCopyType();
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 void mutate(ExpensiveToCopyType &);
@@ -21,6 +24,9 @@
   SomewhatTrivial(const SomewhatTrivial&) = default;
   ~SomewhatTrivial() = default;
   SomewhatTrivial& operator=(const SomewhatTrivial&);
+private:
+  // Greater than a pointer
+  long Data1, Data2;
 };
 
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/p

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

Ohh awesome, I didn't know the LSP supported that.  I'll try it it Theia when I 
have time.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52311



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

[PATCH] D52313: [clang-doc] Avoid parsing undefined base classes

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: jakehehrlich, leonardchan, lebedev.ri.
juliehockett added a project: clang-tools-extra.

Don't try to parse base classes for declarations that are not definitions 
(segfaults, as there is no DefinitionData to access).


https://reviews.llvm.org/D52313

Files:
  clang-tools-extra/clang-doc/Serialize.cpp


Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -244,6 +244,9 @@
 }
 
 static void parseBases(RecordInfo &I, const CXXRecordDecl *D) {
+  // Don't parse bases if this isn't a definition.
+  if (!D->isThisDeclarationADefinition())
+return;
   for (const CXXBaseSpecifier &B : D->bases()) {
 if (B.isVirtual())
   continue;


Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -244,6 +244,9 @@
 }
 
 static void parseBases(RecordInfo &I, const CXXRecordDecl *D) {
+  // Don't parse bases if this isn't a definition.
+  if (!D->isThisDeclarationADefinition())
+return;
   for (const CXXBaseSpecifier &B : D->bases()) {
 if (B.isVirtual())
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52300: [clangd] Implement VByte PostingList compression

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Very nice!

I think the data structures can be slightly tighter, and some of the 
implementation could be easier to follow. But this seems like a nice win.

Right-sizing the vectors seems like an important optimization.




Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:29
+  DecompressedChunk = ChunkIndex->decompress();
+  InnerIndex = begin(DecompressedChunk);
+}

nit: we generally use members (DecompressedChunk.begin()) unless actually 
dealing with arrays or templates, since lookup rules are simpler
 



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:39
"Posting List iterator can't advance() at the end.");
-++Index;
+if (++InnerIndex != end(DecompressedChunk))
+  return; // Return if current chunk is not exhausted.

nit: I think this might be clearer with the special/unlikely cases (hit end) 
inside the if:
```
if (++InnerIndex == DecompressedChunks.begin()) { // end of chunk
  if (++ChunkIndex == Chunks.end()) // end of iterator
return;
  DecompressedChunk = ChunkIndex->decompress();
  InnerIndex = DecompressedChunk.begin();
}
```
also I think the indirection via `reachedEnd()` mostly serves to confuse here, 
as the other lines deal with the data structures directly. It's not clear 
(without reading the implementation) what the behavior is when class invariants 
are violated.



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:58
+// does.
+if ((ChunkIndex != end(Chunks) - 1) && ((ChunkIndex + 1)->Head <= ID)) {
+  // Find the next chunk with Head >= ID.

this again puts the "normal case" (need to choose a chunk) inside the if(), 
instead of the exceptional case.

In order to write this more naturally, I think pulling out a private helper 
`advanceToChunk(DocID)` might be best here, you can early return from there.



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:61
+  ChunkIndex = std::lower_bound(
+  ChunkIndex, end(Chunks), ID,
+  [](const Chunk &C, const DocID ID) { return C.Head < ID; });

ChunkIndex + 1? You've already eliminated the current chunk.



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:62
+  ChunkIndex, end(Chunks), ID,
+  [](const Chunk &C, const DocID ID) { return C.Head < ID; });
+  if (reachedEnd())

This seems unneccesarily two-step (found the chunk... or it could be the first 
element of the next).
Understandably, because std::*_bound has such a silly API.

You want to find the *last* chunk such that Head <= ID.
So find the first one with Head > ID, and subtract one.

std::lower_bound returns the first element for which its predicate is false.

Therefore:
```
ChunkIndex = std::lower_bound(ChunkIndex, Chunks.end(), ID,
 [](const Chunk &C, const DocID D) { return C.Head <= ID; }) - 1;
```



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:63
+  [](const Chunk &C, const DocID ID) { return C.Head < ID; });
+  if (reachedEnd())
+return;

(again I'd avoid reachedEnd() here as you haven't reestablished invariants, so 
it's easier to just deal with the data structures)



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:76
+// Return if the position was found in current chunk.
+if (InnerIndex != std::end(DecompressedChunk))
+  return;

(this can become an assert)



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:115
+  // Iterator over chunks.
+  std::vector::const_iterator ChunkIndex;
+  std::vector DecompressedChunk;

nit: please don't call these indexes if they're actually iterators: 
CurrentChunk seems fine



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:116
+  std::vector::const_iterator ChunkIndex;
+  std::vector DecompressedChunk;
+  // Iterator over DecompressedChunk.

(again, SmallVector)



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:121
 
+/// Single-byte masks used for VByte compression bit manipulations.
+constexpr uint8_t SevenBytesMask = 0x7f;  // 0b0111

move to the function where they're used



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:127
+/// Fills chunk with the maximum number of bits available.
+Chunk createChunk(DocID Head, std::queue &Payload,
+  size_t DocumentsCount, size_t MeaningfulBytes) {

What's the purpose of this? Why can't the caller just construct the Chunk 
themselves - what does the std::queue buy us?



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:140
+
+/// Byte offsets of Payload contents within DocID.
+const size_t 

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;

shuaiwang wrote:
> JonasToth wrote:
> > shuaiwang wrote:
> > > JonasToth wrote:
> > > > That doesn't look like a super idea. It is super hidden that variable 
> > > > 'p*' will be analyzed for pointee mutation and others aren't. 
> > > > Especially in 12 months and when someone else wants to change 
> > > > something, this will be the source of headaches.
> > > > 
> > > > Don't you think there could be a better way of doing this switch?
> > > Yeah... agree :)
> > > But how about let's keep it this way just for now, and I'll pause the 
> > > work on supporting pointee analysis and fix it properly but in a separate 
> > > diff following this one?
> > > 
> > > I've been thinking about this and also did some prototyping, and here are 
> > > my thoughts:
> > > In order to properly fix this we need to change the interface of 
> > > `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a 
> > > `MutationTrace` with additional metadata. This wasn't needed before 
> > > because we can reconstruct a mutation chain by continuously calling 
> > > `findMutation`, and that works well for cases like "int x; int& r0 = x; 
> > > int& r1 = r0; r1 = 123;". But now that pointers come into play, and we 
> > > need to handle cases like "int *p; int *p2 = p; int& r = *p2; r = 123;", 
> > > and in order to reconstruct the mutation chain, we need to do 
> > > `findPointeeMutation(p)` -> `findPointeeMutation(p2)` -> 
> > > `findMutation(r)`, notice that we need to switch from calling 
> > > `findPointeeMutation` to calling `findMutation`. However we don't know 
> > > where the switch should happen simply based on what's returned from 
> > > `findPointeeMutation`, we need additional metadata for that.
> > > 
> > > That interface change will unavoidably affect how memoization is done so 
> > > we need to change memoization as well.
> > > 
> > > Now since we need to change both the interface and memoization anyway, we 
> > > might as well design them properly so that multiple-layers of pointers 
> > > can be supported nicely. I think we can end up with something like this:
> > > ```
> > > class ExprMutationAnalyzer {
> > > public:
> > >   struct MutationTrace {
> > > const Stmt *Value;
> > > const MutationTrace *Next;
> > > // Other metadata
> > >   };
> > > 
> > >   // Finds whether any mutative operations are performed on the given 
> > > expression after dereferencing `DerefLayers` times.
> > >   const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
> > >   const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> > > };
> > > ```
> > > This involves quite some refactoring, and doing this refactoring before 
> > > this diff and later rebasing seems quite some trouble that I'd like to 
> > > avoid.
> > > 
> > > Let me know what do you think :)
> > Is the second part of your answer part to adressing this testing issue?
> > Given the whole Analyzer is WIP it is ok to postpone the testing change to 
> > later for me. But I feel that this is a quality issue that more experience 
> > clang develops should comment on because it still lands in trunk. Are you 
> > aware of other patches then clang-tidy that rely on EMA?
> > 
> > Regarding you second part (its related to multi-layer pointer mutation?!):
> > Having a dependency-graph that follows the general data flow _with_ 
> > mutation annotations would be nice to have. There is probably even 
> > something in clang (e.g. `CFG` is probably similar in idea, just with all 
> > execution paths), but I don't know enough to properly judge here.
> > 
> > In my opinion it is ok, to not do the refactoring now and just support one 
> > layer of pointer indirection. Especially in C++ there is usually no need 
> > for multi-layer pointers but more a relict from C-style.
> > C on the other hand relies on that.
> > Designing EMA new for the more generalized functionality (if necessary) 
> > would be of course great, but again: Better analyzer ppl should be involved 
> > then me, even though I would still be very interested to help :)
> The second part is more of trying to explain why I'd prefer to keep the test 
> this way just for this diff.
> Basically we need a refactoring that makes EMA returns MutationTrace in order 
> to fix this test util here.
> 
> But thinking more about this, maybe we don't need that (for now), this util 
> is to help restore a mutation chain, and since we don't support a chain of 
> mutation for pointers yet we can have a simpler version of `pointeeMutatedBy` 
> that doesn't support chaining.
> 
> I mentioned multi-layer pointer mutation because _if_ we were to do a 
> refactoring, we'd better just do it right and take possible features needed 
> in t

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

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;

JonasToth wrote:
> shuaiwang wrote:
> > JonasToth wrote:
> > > That doesn't look like a super idea. It is super hidden that variable 
> > > 'p*' will be analyzed for pointee mutation and others aren't. Especially 
> > > in 12 months and when someone else wants to change something, this will 
> > > be the source of headaches.
> > > 
> > > Don't you think there could be a better way of doing this switch?
> > Yeah... agree :)
> > But how about let's keep it this way just for now, and I'll pause the work 
> > on supporting pointee analysis and fix it properly but in a separate diff 
> > following this one?
> > 
> > I've been thinking about this and also did some prototyping, and here are 
> > my thoughts:
> > In order to properly fix this we need to change the interface of 
> > `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a 
> > `MutationTrace` with additional metadata. This wasn't needed before because 
> > we can reconstruct a mutation chain by continuously calling `findMutation`, 
> > and that works well for cases like "int x; int& r0 = x; int& r1 = r0; r1 = 
> > 123;". But now that pointers come into play, and we need to handle cases 
> > like "int *p; int *p2 = p; int& r = *p2; r = 123;", and in order to 
> > reconstruct the mutation chain, we need to do `findPointeeMutation(p)` -> 
> > `findPointeeMutation(p2)` -> `findMutation(r)`, notice that we need to 
> > switch from calling `findPointeeMutation` to calling `findMutation`. 
> > However we don't know where the switch should happen simply based on what's 
> > returned from `findPointeeMutation`, we need additional metadata for that.
> > 
> > That interface change will unavoidably affect how memoization is done so we 
> > need to change memoization as well.
> > 
> > Now since we need to change both the interface and memoization anyway, we 
> > might as well design them properly so that multiple-layers of pointers can 
> > be supported nicely. I think we can end up with something like this:
> > ```
> > class ExprMutationAnalyzer {
> > public:
> >   struct MutationTrace {
> > const Stmt *Value;
> > const MutationTrace *Next;
> > // Other metadata
> >   };
> > 
> >   // Finds whether any mutative operations are performed on the given 
> > expression after dereferencing `DerefLayers` times.
> >   const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
> >   const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> > };
> > ```
> > This involves quite some refactoring, and doing this refactoring before 
> > this diff and later rebasing seems quite some trouble that I'd like to 
> > avoid.
> > 
> > Let me know what do you think :)
> Is the second part of your answer part to adressing this testing issue?
> Given the whole Analyzer is WIP it is ok to postpone the testing change to 
> later for me. But I feel that this is a quality issue that more experience 
> clang develops should comment on because it still lands in trunk. Are you 
> aware of other patches then clang-tidy that rely on EMA?
> 
> Regarding you second part (its related to multi-layer pointer mutation?!):
> Having a dependency-graph that follows the general data flow _with_ mutation 
> annotations would be nice to have. There is probably even something in clang 
> (e.g. `CFG` is probably similar in idea, just with all execution paths), but 
> I don't know enough to properly judge here.
> 
> In my opinion it is ok, to not do the refactoring now and just support one 
> layer of pointer indirection. Especially in C++ there is usually no need for 
> multi-layer pointers but more a relict from C-style.
> C on the other hand relies on that.
> Designing EMA new for the more generalized functionality (if necessary) would 
> be of course great, but again: Better analyzer ppl should be involved then 
> me, even though I would still be very interested to help :)
The second part is more of trying to explain why I'd prefer to keep the test 
this way just for this diff.
Basically we need a refactoring that makes EMA returns MutationTrace in order 
to fix this test util here.

But thinking more about this, maybe we don't need that (for now), this util is 
to help restore a mutation chain, and since we don't support a chain of 
mutation for pointers yet we can have a simpler version of `pointeeMutatedBy` 
that doesn't support chaining.

I mentioned multi-layer pointer mutation because _if_ we were to do a 
refactoring, we'd better just do it right and take possible features needed in 
the future into account. But the refactoring itself is motivated by the need of 
fixing this testing util (and making the return value from `findMutation` 
generally more useful.)

BTW by `MutationTr

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D52308#1240680, @jkorous wrote:

> Sorry my bad. You are right, we aren't showing destructors in clangd for 
> normal classes. The case where I noticed is kind of a corner case with 
> template class.
>
>   
> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
>   ---
>   
> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template  T> struct foo {}; template<> struct foo {}; foo::~"}}}
>   ---
>   
> {"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}
>
>
>
>
>   {
> "detail": "void",
> "filterText": "~foo",
> "insertText": "~foo",
> "insertTextFormat": 1,
> "kind": 2,
> "label": " ~foo()",
> "sortText": "3f2c~foo",
> "textEdit": {
>   "newText": "~foo",
>   "range": {
> "end": {
>   "character": 76,
>   "line": 0
> },
> "start": {
>   "character": 76,
>   "line": 0
> }
>   }
> }
>   },
>   


Indeed, this looks like a bug to me.


Repository:
  rC Clang

https://reviews.llvm.org/D52308



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

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Posted to make sure it's visible that I've started doing this.
Still need to update the tests and check for the capability from the client 
(and fallback to SymbolInformation if client does not support the new 
implementation).

Nevertheless, it works and looks really good in VSCode.

Will update the thread when the code is ready for review.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52311



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

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D52308#1240642, @jkorous wrote:

> You might be right - I am assuming here that we want consistent behaviour 
> between constructors and destructors.
>
> IIUC ctors are currently skipped in code completions (in most cases) but they 
> also don't have any type associated while result of their call definitely has 
> some type.


Tricky.

**MyClass()** has a type, and should probably have return type MyClass, though 
it's pretty clear from the name (one could make the same argument about 
destructors).
Note that the "return type matches" ranking heuristics will trigger on these 
types so it's not just presentational, I think we should be including the types 
for constructors.

But class Derived : MyClass { Derived : **MyClass()** {} }; doesn't have a type 
(it's not an expression).

And MyClass::**MyClass()** has a type, but it's probably more likely that the 
user is going for a plain **MyClass::MyClass** (e.g. in a class-scoped using 
decl) which doesn't have a useful type.

> Currently we are showing destructors in code completion in clangd (with 
> detail "void") - that's actually how I noticed this.
> 
> I would assume that canonical type in function/method code completion is 
> "it's type" not "type of result of it's call". WDYT?

I don't think so, for better or worse it's result of it's call. Or rather: type 
of the expression that we guess the user's going to form, or that we suggest.
Again, this seems to give the best code completion presentation, and also best 
ranking results (when the expected type of the context is known)


Repository:
  rC Clang

https://reviews.llvm.org/D52308



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

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall, simark.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52311

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/FindSymbols.cpp
  clangd/FindSymbols.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/clients/clangd-vscode/package.json
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h

Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -47,7 +47,7 @@
 llvm::Expected>
 runWorkspaceSymbols(ClangdServer &Server, StringRef Query, int Limit);
 
-llvm::Expected>
+llvm::Expected>
 runDocumentSymbols(ClangdServer &Server, PathRef File);
 
 } // namespace clangd
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -118,7 +118,7 @@
   return std::move(*Result);
 }
 
-llvm::Expected>
+llvm::Expected>
 runDocumentSymbols(ClangdServer &Server, PathRef File) {
   llvm::Optional>> Result;
   Server.documentSymbols(File, capture(Result));
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
-"vscode": "^1.18.0"
+"vscode": "^1.27.0"
 },
 "categories": [
 "Programming Languages",
@@ -32,8 +32,8 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^4.0.0",
-"vscode-languageserver": "^4.0.0"
+"vscode-languageclient": "^5.1.0",
+"vscode-languageserver": "^5.1.0"
 },
 "devDependencies": {
 "typescript": "^2.0.3",
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -40,6 +40,7 @@
   InvalidRequest = -32600,
   MethodNotFound = -32601,
   InvalidParams = -32602,
+
   InternalError = -32603,
 
   ServerNotInitialized = -32002,
@@ -623,6 +624,38 @@
 
 llvm::json::Value toJSON(const Command &C);
 
+/// Represents programming constructs like variables, classes, interfaces etc.
+/// that appear in a document. Document symbols can be hierarchical and they
+/// have two ranges: one that encloses its definition and one that points to its
+/// most interesting range, e.g. the range of an identifier.
+struct DocumentSymbol {
+  /// The name of this symbol.
+  std::string name;
+
+  /// More detail for this symbol, e.g the signature of a function.
+  std::string detail;
+
+  /// The kind of this symbol.
+  SymbolKind kind;
+
+  /// Indicates if this symbol is deprecated.
+  bool deprecated;
+
+  /// The range enclosing this symbol not including leading/trailing whitespace
+  /// but everything else like comments. This information is typically used to
+  /// determine if the clients cursor is inside the symbol to reveal in the
+  /// symbol in the UI.
+  Range range;
+
+  /// The range that should be selected and revealed when this symbol is being
+  /// picked, e.g the name of a function. Must be contained by the `range`.
+  Range selectionRange;
+
+  /// Children of this symbol, e.g. properties of a class.
+  std::vector children;
+};
+llvm::json::Value toJSON(const DocumentSymbol &S);
+
 /// Represents information about programming constructs like variables, classes,
 /// interfaces etc.
 struct SymbolInformation {
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -448,6 +449,16 @@
   return std::move(Cmd);
 }
 
+llvm::json::Value toJSON(const DocumentSymbol &S) {
+  return json::Object{{"name", S.name},
+  {"detail", S.detail},
+  {"kind", static_cast(S.kind)},
+  {"deprecated", S.deprecated},
+  {"children", json::Array{S.children}},
+  {"range", S.range},
+  {"selectionRange", S.selectionRange}};
+}
+
 json::Value toJSON(const WorkspaceEdit &WE) {
   if (!WE.changes)
 return json::Object{};
Index: clangd/FindSymbols.h
===
--- clangd/FindSymbols.h
+++ clangd/FindSymbols.h
@@ -36,8 +36,7 @@
 
 /// Retrieves the symbols contained in the "main file" section of a

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Sorry my bad. You are right, we aren't showing destructors in clangd for normal 
classes. The case where I noticed is kind of a corner case with template class.

  
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
  ---
  
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template struct foo {}; template<> struct foo {}; foo::~"}}}
  ---
  
{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}



  {
"detail": "void",
"filterText": "~foo",
"insertText": "~foo",
"insertTextFormat": 1,
"kind": 2,
"label": " ~foo()",
"sortText": "3f2c~foo",
"textEdit": {
  "newText": "~foo",
  "range": {
"end": {
  "character": 76,
  "line": 0
},
"start": {
  "character": 76,
  "line": 0
}
  }
}
  },


Repository:
  rC Clang

https://reviews.llvm.org/D52308



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

[PATCH] D51809: [CUDA][HIP] Fix assertion in LookupSpecialMember

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


https://reviews.llvm.org/D51809



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Sorry, I didn't get time to review the patch properly, these are few stylistic 
comments. Hopefully, I'll be able to give more feedback when I get more time.




Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:21
+
+#define PRINT_DEBUG 1
+

Do you plan to submit the patch with debugging messages or are you just using 
these for better local debugging experience?

If you plan to upload the patch with the messages, please use `LLVM_DEBUG` (see 
`readability/IdentifierNamingCheck.cpp` for reference) and `llvm::dbgs()` ([[ 
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden | 
`` traits shouldn't be used ]] and should be replaced with their LLVM 
counterparts: `llvm::outs()`, `llvm::errs()`, etc). Also, I believe the 
messages should be more informative if you think debug info is really important 
here.



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:63
+
+if (!CurrentToken.hasValue())
+  return SourceLocation();

nit: `if (!CurrentToken)`



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:323
+
+static std::string &TrimRight(std::string &str) {
+  auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {

`llvm::StringRef::rtrim()`?

Also, naming `str` should be at least `Str`, `it1` -> `It`, `ch` -> `Ch` (or 
better `Symbol`) ...



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:331
+
+static std::string Concat(const std::vector &Decls,
+  StringRef Indent) {

Use `llvm::join` with `";\n" + Indent` as the `Separator`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:333
+  StringRef Indent) {
+  std::string R;
+  for (const StringRef &D : Decls)

`Range`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+  diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+  << static_cast(

How about `multiple declarations within a single statement hurts readability`?



Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+return TypeAndName + " = " + Initializer + ";";
+  }

JonasToth wrote:
> kbobyrev wrote:
> > JonasToth wrote:
> > > kbobyrev wrote:
> > > > This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. 
> > > > I don't think that it becomes cleaner (in fact, without spaces in 
> > > > between it looks cryptic). Consider formatting it or simply applying 
> > > > newlines (if there were no newlines inbetween before).
> > > I do not plan to do a lot of formatting here (maybe space or newline), 
> > > because that clang-format area.
> > While Clang-Tidy can apply Clang-Format on top of the Fix-Its, it will 
> > still look weird in the Fix-Its previews. While supporting proper 
> > formatting, in general, might be hard, it totally makes sense to do some 
> > basic formatting so that editor integration warnings would look better, for 
> > example.
> The current version adds a new line for each decl and keeps the indendation 
> (as the other check does).
> 
> Because it does the slicing on commas the manual/custom formatting of the 
> original code will stay. That might result in weird looking output for exotic 
> variable declarations. I would like to ignore these cases, what do you think 
> @kbobyrev ?
Yes, sure, it's hard (and probably impossible) to support the generic case. 
This approach sounds good to me, thanks!



Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-*- C++ 
-*-===//
+//

JonasToth wrote:
> kbobyrev wrote:
> > nit: space between clang-tidy (also, in .cpp file)
> That comes from the template `add_new_check.py`, do you want me to fix it 
> there?
Ah, okay, I was thinking whether it was a template issue (since I thought it's 
unlikely that one would edit the header template). Yes, it looks like a typo in 
the `clang-tidy` check adding tool implementation. It seems to be fixed in 
rL342601.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

You might be right - I am assuming here that we want consistent behaviour 
between constructors and destructors.

IIUC ctors are currently skipped in code completions (in most cases) but they 
also don't have any type associated while result of their call definitely has 
some type.

Currently we are showing destructors in code completion in clangd (with detail 
"void") - that's actually how I noticed this.

I would assume that canonical type in function/method code completion is "it's 
type" not "type of result of it's call". WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D52308



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

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52280



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

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

When you're *calling* a destructor, I believe the expression does have type 
void. Are we sure this is incorrect?

Calling a destructor is really unusual though. IIRC we decided to just not show 
them in clangd in member context (maybe this is broken or was never 
implemented, though).


Repository:
  rC Clang

https://reviews.llvm.org/D52308



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

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In https://reviews.llvm.org/D52266#1240304, @hans wrote:

> Sorry, I didn't realize we both set off to do this in parallel. I've 
> incorporated your changes into my patch.


No worries, I didn't do anything I wouldn't have done for reviewing this :-)

Thoughts on "As far as I can tell we do not make 
/https://reviews.llvm.org/owners/package/1/ and 
/https://reviews.llvm.org/owners/package/2/ imply /Gs -- we keep it at the 
default value of 4096 (?) Fixing this probably means increasing chrome's size 
(?)."?




Comment at: include/clang/Driver/CLCompatOptions.td:94
+def : CLFlag<"Gs">, HelpText<"Same as /Gs0">, Alias, 
AliasArgs<["0"]>;
+def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">, 
Alias;
 def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,

Want to add "(default 4096)" here?



Comment at: include/clang/Driver/CLCompatOptions.td:115
+// The _SLASH_O option handles all the /O flags, but we also provide separate 
aliased options to provide separate help messages.
+// FIXME: Not sure why we have -O0 here; MSVC doesn't support that.
+def _SLASH_O : CLJoined<"O">, HelpText<"Set several /O flags at once; e.g. 
'/O2y-' is the same as '/O2 /y-'">, MetaVarName<"">;

Move FIXME down one so it's next to the O0 flag?



Comment at: include/clang/Driver/CLCompatOptions.td:118
+def : CLFlag<"O0">, Alias, HelpText<"Disable optimization">;
+def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>, HelpText<"Optimize for 
small size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy)">;
+def : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>, HelpText<"Optimize for 
maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy)">;

Nit: Just "optimize for size" / "optimize for speed" is shorter



Comment at: include/clang/Driver/CLCompatOptions.td:123
+def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline 
functions as deemed beneficial by the compiler">;
+def : CLFlag<"Od">, Alias, HelpText<"Disable optimization">;
+def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>, HelpText<"No effect">;

Why not alias this to _SLASH_O, d like the rest?



Comment at: include/clang/Driver/CLCompatOptions.td:128
+def : CLFlag<"Os">, Alias<_SLASH_O>, AliasArgs<["s"]>, HelpText<"Optimize for 
small size over speed">;
+def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for 
speed over small size">;
+def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; 
use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">;

nit: I liked the old help text for the previous 4 more, it was more concise



Comment at: include/clang/Driver/CLCompatOptions.td:129
+def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for 
speed over small size">;
+def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; 
use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">;
+def : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, HelpText<"Enable frame 
pointer omission (x86 only)">;

nit: With this punctuation it looks ambiguous to me if the parenthetical refers 
to /O2 or /Ox.


https://reviews.llvm.org/D52266



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

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai, sammccall, ilya-biryukov.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, eraman.

Destructors don't have return type "void", they don't have any return type at 
all.


Repository:
  rC Clang

https://reviews.llvm.org/D52308

Files:
  Index/code-completion.cpp
  Index/complete-access-checks.cpp
  Index/complete-arrow-dot.cpp
  Index/complete-cxx-inline-methods.cpp
  Index/complete-qualified.cpp
  Index/complete-with-annotations.cpp
  Sema/SemaCodeComplete.cpp

Index: Index/complete-with-annotations.cpp
===
--- Index/complete-with-annotations.cpp
+++ Index/complete-with-annotations.cpp
@@ -19,5 +19,5 @@
 // CHECK: FieldDecl:{ResultType int}{TypedText member2} (35) ("another annotation", "some annotation")
 // CHECK: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79)
 // CHECK: ClassDecl:{TypedText X}{Text ::} (75)
-// CHECK: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79)
+// CHECK: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79)
 
Index: Index/complete-qualified.cpp
===
--- Index/complete-qualified.cpp
+++ Index/complete-qualified.cpp
@@ -17,4 +17,4 @@
 // CHECK-CC1: FieldDecl:{ResultType C}{TypedText c} (35)
 // CHECK-CC1: ClassDecl:{TypedText Foo} (35)
 // CHECK-CC1: CXXMethod:{ResultType Foo &}{TypedText operator=}{LeftParen (}{Placeholder const Foo &}{RightParen )}
-// CHECK-CC1: CXXDestructor:{ResultType void}{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
+// CHECK-CC1: CXXDestructor:{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
Index: Index/complete-cxx-inline-methods.cpp
===
--- Index/complete-cxx-inline-methods.cpp
+++ Index/complete-cxx-inline-methods.cpp
@@ -29,7 +29,7 @@
 // CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
-// CHECK-NEXT: CXXDestructor:{ResultType void}{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
+// CHECK-NEXT: CXXDestructor:{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
 // CHECK-NEXT: Completion contexts:
 // CHECK-NEXT: Dot member access
 // CHECK-NEXT: Container Kind: StructDecl
Index: Index/complete-arrow-dot.cpp
===
--- Index/complete-arrow-dot.cpp
+++ Index/complete-arrow-dot.cpp
@@ -22,33 +22,33 @@
 // CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-NOT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-NOT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK: Completion contexts:
 // CHECK-NEXT: Dot member access
 
 // CHECK-WITH-CORRECTION: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
 // CHECK-WITH-CORRECTION-NEXT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
 // CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-WITH-CORRECTION-NEXT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-WITH-CORRECTION-NEXT: Completion contexts:
 // CHECK-WITH-CORRECTION-NEXT: Dot member access
 
 // CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
 // CHECK-ARROW-TO-DOT-NOT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
 // CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
 // CHECK-ARROW-TO-DOT-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-ARROW-TO-DOT-NOT: CXXDestructor:{ResultType

r342614 - [PowerPC] [Clang] Add vector int128 pack/unpack builtins

[QingShan Zhang via cfe-commits]

Author: qshanz
Date: Wed Sep 19 22:04:57 2018
New Revision: 342614

URL: http://llvm.org/viewvc/llvm-project?rev=342614&view=rev
Log:
[PowerPC] [Clang] Add vector int128 pack/unpack builtins

unsigned long long builtin_unpack_vector_int128 (vector int128_t, int);
vector int128_t builtin_pack_vector_int128 (unsigned long long, unsigned long 
long);

Builtins should behave the same way as in GCC.

Patch By: wuzish (Zixuan Wu)
Differential Revision: https://reviews.llvm.org/D52074

Modified:
cfe/trunk/include/clang/Basic/BuiltinsPPC.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/builtins-ppc-error.c
cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
cfe/trunk/test/CodeGen/builtins-ppc-vsx.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsPPC.def?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def Wed Sep 19 22:04:57 2018
@@ -470,6 +470,10 @@ BUILTIN(__builtin_divde, "SLLiSLLiSLLi",
 BUILTIN(__builtin_divdeu, "ULLiULLiULLi", "")
 BUILTIN(__builtin_bpermd, "SLLiSLLiSLLi", "")
 
+// Vector int128 (un)pack
+BUILTIN(__builtin_unpack_vector_int128, "ULLiV1LLLii", "")
+BUILTIN(__builtin_pack_vector_int128, "V1LLLiULLiULLi", "")
+
 // FIXME: Obviously incomplete.
 
 #undef BUILTIN

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Sep 19 22:04:57 2018
@@ -11233,6 +11233,28 @@ Value *CodeGenFunction::EmitPPCBuiltinEx
 auto RetTy = ConvertType(BIRetType);
 return Builder.CreateBitCast(ShuffleCall, RetTy);
   }
+
+  case PPC::BI__builtin_pack_vector_int128: {
+bool isLittleEndian = getTarget().isLittleEndian();
+Value *UndefValue =
+llvm::UndefValue::get(llvm::VectorType::get(Ops[0]->getType(), 2));
+Value *Res = Builder.CreateInsertElement(
+UndefValue, Ops[0], (uint64_t)(isLittleEndian ? 1 : 0));
+Res = Builder.CreateInsertElement(Res, Ops[1],
+  (uint64_t)(isLittleEndian ? 0 : 1));
+return Builder.CreateBitCast(Res, ConvertType(E->getType()));
+  }
+
+  case PPC::BI__builtin_unpack_vector_int128: {
+ConstantInt *Index = cast(Ops[1]);
+Value *Unpacked = Builder.CreateBitCast(
+Ops[0], llvm::VectorType::get(ConvertType(E->getType()), 2));
+
+if (getTarget().isLittleEndian())
+  Index = ConstantInt::get(Index->getType(), 1 - Index->getZExtValue());
+
+return Builder.CreateExtractElement(Unpacked, Index);
+  }
   }
 }
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep 19 22:04:57 2018
@@ -2970,6 +2970,13 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
 return Diag(TheCall->getBeginLoc(), diag::err_ppc_builtin_only_on_pwr7)
<< TheCall->getSourceRange();
 
+  auto SemaVSXCheck = [&](CallExpr *TheCall) -> bool {
+if (!Context.getTargetInfo().hasFeature("vsx"))
+  return Diag(TheCall->getBeginLoc(), diag::err_ppc_builtin_only_on_pwr7)
+ << TheCall->getSourceRange();
+return false;
+  };
+
   switch (BuiltinID) {
   default: return false;
   case PPC::BI__builtin_altivec_crypto_vshasigmaw:
@@ -2988,6 +2995,11 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);
+  case PPC::BI__builtin_unpack_vector_int128:
+return SemaVSXCheck(TheCall) ||
+   SemaBuiltinConstantArgRange(TheCall, 1, 0, 1);
+  case PPC::BI__builtin_pack_vector_int128:
+return SemaVSXCheck(TheCall);
   }
   return SemaBuiltinConstantArgRange(TheCall, i, l, u);
 }

Modified: cfe/trunk/test/CodeGen/builtins-ppc-error.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-error.c?rev=342614&r1=342613&r2=342614&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c Wed Sep 19 22:04:57 2018
@@ -14,6 +14,7 @@ extern vector signed int vsi;
 extern vector signed int vui;
 extern vector float vf;
 extern vector unsigned char vuc;
+extern vector signed __int128 vsllli;
 
 void testInsertWord(void) {
   int index = 5;
@@ -67,3 +68,8 @@ void

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;

shuaiwang wrote:
> JonasToth wrote:
> > That doesn't look like a super idea. It is super hidden that variable 'p*' 
> > will be analyzed for pointee mutation and others aren't. Especially in 12 
> > months and when someone else wants to change something, this will be the 
> > source of headaches.
> > 
> > Don't you think there could be a better way of doing this switch?
> Yeah... agree :)
> But how about let's keep it this way just for now, and I'll pause the work on 
> supporting pointee analysis and fix it properly but in a separate diff 
> following this one?
> 
> I've been thinking about this and also did some prototyping, and here are my 
> thoughts:
> In order to properly fix this we need to change the interface of 
> `ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a 
> `MutationTrace` with additional metadata. This wasn't needed before because 
> we can reconstruct a mutation chain by continuously calling `findMutation`, 
> and that works well for cases like "int x; int& r0 = x; int& r1 = r0; r1 = 
> 123;". But now that pointers come into play, and we need to handle cases like 
> "int *p; int *p2 = p; int& r = *p2; r = 123;", and in order to reconstruct 
> the mutation chain, we need to do `findPointeeMutation(p)` -> 
> `findPointeeMutation(p2)` -> `findMutation(r)`, notice that we need to switch 
> from calling `findPointeeMutation` to calling `findMutation`. However we 
> don't know where the switch should happen simply based on what's returned 
> from `findPointeeMutation`, we need additional metadata for that.
> 
> That interface change will unavoidably affect how memoization is done so we 
> need to change memoization as well.
> 
> Now since we need to change both the interface and memoization anyway, we 
> might as well design them properly so that multiple-layers of pointers can be 
> supported nicely. I think we can end up with something like this:
> ```
> class ExprMutationAnalyzer {
> public:
>   struct MutationTrace {
> const Stmt *Value;
> const MutationTrace *Next;
> // Other metadata
>   };
> 
>   // Finds whether any mutative operations are performed on the given 
> expression after dereferencing `DerefLayers` times.
>   const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
>   const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
> };
> ```
> This involves quite some refactoring, and doing this refactoring before this 
> diff and later rebasing seems quite some trouble that I'd like to avoid.
> 
> Let me know what do you think :)
Is the second part of your answer part to adressing this testing issue?
Given the whole Analyzer is WIP it is ok to postpone the testing change to 
later for me. But I feel that this is a quality issue that more experience 
clang develops should comment on because it still lands in trunk. Are you aware 
of other patches then clang-tidy that rely on EMA?

Regarding you second part (its related to multi-layer pointer mutation?!):
Having a dependency-graph that follows the general data flow _with_ mutation 
annotations would be nice to have. There is probably even something in clang 
(e.g. `CFG` is probably similar in idea, just with all execution paths), but I 
don't know enough to properly judge here.

In my opinion it is ok, to not do the refactoring now and just support one 
layer of pointer indirection. Especially in C++ there is usually no need for 
multi-layer pointers but more a relict from C-style.
C on the other hand relies on that.
Designing EMA new for the more generalized functionality (if necessary) would 
be of course great, but again: Better analyzer ppl should be involved then me, 
even though I would still be very interested to help :)



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

JonasToth wrote:
> I feel that there a multiple tests missing:
> 
> - multiple levels of pointers `int ***`, `int * const *`
> - pointers to references `int &*`
> - references to pointers `int *&`
> - ensure that having a const pointer does no influence the pointee analysis 
> `int * const p = &i; *p = 42;`
> - a class with `operator*` + `operator->` const/non-const and the analysis 
> for pointers to that class
> - pointer returned from a function
> - non-const reference returned 
> ```
> int& foo(int *p) {
>   return *p;
> }
> ```
> 
for the multi-level pointer mutation: it would be enough to test, that the 
second layer is analyzed properly, and that the `int * >const< *` would be 
detected.


Repository:
  rC Clang

https://reviews.llvm.org/D52219



__

r342648 - [OPENMP] Add support for mapping memory pointed by member pointer.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Sep 20 06:54:02 2018
New Revision: 342648

URL: http://llvm.org/viewvc/llvm-project?rev=342648&view=rev
Log:
[OPENMP] Add support for mapping memory pointed by member pointer.

Added support for map(s, s.ptr[0:1]) kind of mapping.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp
cfe/trunk/test/OpenMP/target_map_messages.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=342648&r1=342647&r2=342648&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Sep 20 06:54:02 2018
@@ -6752,7 +6752,9 @@ private:
   MapBaseValuesArrayTy &BasePointers, MapValuesArrayTy &Pointers,
   MapValuesArrayTy &Sizes, MapFlagsArrayTy &Types,
   StructRangeInfoTy &PartialStruct, bool IsFirstComponentList,
-  bool IsImplicit) const {
+  bool IsImplicit,
+  ArrayRef
+  OverlappedElements = llvm::None) const {
 // The following summarizes what has to be generated for each map and the
 // types below. The generated information is expressed in this order:
 // base pointer, section pointer, size, flags
@@ -7023,7 +7025,6 @@ private:
 
 Address LB =
 CGF.EmitOMPSharedLValue(I->getAssociatedExpression()).getAddress();
-llvm::Value *Size = getExprTypeSize(I->getAssociatedExpression());
 
 // If this component is a pointer inside the base struct then we don't
 // need to create any entry for it - it will be combined with the 
object
@@ -7032,6 +7033,70 @@ private:
 IsPointer && EncounteredME &&
 (dyn_cast(I->getAssociatedExpression()) ==
  EncounteredME);
+if (!OverlappedElements.empty()) {
+  // Handle base element with the info for overlapped elements.
+  assert(!PartialStruct.Base.isValid() && "The base element is set.");
+  assert(Next == CE &&
+ "Expected last element for the overlapped elements.");
+  assert(!IsPointer &&
+ "Unexpected base element with the pointer type.");
+  // Mark the whole struct as the struct that requires allocation on 
the
+  // device.
+  PartialStruct.LowestElem = {0, LB};
+  CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(
+  I->getAssociatedExpression()->getType());
+  Address HB = CGF.Builder.CreateConstGEP(
+  CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(LB,
+  CGF.VoidPtrTy),
+  TypeSize.getQuantity() - 1, CharUnits::One());
+  PartialStruct.HighestElem = {
+  std::numeric_limits::max(),
+  HB};
+  PartialStruct.Base = BP;
+  // Emit data for non-overlapped data.
+  OpenMPOffloadMappingFlags Flags =
+  OMP_MAP_MEMBER_OF |
+  getMapTypeBits(MapType, MapTypeModifier, IsImplicit,
+ /*AddPtrFlag=*/false,
+ /*AddIsTargetParamFlag=*/false);
+  LB = BP;
+  llvm::Value *Size = nullptr;
+  // Do bitcopy of all non-overlapped structure elements.
+  for (OMPClauseMappableExprCommon::MappableExprComponentListRef
+   Component : OverlappedElements) {
+Address ComponentLB = Address::invalid();
+for (const OMPClauseMappableExprCommon::MappableComponent &MC :
+ Component) {
+  if (MC.getAssociatedDeclaration()) {
+ComponentLB =
+CGF.EmitOMPSharedLValue(MC.getAssociatedExpression())
+.getAddress();
+Size = CGF.Builder.CreatePtrDiff(
+CGF.EmitCastToVoidPtr(ComponentLB.getPointer()),
+CGF.EmitCastToVoidPtr(LB.getPointer()));
+break;
+  }
+}
+BasePointers.push_back(BP.getPointer());
+Pointers.push_back(LB.getPointer());
+Sizes.push_back(Size);
+Types.push_back(Flags);
+LB = CGF.Builder.CreateConstGEP(ComponentLB, 1,
+CGF.getPointerSize());
+  }
+  BasePointers.push_back(BP.getPointer());
+  Pointers.push_back(LB.getPointer());
+  Size = CGF.Builder.CreatePtrDiff(
+  CGF.EmitCastToVoidPtr(
+  CGF.Builder.CreateConstGEP(HB, 1, CharUnits::One())
+  .getPointer()),
+  CGF.EmitCastToVoidPtr(LB.getPointer()));
+  Sizes.push_back(Size);
+  Types.push_back(Flags);
+  break;
+}
+llvm::Value *Size = getExprTypeSize(I->getAssoc

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D52261#1240143, @yvvan wrote:

> I tried that first but did not I find a way just to copy an expression (we 
> basically need the same expr for such case). Do you know how to properly 
> generate a copy of expression or some other way to get the same expression?


It seems `Sema::ActOnStartCXXMemberReference` only changes expression when 
overloading for C++'s `operator ->` is required, otherwise it keeps the same 
expression. Since C does not have that, we can just leave the expression as is.
So setting `CorrectedLHS = LHS` for C should do the trick (no need to copy the 
expression IIUC, it's fine to use the same pointer for both `CorrectedLHS` and 
`LHS`).


https://reviews.llvm.org/D52261



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

[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Also not sure about the trick:

- Would be surprising to see the "ms" instead of "mbytes"
- Time tends to vary between runs, so using Google Benchmark's capabilities to 
run multiple times and report aggregated times seems useful. Not so much for 
the memory usage: it's deterministic and running multiple times is just a waste 
of time.

I wonder if a separate (and trivial) C++ binary that would load the index and 
report the index size is any worse than the benchmark hack? What are the pros 
of the latter?




Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:81
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?

Given the trick we use for display, how are we going to show **two** memory 
uses?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:154
   ::benchmark::RunSpecifiedBenchmarks();
+  return 0;
 }

Since `return 0` is implied for main in C++, there's no need to add one.
May add clarity though, so feel free to keep it!




Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:189
+  if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;

Return `llvm::Expected` instead of `Optional` and create errors with the 
specified text instead.
This is a slightly better option for the library code (callers can report 
errors in various ways, e.g. one can imagine reporting it in the LSP instead of 
stderr)



Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:200
+else
+  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+  } else {

Same here: just propagate the error to the caller.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:239
   for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
   return Bytes + BackingDataSize;

Just to clarify: `P.first.Data.size()` is the size of the arena for all the 
symbols?


https://reviews.llvm.org/D52047



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

[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52301

Files:
  include/clang/AST/PrettyPrinter.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/TypePrinter.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  test/Analysis/scopes-cfg-output.cpp
  test/Driver/Xarch.c
  test/SemaOpenCL/to_addr_builtin.cl

Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
 
 void test(void) {
   global int *glob;
@@ -43,15 +43,13 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
-  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
-  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
 }
Index: test/Driver/Xarch.c
===
--- test/Driver/Xarch.c
+++ test/Driver/Xarch.c
@@ -1,12 +1,9 @@
-// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2ONCE %s
-// O2ONCE: "-O2"
-// O2ONCE-NOT: "-O2"
+// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2> %t.log
+// RUN: grep ' "-O2" ' %t.log | count 1
+// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2> %t.log
+// RUN: not grep ' "-O2" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O2'" %t.log
+// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2> %t.log
+// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -o'" %t.log | count 2
+// RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -S'" %t.log
 
-// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2>&1 | FileCheck -check-prefix=O2NONE %s
-// O2NONE-NOT: "-O2"
-// O2NONE: argument unused during compilation: '-Xarch_i386 -O2'
-
-// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 -S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -S'
-// INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
Index: test/Analysis/scopes-cfg-output.cpp
===
--- test/Analysis/scopes-cfg-output.cpp
+++ test/Analysis/scopes-cfg-output.cpp
@@ -820,7 +820,7 @@
 // CHECK-NEXT:   3: __end1
 // CHECK-NEXT:   4: [B2.3] (ImplicitCastExpr, LValueToRValue, class A *)
 // CHECK-NEXT:   5: [B2.2] != [B2.4]
-// CHECK-NEXT:   T: for (auto &i : [B5.4]) {
+// CHECK-NEXT:   T: for (A &i : [B5.4]) {
 // CHECK: [B4.11];
 // CHECK-NEXT:}
 // CHECK-NEXT:   Preds (2): B3 B5
@@ -835,7 +835,7 @@
 // CHECK-NEXT:   2: __begin1
 // CHECK-NEXT:   3: [B4.2] (ImplicitCastExpr, LValueToRValue, class A *)
 // CHECK-NEXT:   4: *[B4.3]
-// CHECK-NEXT:   5: auto &i = *__begin1;
+// CHECK-NEXT:   5: A &i = *__begin1;
 // CHECK-NEXT:   6: operator=
 // CHECK-NEXT:   7: [B4.6] (ImplicitCastExpr, FunctionToPointerDecay, class A &(*)(const class A &)
 // CHECK-NEXT:   8: i
@@ -850,16 +850,16 @@
 // CHECK-NEXT:   2:  (CXXConstructExpr, [B5.3], class A [10])
 // CHECK-NEXT:   3: A a[10];
 // CHECK-NEXT:   4: a
-// CHECK-NEXT:   5: auto &&__range1 = a;
+// CHECK-NEXT:   5: A (&__range1)[10] = a;
 // CHECK-NEXT:   6: CFGScopeBegin(__end1)
 // CHECK-NEXT:   7: __range1
 // CHECK-NEXT:   8: [B5.7] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
 // CHECK-NEXT:   9: 10
 // CHECK-NEXT:  10: [B5.8] + [B5.9]
-// CHECK-NEXT:  11: auto __end1 = __range1 + 10
+// CHECK-NEXT:  11: A *__end1 = __range1 + 10
 // CHECK-NEXT:  12: __range1
 // CHECK-NEXT:  13: [B5.12] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
-

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

Another ping. Anyone up for reviewing this patch?


Repository:
  rC Clang

https://reviews.llvm.org/D51211



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

[PATCH] D52300: [clangd] Implement VByte PostingList compression

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 166278.
kbobyrev added a comment.

- Update unit tests with iterator tree string representation to comply with the 
new format
- Don't mark constructor `explicit` (previously it only had one parameter)
- Fix `Limits` explanation comment (`ID > Limits[I]` -> `ID >= Limits[I]`)


https://reviews.llvm.org/D52300

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h
  clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,14 @@
   const PostingList L4({0, 1, 5});
   const PostingList L5({});
 
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
 
   auto Nested =
   createAnd(createAnd(L1.iterator(), L2.iterator()),
 createOr(L3.iterator(), L4.iterator(), L5.iterator()));
 
-  EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+  EXPECT_EQ(llvm::to_string(*Nested),
+"(& (| [... 5] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
 }
 
 TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
@@ -0,0 +1,64 @@
+//===-- VByteFuzzer.cpp - Fuzz VByte Posting List encoding ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// \brief This file implements a function that runs clangd on a single input.
+/// This function is then linked into the Fuzzer library.
+///
+//===--===//
+
+#include "../Iterator.h"
+#include "../PostingList.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+using DocID = clang::clangd::dex::DocID;
+
+/// Transform raw byte sequence into list of DocIDs.
+std::vector generateDocuments(uint8_t *Data, size_t Size) {
+  std::vector Result;
+  DocID ID = 0;
+  for (size_t I = 0; I < Size; ++I) {
+size_t Offset = I % 4;
+if (Offset == 0 && I != 0) {
+  ID = 0;
+  Result.push_back(ID);
+}
+ID |= (Data[I] << Offset);
+  }
+  if (Size > 4 && Size % 4 != 0)
+Result.push_back(ID);
+  return Result;
+}
+
+/// This fuzzer checks that compressed PostingList contains can be successfully
+/// decoded into the original sequence.
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size) {
+  if (Size == 0)
+return 0;
+  const auto OriginalDocuments = generateDocuments(Data, Size);
+  if (OriginalDocuments.empty())
+return 0;
+  // Ensure that given sequence of DocIDs is sorted.
+  for (size_t I = 1; I < OriginalDocuments.size(); ++I)
+if (OriginalDocuments[I] <= OriginalDocuments[I - 1])
+  return 0;
+  const clang::clangd::dex::PostingList List(OriginalDocuments);
+  const auto DecodedDocuments = clang::clangd::dex::consume(*List.iterator());
+  // Compare decoded sequence against the original PostingList contents.
+  if (DecodedDocuments.size() != OriginalDocuments.size())
+LLVM_BUILTIN_TRAP;
+  for (size_t I = 0; I < DecodedDocuments.size(); ++I)
+if (DecodedDocuments[I].first != OriginalDocuments[I])
+  LLVM_BUILTIN_TRAP;
+  return 0;
+}
Index: clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
@@ -0,0 +1,19 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+set(LLVM_LINK_COMPONENTS Support)
+
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
+
+add_clang_executable(clangd-vbyte-fuzzer
+  EXCLUDE_FROM_ALL
+  VByteFuzzer.cpp
+  )
+
+target_link_libraries(clangd-vbyte-fuzzer
+  PRIVATE
+  clangBasic
+  clangDaemon
+  ${LLVM_LIB_FUZZING_ENGINE}
+  )
Index: clang-tools-extra/clangd/index/dex/PostingList.h
===
--- clang-tools-extra/clangd/index/dex/PostingList.h
+++ clang-tools-extra/clangd/index/dex/PostingList.h
@@ -6,13 +6,19 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-

[PATCH] D52300: [clangd] Implement VByte PostingList compression

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
kbobyrev edited the summary of this revision.

This patch implements Variable-length Byte compression of `PostingList`s to 
sacrifice some performance for lower memory consumption.

`PostingList` compression and decompression was extensively tested using fuzzer 
for multiple hours and runnning significant number of realistic 
`FuzzyFindRequests`. AddressSanitizer and UndefinedBehaviorSanitizer were used 
to ensure the correct behaviour.

Performance evaluation was conducted with recent LLVM symbol index (292k 
symbols) and the collection of user-recorded queries (5540 `FuzzyFindRequest` 
JSON dumps):

| Metrics | Before | After | Change (%) |
| --- | -- | - | -- |
| Memory consumption (index only), MB | 65 | 52| -20%   |
| Time to process queries, sec| 5.370  | 7.145 | +25%   |


https://reviews.llvm.org/D52300

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h
  clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp

Index: clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/VByteFuzzer.cpp
@@ -0,0 +1,64 @@
+//===-- VByteFuzzer.cpp - Fuzz VByte Posting List encoding ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// \brief This file implements a function that runs clangd on a single input.
+/// This function is then linked into the Fuzzer library.
+///
+//===--===//
+
+#include "../Iterator.h"
+#include "../PostingList.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+using DocID = clang::clangd::dex::DocID;
+
+/// Transform raw byte sequence into list of DocIDs.
+std::vector generateDocuments(uint8_t *Data, size_t Size) {
+  std::vector Result;
+  DocID ID = 0;
+  for (size_t I = 0; I < Size; ++I) {
+size_t Offset = I % 4;
+if (Offset == 0 && I != 0) {
+  ID = 0;
+  Result.push_back(ID);
+}
+ID |= (Data[I] << Offset);
+  }
+  if (Size > 4 && Size % 4 != 0)
+Result.push_back(ID);
+  return Result;
+}
+
+/// This fuzzer checks that compressed PostingList contains can be successfully
+/// decoded into the original sequence.
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size) {
+  if (Size == 0)
+return 0;
+  const auto OriginalDocuments = generateDocuments(Data, Size);
+  if (OriginalDocuments.empty())
+return 0;
+  // Ensure that given sequence of DocIDs is sorted.
+  for (size_t I = 1; I < OriginalDocuments.size(); ++I)
+if (OriginalDocuments[I] <= OriginalDocuments[I - 1])
+  return 0;
+  const clang::clangd::dex::PostingList List(OriginalDocuments);
+  const auto DecodedDocuments = clang::clangd::dex::consume(*List.iterator());
+  // Compare decoded sequence against the original PostingList contents.
+  if (DecodedDocuments.size() != OriginalDocuments.size())
+LLVM_BUILTIN_TRAP;
+  for (size_t I = 0; I < DecodedDocuments.size(); ++I)
+if (DecodedDocuments[I].first != OriginalDocuments[I])
+  LLVM_BUILTIN_TRAP;
+  return 0;
+}
Index: clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/fuzzer/CMakeLists.txt
@@ -0,0 +1,19 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+set(LLVM_LINK_COMPONENTS Support)
+
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
+
+add_clang_executable(clangd-vbyte-fuzzer
+  EXCLUDE_FROM_ALL
+  VByteFuzzer.cpp
+  )
+
+target_link_libraries(clangd-vbyte-fuzzer
+  PRIVATE
+  clangBasic
+  clangDaemon
+  ${LLVM_LIB_FUZZING_ENGINE}
+  )
Index: clang-tools-extra/clangd/index/dex/PostingList.h
===
--- clang-tools-extra/clangd/index/dex/PostingList.h
+++ clang-tools-extra/clangd/index/dex/PostingList.h
@@ -6,13 +6,19 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-//
-// This defines posting list interface: a storage for identifiers of 

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

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

This needs a test when calling in a `constexpr` function. I believe 
`std::invoke` is not `constepxr`, so a function object call in a `constexpr` 
function should not suggest `std::invoke`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



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

[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 166271.
kbobyrev added a comment.

Remove `BuildMem` benchmark, which collects data about `MemIndex` building time 
(which is essentially nothing and therefore not really interesting).


https://reviews.llvm.org/D52047

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/SymbolYAML.cpp
  clang-tools-extra/clangd/index/SymbolYAML.h
  clang-tools-extra/clangd/index/dex/Dex.cpp

Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
   for (const auto &TokenToPostingList : TempInvertedIndex)
 InvertedIndex.insert({TokenToPostingList.first,
   PostingList(move(TokenToPostingList.second))});
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-   estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
   for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
 // Read symbols from a YAML-format string.
 SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
 
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
 // Read one symbol from a YAML-stream.
 // The returned symbol is backed by Input.
 Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
 
 #include "SymbolYAML.h"
 #include "Index.h"
+#include "Logger.h"
 #include "Serialization.h"
 #include "Trace.h"
 #include "dex/Dex.h"
@@ -182,6 +183,28 @@
   return std::move(Syms).build();
 }
 
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+  if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+  }
+  StringRef Data = Buffer->get()->getBuffer();
+
+  llvm::Optional Slab;
+  if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+  Slab = std::move(RIFF->Symbols);
+else
+  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+  } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+  }
+  return Slab;
+}
+
 Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
   Symbol S;
   Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
   trace::Span OverallTracer("LoadIndex");
-  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
-  }
-  StringRef Data = Buffer->get()->getBuffer();
-
-  llvm::Optional Slab;
-  if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
-  Slab = std::move(RIFF->Symbols);
-else
-  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
-  } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
-  }
-
+  auto Slab = symbolsFromFile(SymbolFilename);
   if (!Slab)
 return nullptr;
   trace::Span Tracer("BuildIndex");
-  return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+  auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+  : MemIndex::build(std::move(*Slab), RefSlab());
+  vlog("Loaded {0} from {1} with estimated memory usage {2}",
+   UseDex ? "Dex" : "MemIndex", SymbolFilename,
+   Index->estimateMemoryUsage());
+  return Index;
 }
 
 } // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
   return loadIndex(IndexFilename, {}, true);
 }
 
+SymbolSlab loadSlab() {
+  auto Slab = symbolsFromFile(IndexFilename);
+  if (!Slab) {
+llvm::errs() << "Error when loading SymbolSlab from file: " << IndexFilenam

[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 166269.
kbobyrev added a comment.

Add benchmark for `SymbolSlab` loading from file. This might be useful for 
RIFF/YAML symbol loader optimizations.


https://reviews.llvm.org/D52047

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/SymbolYAML.cpp
  clang-tools-extra/clangd/index/SymbolYAML.h
  clang-tools-extra/clangd/index/dex/Dex.cpp

Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
   for (const auto &TokenToPostingList : TempInvertedIndex)
 InvertedIndex.insert({TokenToPostingList.first,
   PostingList(move(TokenToPostingList.second))});
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-   estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
   for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
 // Read symbols from a YAML-format string.
 SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
 
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
 // Read one symbol from a YAML-stream.
 // The returned symbol is backed by Input.
 Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
 
 #include "SymbolYAML.h"
 #include "Index.h"
+#include "Logger.h"
 #include "Serialization.h"
 #include "Trace.h"
 #include "dex/Dex.h"
@@ -182,6 +183,28 @@
   return std::move(Syms).build();
 }
 
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+  if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+  }
+  StringRef Data = Buffer->get()->getBuffer();
+
+  llvm::Optional Slab;
+  if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+  Slab = std::move(RIFF->Symbols);
+else
+  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+  } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+  }
+  return Slab;
+}
+
 Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
   Symbol S;
   Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
   trace::Span OverallTracer("LoadIndex");
-  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
-  }
-  StringRef Data = Buffer->get()->getBuffer();
-
-  llvm::Optional Slab;
-  if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
-  Slab = std::move(RIFF->Symbols);
-else
-  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
-  } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
-  }
-
+  auto Slab = symbolsFromFile(SymbolFilename);
   if (!Slab)
 return nullptr;
   trace::Span Tracer("BuildIndex");
-  return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+  auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+  : MemIndex::build(std::move(*Slab), RefSlab());
+  vlog("Loaded {0} from {1} with estimated memory usage {2}",
+   UseDex ? "Dex" : "MemIndex", SymbolFilename,
+   Index->estimateMemoryUsage());
+  return Index;
 }
 
 } // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
   return loadIndex(IndexFilename, {}, true);
 }
 
+SymbolSlab loadSlab() {
+  auto Slab = symbolsFromFile(IndexFilename);
+  if (!Slab) {
+llvm::errs() << "Error when loading SymbolSlab from file: " << IndexFilename
+ << '\n';
+e

[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates

[Dmitry Sidorov via Phabricator via cfe-commits]

sidorovd added inline comments.



Comment at: include/clang/Sema/Sema.h:8576
+
+  /// Find and extension in an extension map and return its name
+  template

Anastasia wrote:
> and extension -> an extension ?
Thanks!



Comment at: lib/Sema/Sema.cpp:1856
 
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+  if (!OpenCLDeclExtMap.empty())

Anastasia wrote:
> Is this function to be used for both `OpenCLDeclExtMap` and 
> `OpenCLTypeExtMap`? If yes, may be we could give it more generic name like 
> 'getOpenCLExtensionsFromExtMap'...
No, this exact function is only for 'OpenCLDeclExtMap', for the type map one 
should implement a new function 'getOpenCLExtensionsFromTypeExtMap'. Actually I 
have done this for https://reviews.llvm.org/D51341 to make the patch more 
generic, but since I'm not sure if it ever came to light I can add it here 
unused.


https://reviews.llvm.org/D52292



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

[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates

[Dmitry Sidorov via Phabricator via cfe-commits]

sidorovd updated this revision to Diff 166270.
sidorovd marked an inline comment as done.

https://reviews.llvm.org/D52292

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaOpenCL/extension-begin.cl


Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
   PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 
'const struct A *') requires my_ext extension to be enabled}}
   f(); // expected-error {{use of declaration 'f' requires my_ext extension to 
be enabled}}
   g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL 
extension 'my_ext' to be disabled}}
 // expected-note@-22 {{candidate function not viable: requires 0 
arguments, but 1 was provided}}
 }
 
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -10242,7 +10242,8 @@
   FunctionDecl *Callee = Cand->Function;
 
   S.Diag(Callee->getLocation(),
- diag::note_ovl_candidate_disabled_by_extension);
+ diag::note_ovl_candidate_disabled_by_extension)
+<< S.getOpenCLExtensionsFromDeclExtMap(Callee);
 }
 
 /// Generates a 'note' diagnostic for an overload candidate.  We've
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1853,6 +1853,27 @@
   setOpenCLExtensionForDecl(D, CurrOpenCLExtension);
 }
 
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+  if (!OpenCLDeclExtMap.empty())
+return getOpenCLExtensionsFromExtMap(FD, OpenCLDeclExtMap);
+
+  return "";
+}
+
+template 
+std::string Sema::getOpenCLExtensionsFromExtMap(T *FDT, MapT &Map) {
+  std::string ExtensionNames = "";
+  auto Loc = Map.find(FDT);
+
+  for (auto const& I : Loc->second) {
+ExtensionNames += I;
+ExtensionNames += " ";
+  }
+  ExtensionNames.pop_back();
+
+  return ExtensionNames;
+}
+
 bool Sema::isOpenCLDisabledDecl(Decl *FD) {
   auto Loc = OpenCLDeclExtMap.find(FD);
   if (Loc == OpenCLDeclExtMap.end())
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8567,6 +8567,16 @@
   llvm::StringRef getCurrentOpenCLExtension() const {
 return CurrOpenCLExtension;
   }
+
+  /// Check if a function declaration \p FD associates with any
+  /// extensions present in OpenCLDeclExtMap and if so return the
+  /// extension(s) name(s).
+  std::string getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD);
+
+  /// Find an extension in appropriate extension map and return its name
+  template
+  std::string getOpenCLExtensionsFromExtMap(T* FT, MapT &Map);
+
   void setCurrentOpenCLExtension(llvm::StringRef Ext) {
 CurrOpenCLExtension = Ext;
   }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3652,7 +3652,7 @@
 def note_ovl_candidate_disabled_by_function_cond_attr : Note<
 "candidate disabled: %0">;
 def note_ovl_candidate_disabled_by_extension : Note<
-"candidate disabled due to OpenCL extension">;
+"candidate unavailable as it requires OpenCL extension '%0' to be 
disabled">;
 def err_addrof_function_disabled_by_enable_if_attr : Error<
 "cannot take address of function %0 because it has one or more "
 "non-tautological enable_if conditions">;


Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -48,7 +48,7 @@
   PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const struct A *') requires my_ext extension to be enabled}}
   f(); // expected-error {{use of declaration 'f' requires my_ext extension to be enabled}}
   g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@-26 {{candidate disabled due to OpenCL extension}}
+// expected-note@-26 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be disabled}}
 // expected-note@-22 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
 }
 
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -10242,7 +10242,8 @@
   FunctionDecl *Callee = Cand->Function;
 
   S.Diag(Callee->getLocation(),
- diag::no

[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 166266.
kbobyrev retitled this revision from "[clangd] Add a "benchmark" for tracking 
memory" to "[clangd] Add building benchmark and memory consumption tracking".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.

Add symbol index building benchmarks, split `loadIndex()` into 
`symbolsFromFile()` and actual index build.


https://reviews.llvm.org/D52047

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/SymbolYAML.cpp
  clang-tools-extra/clangd/index/SymbolYAML.h
  clang-tools-extra/clangd/index/dex/Dex.cpp

Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
   for (const auto &TokenToPostingList : TempInvertedIndex)
 InvertedIndex.insert({TokenToPostingList.first,
   PostingList(move(TokenToPostingList.second))});
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-   estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
   for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes();
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
 // Read symbols from a YAML-format string.
 SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
 
+// Read symbols from YAML or RIFF file.
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename);
+
 // Read one symbol from a YAML-stream.
 // The returned symbol is backed by Input.
 Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
 
 #include "SymbolYAML.h"
 #include "Index.h"
+#include "Logger.h"
 #include "Serialization.h"
 #include "Trace.h"
 #include "dex/Dex.h"
@@ -182,6 +183,28 @@
   return std::move(Syms).build();
 }
 
+llvm::Optional symbolsFromFile(llvm::StringRef SymbolFilename) {
+  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+  if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
+return llvm::None;
+  }
+  StringRef Data = Buffer->get()->getBuffer();
+
+  llvm::Optional Slab;
+  if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
+if (auto RIFF = readIndexFile(Data))
+  Slab = std::move(RIFF->Symbols);
+else
+  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
+  } else {
+trace::Span Tracer("ParseYAML");
+Slab = symbolsFromYAML(Data);
+  }
+  return Slab;
+}
+
 Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
   Symbol S;
   Input >> S;
@@ -206,30 +229,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
   trace::Span OverallTracer("LoadIndex");
-  auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
-  }
-  StringRef Data = Buffer->get()->getBuffer();
-
-  llvm::Optional Slab;
-  if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
-  Slab = std::move(RIFF->Symbols);
-else
-  llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
-  } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
-  }
-
+  auto Slab = symbolsFromFile(SymbolFilename);
   if (!Slab)
 return nullptr;
   trace::Span Tracer("BuildIndex");
-  return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+  auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+  : MemIndex::build(std::move(*Slab), RefSlab());
+  vlog("Loaded {0} from {1} with estimated memory usage {2}",
+   UseDex ? "Dex" : "MemIndex", SymbolFilename,
+   Index->estimateMemoryUsage());
+  return Index;
 }
 
 } // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -33,6 +33,16 @@
   return loadIndex(IndexFilename, {}, true);
 }
 
+Symbol

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Christy Lee via Phabricator via cfe-commits]

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

Thanks for the fix!


Repository:
  rC Clang

https://reviews.llvm.org/D52280



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 166264.
riccibruno marked 3 inline comments as done.
riccibruno added a comment.

Removed the superfluous static_assert.


Repository:
  rC Clang

https://reviews.llvm.org/D52268

Files:
  lib/AST/Linkage.h


Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 
 namespace clang {
 /// Kinds of LV computation.  The linkage side of the computation is
@@ -36,6 +37,8 @@
   /// in computing linkage.
   unsigned IgnoreAllVisibility : 1;
 
+  enum { NumLVComputationKindBits = 3 };
+
   explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
   : ExplicitKind(EK), IgnoreExplicitVisibility(false),
 IgnoreAllVisibility(false) {}
@@ -78,12 +81,14 @@
   // using C = Foo;
   // using D = Foo;
   //
-  // The unsigned represents an LVComputationKind.
-  using QueryType = std::pair;
+  // The integer represents an LVComputationKind.
+  using QueryType =
+  llvm::PointerIntPair;
   llvm::SmallDenseMap CachedLinkageInfo;
 
   static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
   }
 
   llvm::Optional lookup(const NamedDecl *ND,


Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 
 namespace clang {
 /// Kinds of LV computation.  The linkage side of the computation is
@@ -36,6 +37,8 @@
   /// in computing linkage.
   unsigned IgnoreAllVisibility : 1;
 
+  enum { NumLVComputationKindBits = 3 };
+
   explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
   : ExplicitKind(EK), IgnoreExplicitVisibility(false),
 IgnoreAllVisibility(false) {}
@@ -78,12 +81,14 @@
   // using C = Foo;
   // using D = Foo;
   //
-  // The unsigned represents an LVComputationKind.
-  using QueryType = std::pair;
+  // The integer represents an LVComputationKind.
+  using QueryType =
+  llvm::PointerIntPair;
   llvm::SmallDenseMap CachedLinkageInfo;
 
   static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
   }
 
   llvm::Optional lookup(const NamedDecl *ND,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Headers/opencl-c.h:26
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#ifndef cl_intel_planar_yuv
+#define cl_intel_planar_yuv

Anastasia wrote:
> @yaxunl, do you think we need to add some kind of architecture guard for such 
> things? Like it should only be added if the architecture supports the 
> extension? But I guess `-cl-ext=+cl_intel_planar_yuv` trick might not work 
> here because it's not a Clang known extension?
> 
> So may be the right solution here is to introduce a target specific header? 
> For now it can be explicitly included but we could think of a target hook to 
> preload a target specific header...
@sidorovd, I am wondering if we could instead extend 
'-cl-ext=+cl_intel_planar_yuv' to work with non-builtin extensions? Would that 
be an acceptable solution for vendor extensions to be added to the common 
header?


https://reviews.llvm.org/D51402



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

[PATCH] D52292: [Sema][OpenCL] Improve diagnostics for not viable overloadable function candidates

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Sema/Sema.h:8576
+
+  /// Find and extension in an extension map and return its name
+  template

and extension -> an extension ?



Comment at: lib/Sema/Sema.cpp:1856
 
+std::string Sema::getOpenCLExtensionsFromDeclExtMap(FunctionDecl *FD) {
+  if (!OpenCLDeclExtMap.empty())

Is this function to be used for both `OpenCLDeclExtMap` and `OpenCLTypeExtMap`? 
If yes, may be we could give it more generic name like 
'getOpenCLExtensionsFromExtMap'...


Repository:
  rC Clang

https://reviews.llvm.org/D52292



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

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Basic/OpenCLExtensionTypes.def:27
+
+INTEL_SGAVC_TYPE(mce_payload_t, McePayload)
+INTEL_SGAVC_TYPE(ime_payload_t, ImePayload)

AlexeySotkin wrote:
> Anastasia wrote:
> > AlexeySachkov wrote:
> > > Anastasia wrote:
> > > > From the specification of this extension I can't quite see if these 
> > > > types have to be in Clang instead of the header. Can you please 
> > > > elaborate on any example where it wouldn't be possible for this type to 
> > > > be declared in the header using the technique explained in:
> > > > https://clang.llvm.org/docs/UsersManual.html#opencl-extensions 
> > > We cannot define these types in header because their layout is not 
> > > defined in specification, i.e. all of these types are opaque
> > This is not the reason to add functionality to Clang. You can easily sort 
> > such things with target specific headers or even general headers (see 
> > `ndrange_t` for example). Spec doesn't have to describe everything. The 
> > question is whether there is something about those types that can't be 
> > handled using standard include mechanisms. Usually it's prohibited 
> > behaviors that can't be represented with such mechanisms. Like if some 
> > operations have to be disallowed or allowed (since in OpenCL C you can't 
> > define user defined operators) with the types.
> > 
> > I think the trend is to avoid adding complexity into Clang, unless there is 
> > no other way to implement some feature correctly.
> Major part of these types must support initialization only by zero. 
> intel_sub_group_avc_mce_payload_t and intel_sub_group_avc_mce_result_t must 
> support initialization only via special builtins defined in the spec. 
> Corresponding errors must be reported. I think we can't implement this 
> behavior using standard include mechanism, can we?
> 
> Possible value of the additional complexity, except builtin declaration, is 
> that the patch is quite generic. So next time anyone wants to implement an 
> extension with a type restrictions which can't be handled with the include 
> mechanism, all that they needs to do is to modify this single file.
> Major part of these types must support initialization only by zero. 
> intel_sub_group_avc_mce_payload_t and intel_sub_group_avc_mce_result_t must 
> support initialization only via special builtins defined in the spec. 
> Corresponding errors must be reported. I think we can't implement this 
> behavior using standard include mechanism, can we?

Are these restrictions not mentioned in the specification document then? Or is 
it not the final version yet (not sure since it says First Draft). Do you plan 
to add the diagnostics for the restrictions afterwards? It doesn't have to be 
in the same patch, but just checking because if not I don't think it would make 
sense to go this route.

> Possible value of the additional complexity, except builtin declaration, is 
> that the patch is quite generic. So next time anyone wants to implement an 
> extension with a type restrictions which can't be handled with the include 
> mechanism, all that they needs to do is to modify this single file.
> 

It seems reasonable to implement this extra mechanism, provided that there are 
more of similar use cases.

Btw, considering that there are some modifications to core language spec 
restriction sections in this document, does this extension invalidate or change 
any core language rules that might affect parsing/diagnostics? 




Repository:
  rC Clang

https://reviews.llvm.org/D51484



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

[PATCH] D51464: clang: fix MIPS/N32 triple and paths

[Simon Atanasyan via Phabricator via cfe-commits]

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

This patch fails the following test cases:

- tools/clang/test/CodeGen/target-data.c
- tools/clang/test/Driver/mips-cs.cpp




Comment at: lib/Basic/Targets/Mips.h:75
+  : "n64";
+setABI(getTriple().isMIPS32() ? "o32" : Mips64Abi);
 

Let's write all cases in a uniform manner:
```
if (Triple.isMIPS32())
  setABI("o32");
else if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
  setABI("n32");
else
  setABI("n64");
```



Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:109
 
+  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+ABIName = "n32";

What about the following combination of a command line arguments?

  -target mips-mti-linux-gnuabin32 -mips64

In that case, ABIName is empty, Triple.getVendor() returns MipsTechnologies, 
CPUName is "mips64". So ABIName becomes "n64". And this new `if` statement 
doesn't work.



Comment at: lib/Driver/ToolChains/Gnu.cpp:2426
 if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
-getTriple().isAndroid() ||
-getTriple().isOSFreeBSD() ||
+getTriple().getEnvironment() == llvm::Triple::GNUABIN32 ||
+getTriple().isAndroid() || getTriple().isOSFreeBSD() ||

Before this change we enable integrated assembler for mips64/mips64el 
architectures only when we are sure that target ABI is N64. The problem is that 
there are some bugs which do not allow the integrated assembler generates 
correct N32 code in all cases. After this change we enable integrated assembler 
for N32 ABI. I do not think it's a good idea now.

If we can pass command line arguments to this routine, it probably would be 
possible to detect N32 ABI by checking both GNUABIN32  environment and 
`-mabi=n32` option. And disable integrated assembler for MIPS targets in that 
case only. But anyway this change is for another patch.


Repository:
  rC Clang

https://reviews.llvm.org/D51464



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