[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 148164.
yvvan marked 4 inline comments as done.

https://reviews.llvm.org/D41537

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/CodeCompleteOptions.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp

Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -166,3 +166,47 @@
   typename Template::Nested m;
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:166:25 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
 }
+
+class Proxy2 {
+public:
+  Derived *operator->() const;
+  int member5;
+};
+
+void test2(const Proxy2 &p) {
+  p->
+}
+
+void test3(const Proxy2 &p) {
+  p.
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:177:6 %s -o - | FileCheck -check-prefix=CHECK-CC8 --implicit-check-not="Derived : Derived(" %s
+// CHECK-CC8: Base1 : Base1::
+// CHECK-CC8: member1 : [#int#][#Base1::#]member1
+// CHECK-CC8: member1 : [#int#][#Base2::#]member1
+// CHECK-CC8: member2 : [#float#][#Base1::#]member2
+// CHECK-CC8: member3 : [#double#][#Base2::#]member3
+// CHECK-CC8: member4 : [#int#]member4
+// CHECK-CC8: member5 : [#int#]member5 (requires fix-it: {177:4-177:6} to ".")
+// CHECK-CC8: memfun1 : [#void#][#Base3::#]memfun1(<#float#>)
+// CHECK-CC8: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
+// CHECK-CC8: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>)
+// CHECK-CC8: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
+// CHECK-CC8: memfun3 : [#int#]memfun3(<#int#>)
+// CHECK-CC8: operator-> : [#Derived *#]operator->()[# const#] (requires fix-it: {177:4-177:6} to ".")
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:181:6 %s -o - | FileCheck -check-prefix=CHECK-CC9 --implicit-check-not="Derived : Derived(" %s
+// CHECK-CC9: Base1 : Base1::
+// CHECK-CC9: member1 : [#int#][#Base1::#]member1 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member1 : [#int#][#Base2::#]member1 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member2 : [#float#][#Base1::#]member2 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member3 : [#double#][#Base2::#]member3 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member4 : [#int#]member4 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member5 : [#int#]member5
+// CHECK-CC9: memfun1 : [#void#][#Base3::#]memfun1(<#float#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#] (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun2 : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#]
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1291,19 +1291,22 @@
   class CodeCompletionDeclConsumer : public VisibleDeclConsumer {
 ResultBuilder &Results;
 DeclContext *CurContext;
+std::vector FixIts;
 
   public:
-CodeCompletionDeclConsumer(ResultBuilder &Results, DeclContext *CurContext)
-  : Results(Results), CurContext(CurContext) { }
+CodeCompletionDeclConsumer(
+ResultBuilder &Results, DeclContext *CurContext,
+std::vector FixIts = std::vector())
+: Results(Results), CurContext(CurContext), FixIts(std::move(FixIts)) {}
 
 void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
bool InBaseClass) override {
   bool Accessible = true;
   if (Ctx)
 Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
 
   ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
-   false, Accessible);
+   false, Accessible, FixIts);
   Results.AddResult(Result, CurContext, Hiding, InBaseClass);
 }
 
@@ -3979,14 +3982,18 @@
 static void AddRecordMembersCompletionResults(Sema &SemaRef,
   ResultBuilder &Results, Scope *S,
   QualType BaseType,
-  RecordDecl *RD) {
+  RecordDecl *RD,
+  Optional AccessOpFixIt) {
   // Indicate that we are performing a member access, and the cv-qualifiers
   /

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/AST/ASTContext.cpp:1775
+case BuiltinType::UShortAccum:
   Width = Target->getShortWidth();
   Align = Target->getShortAlign();

Please give the types their own width and alignment accessors/variables in 
TargetInfo and use those instead of reusing the existing ones.



Comment at: lib/Sema/SemaType.cpp:1395
+  case DeclSpec::TST_accum: {
+if (S.getLangOpts().CPlusPlus) {
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_fixed_point_only_allowed_in_c);

This (and the rest of the fixed-point support) should be behind its own option. 
The error should reflect this as well.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D46960: [Fixed Point Arithmetic] Predefined Precision Macros

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: include/clang/Lex/Preprocessor.h:185
+  // Fractional bits of _Accum types
+  IdentifierInfo *Ident__SACCUM_FBIT__;// __SACCUM_FBIT__
+  IdentifierInfo *Ident__ACCUM_FBIT__; // __ACCUM_FBIT__

You should not reserve identifiers like this for built-in limit/precision 
macros. See InitPreprocessor.cpp to see how it's supposed to be done.

We have implemented all of these (at least for DSP-C, but it should not be 
difficult to port this to Embedded-C) in our downstream port. You will also 
need routines to print fixed-point numbers for the rest of the macros in 
7.18a.3. We can provide these patches on request.


Repository:
  rC Clang

https://reviews.llvm.org/D46960



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

Re: r332963 - Revert "Revert r332955 "GNUstep Objective-C ABI version 2""

[Hans Wennborg via cfe-commits]

Hi David,

On Tue, May 22, 2018 at 12:13 PM, David Chisnall via cfe-commits
 wrote:
> Author: theraven
> Date: Tue May 22 03:13:06 2018
> New Revision: 332963
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332963&view=rev
> Log:
> Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
>
> Added:
> cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m
> cfe/trunk/test/CodeGenObjC/gnu-init.m
> cfe/trunk/test/CodeGenObjC/gnustep2-category.m
> cfe/trunk/test/CodeGenObjC/gnustep2-class.m
> cfe/trunk/test/CodeGenObjC/gnustep2-ivar-offset.m
> cfe/trunk/test/CodeGenObjC/gnustep2-proto.m
> Modified:
> cfe/trunk/include/clang/AST/Expr.h
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/test/CodeGenObjC/constant-strings.m
> cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
> cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m
> cfe/trunk/test/Preprocessor/init.c

Do you want to add something about this to the release notes?

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

[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:206
 
+def err_invalid_saturation_spec : Error<"'%0' cannot be saturated. Only _Fract 
and _Accum can.">;
 def err_invalid_sign_spec : Error<"'%0' cannot be signed or unsigned">;

This error should be reworded.

Perhaps `'_Sat' specifier is only valid on '_Fract' or '_Accum', not '%0'`.



Comment at: lib/Sema/DeclSpec.cpp:1139
 
+  bool is_fixed_point_type =
+  (TypeSpecType == TST_accum || TypeSpecType == TST_fract);

This variable name should probably be a different case.



Comment at: lib/Sema/SemaType.cpp:1430
 } else {
-  switch (DS.getTypeSpecWidth()) {
-case DeclSpec::TSW_short:
-  Result = Context.UnsignedShortAccumTy;
-  break;
-case DeclSpec::TSW_unspecified:
-  Result = Context.UnsignedAccumTy;
-  break;
-case DeclSpec::TSW_long:
-  Result = Context.UnsignedLongAccumTy;
-  break;
-case DeclSpec::TSW_longlong:
-  // TODO: Replace with diag
-  llvm_unreachable("Unable to specify long long as _Accum width");
-  break;
+  if (DS.getTypeSpecSign() != DeclSpec::TSS_unsigned) {
+switch (DS.getTypeSpecWidth()) {

You should probably use the 'getCorrespondingSaturatingType' function instead 
of disambiguating the whole thing here again.



Comment at: test/Frontend/fixed_point.c:45
+_Sat _Fract sat_fract;
+_Sat long _Fract sat_long_fract;
+

What about typedefs with the _Sat specifier on them?


Repository:
  rC Clang

https://reviews.llvm.org/D46911



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

[clang-tools-extra] r333066 - [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Wed May 23 00:58:41 2018
New Revision: 333066

URL: http://llvm.org/viewvc/llvm-project?rev=333066&view=rev
Log:
[clang-tidy] new cppcoreguidelines-narrowing-conversions check.

Summary:
Checks for narrowing conversions, e.g.

int i = 0;
i += 0.1;

This has what some might consider false positives for:
i += ceil(d);

Reviewers: alexfh, hokein

Subscribers: srhines, nemanjai, mgorny, JDevlieghere, xazax.hun, kbarton

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

Added:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=333066&r1=333065&r2=333066&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Wed May 
23 00:58:41 2018
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
@@ -92,6 +93,8 @@ public:
 "bugprone-move-forwarding-reference");
 CheckFactories.registerCheck(
 "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-narrowing-conversions");
 CheckFactories.registerCheck(
 "bugprone-parent-virtual-call");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=333066&r1=333065&r2=333066&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Wed May 23 
00:58:41 2018
@@ -48,6 +48,7 @@ add_clang_library(clangTidyBugproneModul
   clangBasic
   clangLex
   clangTidy
+  clangTidyCppCoreGuidelinesModule
   clangTidyUtils
   clangTooling
   )

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=333066&r1=333065&r2=333066&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Wed May 
23 00:58:41 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyCppCoreGuidel
   AvoidGotoCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
+  NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=333066&r1=333065&r2=333066&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Wed May 23 00:58:41 2018
@@ -13,6 +13,7 @@
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
+#include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
@@ -40,6 +41,8 @@ public:
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
+CheckFactories.registerCheck(
+"cppcoreguidelines-narrowing-conversions");
 CheckFactories.registerCheck("cppcoreguidelines-no-malloc");
 CheckFactories.registerCheck(
 

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333066: [clang-tidy] new 
cppcoreguidelines-narrowing-conversions check. (authored by courbet, committed 
by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D38455

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp

Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
@@ -48,6 +48,7 @@
   clangBasic
   clangLex
   clangTidy
+  clangTidyCppCoreGuidelinesModule
   clangTidyUtils
   clangTooling
   )
Index: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
@@ -92,6 +93,8 @@
 "bugprone-move-forwarding-reference");
 CheckFactories.registerCheck(
 "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-narrowing-conversions");
 CheckFactories.registerCheck(
 "bugprone-parent-virtual-call");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -0,0 +1,70 @@
+//===--- NarrowingConversionsCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NarrowingConversionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+// FIXME: Check double -> float truncation. Pay attention to casts:
+void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) {
+  // ceil() and floor() are guaranteed to return integers, even though the type
+  // is not integral.
+  const auto IsCeilFloorCall = callExpr(callee(functionDecl(
+  hasAnyName("::ceil", "::std::ceil", "::floor", "::std::floor";
+
+  const auto IsFloatExpr =
+  expr(hasType(realFloatingPointType()), unless(IsCeilFloorCall));
+
+  // casts:
+  //   i = 0.5;
+  //   void f(int); f(0.5);
+  Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(isInteger()),
+  hasSourceExpression(IsFloatExpr),
+  unless(hasParent(castExpr())),
+  unless(isInTemplateInstantiation()))
+ .bind("cast"),
+ this);
+
+  // Binary operators:
+  //   i += 0.5;
+  Finder->addMatcher(
+  binaryOperator(isAssignmentOperator(),
+ // The `=` case generates an implicit cast which is covered
+ // by the previous matcher.
+ unless(hasOperatorName("=")),
+ hasLHS(hasType(isInteger())), hasRHS(IsFloatExpr),
+ unless(isInTemplateInstantiation()))
+  .bind("op"),
+  this);
+}
+
+void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Op = Result.Nodes.getNodeAs("op")) {
+if (Op->getLocStart().isMacroID())
+  return;
+diag(Op->getOperatorLoc(), "narrowing conversion from %0 to %1")
+<< Op->getRHS()->getType() << Op->getLHS()->getType();
+return;
+  }
+  co

[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak updated this revision to Diff 148169.
jolesiak added a comment.

- Add test


Repository:
  rC Clang

https://reviews.llvm.org/D47195

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
   verifyFormat("[((Foo *)foo) bar];");
   verifyFormat("[((Foo *)foo) bar:1 blech:2];");
 
+  // Message receiver taking multiple lines.
+  Style.ColumnLimit = 20;
+  // Non-corner case.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] a:42 b:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42 b:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42\n"
+   "bb:42];");
+  // Arguments just fit into one line.
+  Style.ColumnLimit = 23;
+  verifyFormat("[[obj a:42\n"
+   "  b:42\n"
+   "  c:42\n"
+   "  d:42] e:42 f:42];");
+
+  // Arguments do not fit into one line with a receiver.
+  Style.ColumnLimit = 20;
+  verifyFormat("[[obj a:42] a:42\n"
+   "b:42];");
+  verifyFormat("[[obj a:42] a:42\n"
+   "b:42\n"
+   "c:42];");
+  verifyFormat("[[obj aa:42\n"
+   "   b:42]\n"
+   "cc:42\n"
+   " d:42];");
+
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1386,6 +1386,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
 State.Stack.pop_back();
 
+  // Reevaluate whether ObjC message arguments fit into one line.
+  // If a receiver spans multiple lines, e.g.:
+  //   [[object block:^{
+  // return 42;
+  //   }] a:42 b:42];
+  // BreakBeforeParameter is calculated based on an incorrect assumption
+  // (it is checked whether the whole expression fits into one line without
+  // considering a line break inside a message receiver).
+  // We check whether arguements fit after receiver scope closer (into the same
+  // line).
+  if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+  int NecessarySpaceInLine =
+  getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+  CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+  if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+  Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+  }
+
   if (Current.is(tok::r_square)) {
 // If this ends the array subscript expr, reset the corresponding value.
 const FormatToken *NextNonComment = Current.getNextNonComment();


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
   verifyFormat("[((Foo *)foo) bar];");
   verifyFormat("[((Foo *)foo) bar:1 blech:2];");
 
+  // Message receiver taking multiple lines.
+  Style.ColumnLimit = 20;
+  // Non-corner case.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] a:42 b:42];");
+  // Arguments just fit into one line.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42 b:42];");
+  // Arguments just over a column limit.
+  verifyFormat("[[object block:^{\n"
+   "  return 42;\n"
+   "}] aaa:42\n"
+   "bb:42];");
+  // Arguments just fit into one line.
+  Style.ColumnLimit = 23;
+  verifyFormat("[[obj a:42\n"
+   "  b:42\n"
+   "  c:42\n"
+   "  d:42] e:42 f:42];");
+
+  // Arguments do not fit into one line with a receiver.
+  Style.ColumnLimit = 20;
+  verifyFormat("[[obj a:42] a:42\n"
+   "b:42];");
+  verifyFormat("[[obj a:42] a:42\n"
+   "b:42\n"
+   "c:42];");
+  verifyFormat("[[obj aa:42\n"
+   "   b:42]\n"
+   "cc:42\n"
+   " d:42];");
+
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationI

[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:815
+
+  // No line break before closing receiver's scope.
+  verifyFormat("[[obj a:42] a:42\n"

krasimir wrote:
> jolesiak wrote:
> > krasimir wrote:
> > > jolesiak wrote:
> > > > krasimir wrote:
> > > > > What's the receiver's scope in this comment referring to?
> > > > > Also, how would the old test cases be formatted?
> > > > For a receiver: `[obj a:42]` I meant `]` as a token closing the scope.
> > > > I'll rephrase the comment to be more precise.
> > > > 
> > > > Old tests were introduced in D46879. After this change the formatting 
> > > > will be the same as it was before D46879, i.e. the same as for last 
> > > > test touched in this change:
> > > > ```
> > > > [[obj aa:42
> > > >b:42]
> > > > cc:42
> > > >  d:42];
> > > > ```
> > > > even if
> > > > ```
> > > > [[obj aa:42
> > > >b:42]
> > > > cc:42 d:42];
> > > > ```
> > > > satisfies the column limit.
> > > Ah, I think  get it now: the new code should only apply to after object 
> > > blocks and not after object receivers? Is this the intention?
> > The intention is to put arguments into one line if they fit but only in the 
> > same line as last character of a receiver expression, e.g.:
> > ```
> > [[object block:^{
> >   return 42;
> > }] aa:42 bb:42];
> > ```
> > instead of
> > ```
> > [[object block:^{
> >   return 42;
> > }] aa:42
> >bb:42];
> > ```
> > but not
> > ```
> > [[obj a:42]
> > a:42 b:42];
> > ```
> > 
> > I think it gets a little bit too complicated for no reason. Let me revert 
> > D46879 and rebase this change.
> In that case, would this be allowed?
> ```
> //  limit:   V
> [[obj a:42
>   b:42
>   c:42
>   d:42] e:42 f:42]
> ```
Yes, I added this test.


Repository:
  rC Clang

https://reviews.llvm.org/D47195



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

[PATCH] D47108: [CodeGenCXX] Add -fforce-emit-vtables

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

In https://reviews.llvm.org/D47108#1109014, @rjmccall wrote:

> I thought we already had places in Sema that marked inline virtual methods as 
> used, instantiated templates, etc. for devirtualization purposes when 
> optimization was enabled.  Did we rip that out?


I only recall the emitting available_externally vtables opportunistically, that 
is emitting it only if all the inline virtual functions are present (and they 
are not hidden).
(https://reviews.llvm.org/D33437)

> The problem we've had over and over with devirtualization is that we have to 
> emit a perfect v-table because LLVM lacks a lot of the key vocabulary for 
> talking about incomplete information.  For example, if something weird 
> happens and we don't have a definition for an inline virtual method, ideally 
> we'd just say "well, you can't devirtualize this slot", then try to fix that 
> as incremental progress; but instead we have to get everything just right or 
> else disable the whole optimization.  Note that vague-linkage v-tables mean 
> that we'd also need to be able to say things like "there is an object with a 
> definition that looks like this, but its symbol is not available and you 
> can't emit it yourself".

That is correcty, my intention was that this flag would cause all inline 
virtual functions to be emitted. Can you give a hint how to achieve this in the 
sane way?


Repository:
  rL LLVM

https://reviews.llvm.org/D47108



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

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[Daniel Cederman via Phabricator via cfe-commits]

dcederman updated this revision to Diff 148170.
dcederman added a comment.

Added missing tests.


https://reviews.llvm.org/D47138

Files:
  lib/Driver/ToolChains/Arch/Sparc.cpp
  test/Driver/sparc-as.c

Index: test/Driver/sparc-as.c
===
--- test/Driver/sparc-as.c
+++ test/Driver/sparc-as.c
@@ -76,6 +76,66 @@
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC-V8PLUSD %s
 
+// RUN: %clang -mcpu=ma2100 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2150 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2155 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2450 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2455 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x5x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2080 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2085 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2480 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2485 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x8x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2.1 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2.2 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2.3 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
 // RUN: %clang -mcpu=leon2 -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC-V8 %s
@@ -90,26 +150,27 @@
 
 // RUN: %clang -mcpu=leon3 -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC-V8 %s
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
 
 // RUN: %clang -mcpu=ut699 -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC-V8 %s
 
 // RUN: %clang -mcpu=gr712rc -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC-V8 %s
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
 
 // RUN: %clang -mcpu=leon4 -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC-V8 %s
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
 
 // RUN: %clang -mcpu=gr740 -no-canonical-prefixes -target sparc \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC-V8 %s
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
 
 // SPARC: as{{.*}}" "-32" "-Av8" "-o"
 // SPARC-V8: as{{.*}}" "-32" "-Av8" "-o"
+// SPARC-LEON: as{

Re: r332963 - Revert "Revert r332955 "GNUstep Objective-C ABI version 2""

[David Chisnall via cfe-commits]

On 23 May 2018, at 08:38, Hans Wennborg  wrote:
> 
> Hi David,
> 
> On Tue, May 22, 2018 at 12:13 PM, David Chisnall via cfe-commits
>  wrote:
>> Author: theraven
>> Date: Tue May 22 03:13:06 2018
>> New Revision: 332963
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=332963&view=rev
>> Log:
>> Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
>> 
>> Added:
>>cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m
>>cfe/trunk/test/CodeGenObjC/gnu-init.m
>>cfe/trunk/test/CodeGenObjC/gnustep2-category.m
>>cfe/trunk/test/CodeGenObjC/gnustep2-class.m
>>cfe/trunk/test/CodeGenObjC/gnustep2-ivar-offset.m
>>cfe/trunk/test/CodeGenObjC/gnustep2-proto.m
>> Modified:
>>cfe/trunk/include/clang/AST/Expr.h
>>cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
>>cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
>>cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>>cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>>cfe/trunk/test/CodeGenObjC/constant-strings.m
>>cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
>>cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m
>>cfe/trunk/test/Preprocessor/init.c
> 
> Do you want to add something about this to the release notes?

Yes, I will do a bit nearer the time, once I’m sure how stable it is - I intend 
to treat it as experimental between now and the 7.0 release, but might want to 
keep it experimental until 8.0 (or back-port fixes to 7.1, given that it’s 
fairly self-contained within clang).

David

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

r333067 - Revert r333044 "Use zeroinitializer for (trailing zero portion of) large array initializers"

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed May 23 01:24:01 2018
New Revision: 333067

URL: http://llvm.org/viewvc/llvm-project?rev=333067&view=rev
Log:
Revert r333044 "Use zeroinitializer for (trailing zero portion of) large array 
initializers"

It caused asserts, see PR37560.

> Use zeroinitializer for (trailing zero portion of) large array initializers
> more reliably.
>
> Clang has two different ways it emits array constants (from InitListExprs and
> from APValues), and both had some ability to emit zeroinitializer, but neither
> was able to catch all cases where we could use zeroinitializer reliably. In
> particular, emitting from an APValue would fail to notice if all the explicit
> array elements happened to be zero. In addition, for large arrays where only 
> an
> initial portion has an explicit initializer, we would emit the complete
> initializer (which could be huge) rather than emitting only the non-zero
> portion. With this change, when the element would have a suffix of more than 8
> zero elements, we emit the array constant as a packed struct of its initial
> portion followed by a zeroinitializer constant for the trailing zero portion.
>
> In passing, I found a bug where SemaInit would sometimes walk the entire array
> when checking an initializer that only covers the first few elements; that's
> fixed here to unblock testing of the rest.
>
> Differential Revision: https://reviews.llvm.org/D47166

Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/CodeGen/init.c
cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
cfe/trunk/test/SemaCXX/aggregate-initialization.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=333067&r1=333066&r2=333067&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 01:24:01 2018
@@ -635,52 +635,6 @@ static ConstantAddress tryEmitGlobalComp
   return ConstantAddress(GV, Align);
 }
 
-static llvm::Constant *
-EmitArrayConstant(llvm::ArrayType *PreferredArrayType,
-  llvm::Type *CommonElementType, unsigned ArrayBound,
-  SmallVectorImpl &Elements,
-  llvm::Constant *Filler) {
-  // Figure out how long the initial prefix of non-zero elements is.
-  unsigned NonzeroLength = ArrayBound;
-  if (Elements.size() < NonzeroLength && Filler->isNullValue())
-NonzeroLength = Elements.size();
-  if (NonzeroLength == Elements.size()) {
-while (NonzeroLength > 0 && Elements[NonzeroLength - 1]->isNullValue())
-  --NonzeroLength;
-  }
-
-  if (NonzeroLength == 0)
-return llvm::ConstantAggregateZero::get(PreferredArrayType);
-
-  // If there's not many trailing zero elements, just emit an array
-  // constant.
-  if (NonzeroLength + 8 >= ArrayBound && CommonElementType) {
-Elements.resize(ArrayBound, Filler);
-return llvm::ConstantArray::get(
-llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
-  }
-
-  // Add a zeroinitializer array filler if we have trailing zeroes.
-  if (unsigned TrailingZeroes = ArrayBound - NonzeroLength) {
-assert(Elements.size() >= NonzeroLength &&
-   "missing initializer for non-zero element");
-Elements.resize(NonzeroLength + 1);
-auto *FillerType = PreferredArrayType->getElementType();
-if (TrailingZeroes > 1)
-  FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
-Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
-  }
-
-  // We have mixed types. Use a packed struct.
-  llvm::SmallVector Types;
-  Types.reserve(Elements.size());
-  for (llvm::Constant *Elt : Elements)
-Types.push_back(Elt->getType());
-  llvm::StructType *SType =
-  llvm::StructType::get(PreferredArrayType->getContext(), Types, true);
-  return llvm::ConstantStruct::get(SType, Elements);
-}
-
 /// This class only needs to handle two cases:
 /// 1) Literals (this is used by APValue emission to emit literals).
 /// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
@@ -880,6 +834,7 @@ public:
   llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
 llvm::ArrayType *AType =
 cast(ConvertType(ILE->getType()));
+llvm::Type *ElemTy = AType->getElementType();
 unsigned NumInitElements = ILE->getNumInits();
 unsigned NumElements = AType->getNumElements();
 
@@ -890,35 +845,55 @@ public:
 QualType EltType = CGM.getContext().getAsArrayType(T)->getElementType();
 
 // Initialize remaining array elements.
-llvm::Constant *fillC = nullptr;
-if (Expr *filler = ILE->getArrayFiller()) {
+llvm::Constant *fillC;
+if (Expr *filler = ILE->getArrayFiller())
   fillC = Emitter.tryEmitAbstractForMemory(filler, EltType);
-  if (!fillC)
-return nullptr;
-}
+else
+ 

Re: r332963 - Revert "Revert r332955 "GNUstep Objective-C ABI version 2""

[Hans Wennborg via cfe-commits]

Sounds good, thanks!

On Wed, May 23, 2018 at 10:26 AM, David Chisnall
 wrote:
> On 23 May 2018, at 08:38, Hans Wennborg  wrote:
>>
>> Hi David,
>>
>> On Tue, May 22, 2018 at 12:13 PM, David Chisnall via cfe-commits
>>  wrote:
>>> Author: theraven
>>> Date: Tue May 22 03:13:06 2018
>>> New Revision: 332963
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=332963&view=rev
>>> Log:
>>> Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
>>>
>>> Added:
>>>cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m
>>>cfe/trunk/test/CodeGenObjC/gnu-init.m
>>>cfe/trunk/test/CodeGenObjC/gnustep2-category.m
>>>cfe/trunk/test/CodeGenObjC/gnustep2-class.m
>>>cfe/trunk/test/CodeGenObjC/gnustep2-ivar-offset.m
>>>cfe/trunk/test/CodeGenObjC/gnustep2-proto.m
>>> Modified:
>>>cfe/trunk/include/clang/AST/Expr.h
>>>cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
>>>cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
>>>cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>>>cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>>>cfe/trunk/test/CodeGenObjC/constant-strings.m
>>>cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
>>>cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m
>>>cfe/trunk/test/Preprocessor/init.c
>>
>> Do you want to add something about this to the release notes?
>
> Yes, I will do a bit nearer the time, once I’m sure how stable it is - I 
> intend to treat it as experimental between now and the 7.0 release, but might 
> want to keep it experimental until 8.0 (or back-port fixes to 7.1, given that 
> it’s fairly self-contained within clang).
>
> David
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r333044 - Use zeroinitializer for (trailing zero portion of) large array initializers

[Hans Wennborg via cfe-commits]

We're hitting asserts after this in Chromium. I've filed PR37560 and
reverted in r333067.

On Wed, May 23, 2018 at 2:09 AM, Richard Smith via cfe-commits
 wrote:
> Author: rsmith
> Date: Tue May 22 17:09:29 2018
> New Revision: 333044
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333044&view=rev
> Log:
> Use zeroinitializer for (trailing zero portion of) large array initializers
> more reliably.
>
> Clang has two different ways it emits array constants (from InitListExprs and
> from APValues), and both had some ability to emit zeroinitializer, but neither
> was able to catch all cases where we could use zeroinitializer reliably. In
> particular, emitting from an APValue would fail to notice if all the explicit
> array elements happened to be zero. In addition, for large arrays where only 
> an
> initial portion has an explicit initializer, we would emit the complete
> initializer (which could be huge) rather than emitting only the non-zero
> portion. With this change, when the element would have a suffix of more than 8
> zero elements, we emit the array constant as a packed struct of its initial
> portion followed by a zeroinitializer constant for the trailing zero portion.
>
> In passing, I found a bug where SemaInit would sometimes walk the entire array
> when checking an initializer that only covers the first few elements; that's
> fixed here to unblock testing of the rest.
>
> Differential Revision: https://reviews.llvm.org/D47166
>
> Modified:
> cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/CodeGen/init.c
> cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
> cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46915: [Fixed Point Arithmetic] Set Fixed Point Precision Bits and Create Fixed Point Literals

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

You should not define the fixed-point precision as compiler macros at build 
configure time. The number of fractional bits (the scaling factor) should be 
added to TargetInfo as target-configurable variables/accessors, and an accessor 
should be added to ASTContext (we call it 'getFixedPointScale') to fetch the 
scaling factor of arbitrary fixed-point types.

As I mentioned on the mailing list, we would also like to solve the problem 
with different scaling factors on the signed and unsigned _Fract types. I'm not 
sure what the best approach is, but the simplest solution could perhaps be 
enabled with a target flag.

If the flag is true, the scaling factor of the unsigned _Fract types is the 
same as the signed ones and the MSB is padding. If the flag is false, the 
scaling factor of the unsigned _Fract types is one greater than the signed 
types and all of the bits are used.

-

Overall, the literal parsing code seems very ad-hoc. Fixed-point values are 
just integers, it should be possible to process them in exact precision instead 
of relying on host system implementation details (using double) and floating 
point types.

I could provide patches from our downstream port, but there are a number of 
hurdles:

- DSP-C does a bunch of extra 'type promotion' when parsing fixed-point literals
- literals in DSP-C cannot contain the 'exponent notation' so the parsing 
routine cannot handle this
- the routine uses utility functions added to APInt in our LLVM branch

If you are interested I can share anyway so you can see how we have done it.




Comment at: include/clang/Lex/LiteralSupport.h:72
 
+  enum FixedPointType { FPT_UNSPECIFIED, FPT_ACCUM, FPT_FRACT };
+

Is there a reason this is not added as two fields 'isAccum' and 'isFract'?



Comment at: include/clang/Lex/LiteralSupport.h:75
+  // We use separate fields for fixed point sizes b/c the isHalf/isLong 
booleans
+  // assume that this literal is an integral type instead of fixed point type.
+  enum FixedPointSize { FPS_UNSPECIFIED, FPS_SHORT, FPS_LONG };

Shouldn't the flags be amended instead?



Comment at: lib/AST/ASTContext.cpp:1788
 case BuiltinType::UShortAccum:
+case BuiltinType::ShortFract:
+case BuiltinType::UShortFract:

See my comments on the _Accum patch.



Comment at: lib/AST/ASTDumper.cpp:2186
+  ColorScope Color(*this, ValueColor);
+  OS << " " << Node->getValue().toString(10, isSigned);
+}

This will not print as a fixed-point number.



Comment at: lib/AST/Expr.cpp:766
+  const auto *BT = type->getAs();
+  assert(BT && "Not a fixed point type!");
+  switch (BT->getKind()) {

Is this possible given the assertion above?



Comment at: lib/AST/Expr.cpp:767
+  assert(BT && "Not a fixed point type!");
+  switch (BT->getKind()) {
+default:

There is no reason to have this switch.



Comment at: lib/AST/Expr.cpp:774
+case BuiltinType::UShortFract:
+  assert(V.getBitWidth() == C.getIntWidth(type) &&
+ "Short fixed point type is not the correct size for constant.");

'getIntWidth' is likely the wrong accessor to use here. Fixed-point types are 
technically not ints.



Comment at: lib/AST/ExprConstant.cpp:9323
+  if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow() &&
+  !HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
+  E->getType()))

Is signed fixed-point overflow actually considered undefined behavior? I'm not 
familiar with what Embedded-C says about this.

Also, this routine will likely print the wrong number for fixed-point values.



Comment at: lib/AST/ExprConstant.cpp:9328
+}
+case UO_Not: {
+  if (!Visit(E->getSubExpr())) return false;

I do not believe ~ is valid on fixed-point types.



Comment at: lib/AST/StmtPrinter.cpp:1532
+  bool isSigned = Node->getType()->isSignedFixedPointType();
+  OS << Node->getValue().toString(10, isSigned);
+

This will not print a fixed-point number.



Comment at: lib/CodeGen/CGExprAgg.cpp:675
+  case CK_IntegralToFixedPoint:
+llvm_unreachable(
+"AggExprEmitter::VisitCastExpr CK_IntegralToFixedPoint");  // TODO

This probably goes in the large default case below, as fixed-point types are 
not aggregates.



Comment at: lib/CodeGen/CGExprComplex.cpp:451
+llvm_unreachable(
+"ComplexExprEmitter::EmitCast CK_IntegralToFixedPoint");  // TODO
   case CK_Dependent: llvm_unreachable("dependent cast kind in IR gen!");

Same as the aggregate case.



Comment at: lib/CodeGen/CGExprScalar.cpp:1789
+  case BuiltinType::ShortAccum:
+fbits = B

[PATCH] D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:2141
+  case BuiltinType::ShortAccum:
+fbits = BUILTIN_SACCUM_FBIT;
+break;

Please see my comments on other patches about these values.



Comment at: lib/CodeGen/CGExprScalar.cpp:2178
+llvm::Value *amt =
+llvm::ConstantInt::get(value->getType(), 1 << fbits,
+   /*isSigned=*/type->isSignedFixedPointType());

Use an APInt with the right size here instead of relying on types in the 
compiler.


Repository:
  rC Clang

https://reviews.llvm.org/D46917



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

Re: r333044 - Use zeroinitializer for (trailing zero portion of) large array initializers

[Richard Smith via cfe-commits]

OK, thanks for the revert.

On Wed, 23 May 2018, 01:28 Hans Wennborg via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:

> We're hitting asserts after this in Chromium. I've filed PR37560 and
> reverted in r333067.
>
> On Wed, May 23, 2018 at 2:09 AM, Richard Smith via cfe-commits
>  wrote:
> > Author: rsmith
> > Date: Tue May 22 17:09:29 2018
> > New Revision: 333044
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=333044&view=rev
> > Log:
> > Use zeroinitializer for (trailing zero portion of) large array
> initializers
> > more reliably.
> >
> > Clang has two different ways it emits array constants (from
> InitListExprs and
> > from APValues), and both had some ability to emit zeroinitializer, but
> neither
> > was able to catch all cases where we could use zeroinitializer reliably.
> In
> > particular, emitting from an APValue would fail to notice if all the
> explicit
> > array elements happened to be zero. In addition, for large arrays where
> only an
> > initial portion has an explicit initializer, we would emit the complete
> > initializer (which could be huge) rather than emitting only the non-zero
> > portion. With this change, when the element would have a suffix of more
> than 8
> > zero elements, we emit the array constant as a packed struct of its
> initial
> > portion followed by a zeroinitializer constant for the trailing zero
> portion.
> >
> > In passing, I found a bug where SemaInit would sometimes walk the entire
> array
> > when checking an initializer that only covers the first few elements;
> that's
> > fixed here to unblock testing of the rest.
> >
> > Differential Revision: https://reviews.llvm.org/D47166
> >
> > Modified:
> > cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> > cfe/trunk/lib/Sema/SemaInit.cpp
> > cfe/trunk/test/CodeGen/init.c
> > cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
> > cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
> ___
> 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] D47247: Fix unaligned memory access when reading INPUT_FILE_OFFSETS data

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333069: Fix unaligned memory access when reading 
INPUT_FILE_OFFSETS data (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47247?vs=148158&id=148173#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47247

Files:
  cfe/trunk/lib/Serialization/ASTReader.cpp


Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -4811,7 +4811,8 @@
 
   unsigned NumInputFiles = Record[0];
   unsigned NumUserFiles = Record[1];
-  const uint64_t *InputFileOffs = (const uint64_t *)Blob.data();
+  const llvm::support::unaligned_uint64_t *InputFileOffs =
+  (const llvm::support::unaligned_uint64_t *)Blob.data();
   for (unsigned I = 0; I != NumInputFiles; ++I) {
 // Go find this input file.
 bool isSystemFile = I >= NumUserFiles;


Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -4811,7 +4811,8 @@
 
   unsigned NumInputFiles = Record[0];
   unsigned NumUserFiles = Record[1];
-  const uint64_t *InputFileOffs = (const uint64_t *)Blob.data();
+  const llvm::support::unaligned_uint64_t *InputFileOffs =
+  (const llvm::support::unaligned_uint64_t *)Blob.data();
   for (unsigned I = 0; I != NumInputFiles; ++I) {
 // Go find this input file.
 bool isSystemFile = I >= NumUserFiles;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r333069 - Fix unaligned memory access when reading INPUT_FILE_OFFSETS data

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Wed May 23 02:02:40 2018
New Revision: 333069

URL: http://llvm.org/viewvc/llvm-project?rev=333069&view=rev
Log:
Fix unaligned memory access when reading INPUT_FILE_OFFSETS data

Summary: The blob data is unaligned, so we also should read it as such. Should 
fix the random failures with the sanitizer builds.

Reviewers: rsmith, v.g.vassilev

Reviewed By: v.g.vassilev

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=333069&r1=333068&r2=333069&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed May 23 02:02:40 2018
@@ -4811,7 +4811,8 @@ bool ASTReader::readASTFileControlBlock(
 
   unsigned NumInputFiles = Record[0];
   unsigned NumUserFiles = Record[1];
-  const uint64_t *InputFileOffs = (const uint64_t *)Blob.data();
+  const llvm::support::unaligned_uint64_t *InputFileOffs =
+  (const llvm::support::unaligned_uint64_t *)Blob.data();
   for (unsigned I = 0; I != NumInputFiles; ++I) {
 // Go find this input file.
 bool isSystemFile = I >= NumUserFiles;


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

[PATCH] D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:2178
+llvm::Value *amt =
+llvm::ConstantInt::get(value->getType(), 1 << fbits,
+   /*isSigned=*/type->isSignedFixedPointType());

ebevhan wrote:
> Use an APInt with the right size here instead of relying on types in the 
> compiler.
Also, what about saturating types?


Repository:
  rC Clang

https://reviews.llvm.org/D46917



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

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D46943#1107880, @ilya-biryukov wrote:

> I've added an initial version of testing for the matching header and wanted 
> to get feedback before proceeding further with tests and other changes.
>
> A few things that bug me so far:
>
> - We need to match headers of items from the index, not only from the Sema 
> results.


This sounds reasonable.

> - Symbols store their paths as URIs ⇒ we need to parse them in order to apply 
> heuristics. We could avoid that by writing a version of header-matching that 
> also works on URIs, but that would mean more complexity.

Yeah, this is a bit unfortunate. It's probably OK to parse URIs; they are not 
that expensive after all (we might want to do some measurement though).

> - Merging quality signals from headers now requires an extra paramater: name 
> of the source file. I wonder if it's worth extracting builders for symbol 
> qualities into separate classes to keep the current nice signatures, i.e. 
> `merge(Symbol& IndexResult)`.

I'm not very familiar with `SymbolQualitySignals`. But if we decide to use main 
file name as a signal, it might make sense to pass it in through the 
constructor?

> - How should we match the header with the main file?  Our options are:
>   - (proposed by Eric) use main file regex from clang-format for that. I'm 
> not entirely sure it suits us well, since the regex is designed to work on 
> paths inside #include directive, but we're getting ours from the Symbols and 
> Sema AST Decls. Moreover, it means we're gonna read .clang-format files to 
> get that style.

I think the ".clang-format problem" is not specific to the header matching 
here. We would eventually need proper format style support in clangd anyway, as 
clangd provides formatting features (e.g. reformat and include insertion).

> - Come up with our own heuristics. There is a similar place in ClangdServer 
> that matches a header with source and back. We could extend those heuristics 
> to also allow figuring out whether the paths are matching header/source. I 
> chose this option for initial implementation, since it's less work and it 
> seems easier to switch to clang-format's regex later.

Not against this option. Just want to point out that the heuristics would not 
work for test files (that include tested main headers) with the current 
matching.




Comment at: clangd/MatchingHeader.cpp:44
+  auto HeaderNoExt = llvm::sys::path::filename(Header);
+  return SourceNoExt.equals_lower(HeaderNoExt);
+}

Why `equals_lower`?



Comment at: clangd/MatchingHeader.h:1
+//===--- MatchingHeader.h - Match source and header files *- 
C++-*-===//
+//

I wonder if we could merge this into Headers.h



Comment at: clangd/MatchingHeader.h:24
+/// header for a \p Source.
+bool isMainHeaderFor(PathRef Header, PathRef Source);
+

We might want to briefly explain what a matching header is and what the 
heuristics are.



Comment at: clangd/Quality.cpp:28-29
+struct DeclFiles {
+  bool AllDeclsInMainFile = false;
+  bool HasDeclsInMatchingHeader = false;
+};

Could we merge these two flags into something like 
`IsDeclaredInMainHeaderOrFile`?



Comment at: clangd/Quality.cpp:40
+  assert(MainFile);
+  for (auto *Redecl : D->redecls()) {
+auto Loc = SM.getSpellingLoc(Redecl->getLocation());

I wonder if it's still necessary to check all `redecls`. Would it be sufficient 
to check `D`, if `D` is the decl we referencing to?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46943



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

[PATCH] D46927: [Fixed Point Arithmetic] Augmented Assignment for Fixed Point Types

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: include/clang/AST/Type.h:6577
+
+inline unsigned getFixedPointFBits(const QualType &Ty) {
+  return getFixedPointFBits(*Ty);

As I mentioned in another patch, this should be in ASTContext and ask 
TargetInfo for the information.

I'm not a fan of the term 'fbits'. The correct terminology should be the 
'scaling factor'.



Comment at: lib/CodeGen/CGExprScalar.cpp:1009
+  // a float and perform some floating point arithmetic, though this may cost
+  // more. Enable that if #pragma FX_FULL_PRECISION is provided.
+  if (dest_fbits > src_fbits) {

For future reference, please do not rely on the behavior of floating point for 
fixed-point operations.



Comment at: lib/CodeGen/CGExprScalar.cpp:1184
+// Casting down, so we will need to shift early as to not lose data
+Src = EmitFixedPointRadixShift(Src, SrcType, DstType);
+  }

I think it's safer to simply do all relevant fixed-point conversion by hand 
here instead of falling through and relying on the semantics of other 
conversions.

We can provide patches for our implementation here if wanted.



Comment at: lib/CodeGen/CGExprScalar.cpp:1214
 
+  if (WorkingOnFixedPoints && order >= 0) {
+// Casting up (or same type), so we can safely shift without losing data

This case should not be needed. Perform all fixed-point conversion before the 
integer type check above and do an early return instead.

It's not safe to rely on the behavior of other conversions for the behavior of 
these.


Repository:
  rC Clang

https://reviews.llvm.org/D46927



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

[PATCH] D46986: [Fixed Point Arithmetic] Validation Test for Fixed Point Binary Operations and Saturated Addition

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

I cannot say that I'm pleased with the CodeGen emission of the operations as 
pure IR. I can only assume that you do not have hardware specifically tailored 
for these operations, as matching this type of code ought to be quite difficult 
after optimization is performed.




Comment at: include/clang/AST/Type.h:6591
+// saturated, return back the type itself.
+QualType getCorrespondingSaturatedFixedPointType(ASTContext &Context,
+ const Type &Ty);

If these need to be passed a ASTContext anyway, why not have these functions on 
ASTContext to begin with?



Comment at: include/clang/Basic/FixedPoint.h.in:35
+// Max values of each _Accum type as integer bytes
+#define SACCUM_MAX_AS_INT   ((1ULL << (BUILTIN_SACCUM_FBIT + 
BUILTIN_SACCUM_IBIT)) - 1)
+#define ACCUM_MAX_AS_INT((1ULL << (BUILTIN_ACCUM_FBIT + 
BUILTIN_ACCUM_IBIT)) - 1)

As mentioned in other patches, these should not be macros (this entire file 
should probably be removed altogether).



Comment at: lib/CodeGen/CGExprScalar.cpp:3151
+const auto &BT = op.Ty->getAs();
+switch (BT->getKind()) {
+  default:

All of these values can clearly be calculated based on the scaling factor and 
the width of the type.



Comment at: lib/CodeGen/CGExprScalar.cpp:3206
+if (op.Ty->isSignedFixedPointType()) {
+  MSBBitShift = getFixedPointIBits(op.Ty) + getFixedPointFBits(op.Ty);
+} else {

Factor out conversion between fixed-point types into its own function so it can 
be reused for other cases, such as for other operations and actual conversions. 
It should probably not take QualTypes to convert to but rather arbitrary widths 
and scales, so it can be used to upscale to/downscale from 'intermediate' 
common calculation types.



Comment at: lib/CodeGen/CGExprScalar.cpp:3211
+
+llvm::Value *Sum = Builder.CreateAdd(op.LHS, op.RHS);
+llvm::Value *LHSMSB = Builder.CreateLShr(op.LHS, MSBBitShift);

I would much rather see these operations emitted as intrinsics rather than 
straight IR... but I know that wasn't part of your proposal.



Comment at: lib/Sema/SemaExpr.cpp:1264
+/// integers or other fixed point types due to potential loss of precision.
+/// For this case of fixed point types, the resulting type in a binary 
operation
+/// does not need to be exactly one of the 2 operand types.

This is incorrect. The resulting type of a binary operation is absolutely one 
of the two operands. However, the calculation might be done as a type that is 
not one of the two, as it must be done in the full (combined) precision of both 
operands.

That is a detail reserved for CodeGen, however.



Comment at: lib/Sema/SemaExpr.cpp:3551
 uint64_t int_part_as_int = static_cast(int_part);
+uint64_t fract_part_as_int =
+static_cast(fract_part * (1ULL << fbits));

Should these changes not be part of the patch that adds the literal parsing 
code?


Repository:
  rC Clang

https://reviews.llvm.org/D46986



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

[PATCH] D46986: [Fixed Point Arithmetic] Validation Test for Fixed Point Binary Operations and Saturated Addition

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

Also, this patch and all of the following 'Validation Test' patches do much 
more than just add tests, they add plenty of functionality as well. In general, 
it's quite difficult to tell which patches add what.

I think it would be much clearer if the patches that claim to add tests only 
add tests.


Repository:
  rC Clang

https://reviews.llvm.org/D46986



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

[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 148177.
yvvan added a comment.

Output range of token being replaced instead of it's text

As a side-effect introduce a call to get a Token starting from specified 
location.


https://reviews.llvm.org/D46862

Files:
  include/clang-c/Index.h
  test/Index/complete-arrow-dot.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/CIndexCodeCompletion.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -171,6 +171,8 @@
 clang_getCompletionChunkCompletionString
 clang_getCompletionChunkKind
 clang_getCompletionChunkText
+clang_getCompletionNumFixIts
+clang_getCompletionFixIt
 clang_getCompletionNumAnnotations
 clang_getCompletionParent
 clang_getCompletionPriority
@@ -260,6 +262,7 @@
 clang_getSpellingLocation
 clang_getTUResourceUsageName
 clang_getTemplateCursorKind
+clang_getToken
 clang_getTokenExtent
 clang_getTokenKind
 clang_getTokenLocation
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -16,6 +16,7 @@
 #include "CIndexDiagnostic.h"
 #include "CLog.h"
 #include "CXCursor.h"
+#include "CXSourceLocation.h"
 #include "CXString.h"
 #include "CXTranslationUnit.h"
 #include "clang/AST/Decl.h"
@@ -302,10 +303,53 @@
   /// A string containing the Objective-C selector entered thus far for a
   /// message send.
   std::string Selector;
+
+  /// \brief Vector of fix-its for each completion result that *must* be applied
+  /// before that result for the corresponding completion item.
+  std::vector> FixItsVector;
 };
 
 } // end anonymous namespace
 
+unsigned clang_getCompletionNumFixIts(CXCodeCompleteResults *results,
+  unsigned completion_index) {
+  AllocatedCXCodeCompleteResults *allocated_results = (AllocatedCXCodeCompleteResults *)results;
+
+  if (!allocated_results || allocated_results->FixItsVector.size() <= completion_index)
+return 0;
+
+  return static_cast(allocated_results->FixItsVector[completion_index].size());
+}
+
+CXString clang_getCompletionFixIt(CXCodeCompleteResults *results,
+  unsigned completion_index,
+  unsigned fixit_index,
+  CXSourceRange *replacement_range) {
+  AllocatedCXCodeCompleteResults *allocated_results = (AllocatedCXCodeCompleteResults *)results;
+
+  if (!allocated_results || allocated_results->FixItsVector.size() <= completion_index) {
+if (replacement_range)
+  *replacement_range = clang_getNullRange();
+return cxstring::createNull();
+  }
+
+  ArrayRef FixIts = allocated_results->FixItsVector[completion_index];
+  if (FixIts.size() <= fixit_index) {
+if (replacement_range)
+  *replacement_range = clang_getNullRange();
+return cxstring::createNull();
+  }
+
+  const FixItHint &FixIt = FixIts[fixit_index];
+  if (replacement_range) {
+*replacement_range = cxloc::translateSourceRange(
+*allocated_results->SourceMgr, allocated_results->LangOpts,
+FixIt.RemoveRange);
+  }
+
+  return cxstring::createRef(FixIt.CodeToInsert.c_str());
+}
+
 /// Tracks the number of code-completion result objects that are 
 /// currently active.
 ///
@@ -531,18 +575,22 @@
 CodeCompletionResult *Results,
 unsigned NumResults) override {
   StoredResults.reserve(StoredResults.size() + NumResults);
+  if (includeFixIts())
+AllocatedResults.FixItsVector.reserve(NumResults);
   for (unsigned I = 0; I != NumResults; ++I) {
-CodeCompletionString *StoredCompletion
+CodeCompletionString *StoredCompletion
   = Results[I].CreateCodeCompletionString(S, Context, getAllocator(),
   getCodeCompletionTUInfo(),
   includeBriefComments());
 
 CXCompletionResult R;
 R.CursorKind = Results[I].CursorKind;
 R.CompletionString = StoredCompletion;
 StoredResults.push_back(R);
+if (includeFixIts())
+  AllocatedResults.FixItsVector.emplace_back(std::move(Results[I].FixIts));
   }
-  
+
   enum CodeCompletionContext::Kind contextKind = Context.getKind();
   
   AllocatedResults.ContextKind = contextKind;
@@ -644,13 +692,13 @@
   unsigned options) {
   bool IncludeBriefComments = options & CXCodeComplete_IncludeBriefComments;
   bool SkipPreamble = options & CXCodeComplete_SkipPreamble;
+  bool IncludeFixIts = options & CXCodeComplete_IncludeFixIts;
 
 #ifdef UDP_CODE_COMPLETION_LOGGER
 #ifdef UDP_CODE_COMPLETION_LOGGER_PORT
   const llvm::TimeR

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Sam McCall via Phabricator via cfe-commits]

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

n




Comment at: clangd/index/CanonicalIncludes.cpp:50
+  if (I == Headers.end())
+return Headers[0]; // Fallback to the defining header.
+  llvm::StringRef Header = *I;

nit: here and elsewhere, you probably want "declaring" rather than defining



Comment at: clangd/index/CanonicalIncludes.h:52
+  /// Returns the canonical include for symbol with \p QualifiedName. \p 
Headers
+  /// is a stack of #includes that starts from the last included header (i.e.
+  /// header that defines the symbol) to the header file that is directly

This might be clearer slightly more concretely:
```
\p Headers is the include stack: Headers.front() is the file declaring the 
symbol, and Headers.back() is the main file```
(or "included by the main file", but having it be main-file might be neater)



Comment at: clangd/index/CanonicalIncludes.h:54
+  /// header that defines the symbol) to the header file that is directly
+  /// included by the main file. When mapping headers, this skips files that 
are
+  /// not supposed to be #included directly e.g. .inc file.

Nit: "this" is a bit ambiguous here, the parameter or the algorithm?
Consider just dropping this new part of the comment, it's kind of an 
implementation detail (hiding behind the word "canonical")



Comment at: clangd/index/SymbolCollector.cpp:209
+  while (true) {
+if (!Loc.isValid() || SM.isInMainFile(Loc))
+  break;

(as above, maybe want to include the main file for simplicity/symmetry)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47187



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

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added a comment.

I'm not sure if we have tests for that, but I remember that we kept the 
enumerators in the outer scope so that completion could find them..
Am I right that this patch will change the behavior and we won't get 
enumerators in the following example:

  /// foo.h
  enum Foo {
A, B, C
  };
  
  /// foo.cpp
  #include "foo.h"
  
  int a = ^ // <-- A, B, C should be in completion list here.

It's one of those cases where code completion and workspace symbol search seem 
to want different results :-(
I suggest to add an extra string field for containing unscoped enum name, maybe 
into symbol details? And add a parameter to `Index::fuzzyFind` on whether we 
need to match enum scopes or not.
+@ioeric, +@sammccall,  WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47223



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

[PATCH] D46501: [OpenCL] Guard all half float usage based on cl_khr_fp16 extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D46501#1089777, @yaxunl wrote:

> Only halfn type requires cl_khr_fp16. These functions do not use halfn type, 
> therefore cl_khr_fp16 is not required.


I think the extension is for all half 
https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/cl_khr_fp16.html

  This extension adds support for half scalar and vector types as built-in 
types that can be used for arithmetic operations, conversions, etc. An 
application that wants to use half and halfn types will need to include the 
directive shown above.
  
   


Repository:
  rC Clang

https://reviews.llvm.org/D46501



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

[PATCH] D46501: [OpenCL] Guard all half float usage based on cl_khr_fp16 extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Could you upload the full diff please, otherwise it's not easy to see all the 
functions guarded by the macro.


Repository:
  rC Clang

https://reviews.llvm.org/D46501



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

[PATCH] D46667: [OpenCL, OpenMP] Fix crash when OpenMP used in OpenCL file

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!




Comment at: test/SemaOpenCL/with_openmp.cl:1
+// RUN: %clang_cc1 -verify -fopenmp -ast-dump -x cl %s 2>&1 | FileCheck %s
+// expected-no-diagnostics

mikerice wrote:
> ABataev wrote:
> > Still the same question, do we really want to support full OpenMP for 
> > OpenCL or only simd?
> We think it makes sense to have full support.  We are using OpenCL for 
> devices that can run full OpenMP.  Is there any known problem with allowing 
> full support?
Sounds good! What target do you use though? Would it help to have an IR test as 
well?


https://reviews.llvm.org/D46667



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

[PATCH] D46501: [OpenCL] Guard all half float usage based on cl_khr_fp16 extension

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: lib/Headers/opencl-c.h:12097
+#ifdef cl_khr_fp16
 float __ovld vload_half(size_t offset, const __constant half *p);
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0

These built-ins are part of the core specification, which doesn't allow using 
`half` data type directly, but user can declare a pointer to `half` data type 
and use built-ins like vload_half/vstore_half converting `half` data type to 
`float` data type.

cl_khr_fp16 enables regular uses of half data types as well as built-ins 
returning `half` data type instead of `float` - vload/vstore.


Repository:
  rC Clang

https://reviews.llvm.org/D46501



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

[PATCH] D47251: Add a lit reproducer for PR37091

[David Stenberg via Phabricator via cfe-commits]

dstenb created this revision.
Herald added a subscriber: cfe-commits.

This adds a lit reproducer that verifies that no temporary
assembly files are left behind when using clang-tidy with a
target that does not support the internal assembler.

The fix is in Tooling (https://reviews.llvm.org/D45686), but
as we need to verify that no files are left behind, it is
probably easier and better to create a lit reproducer for a
specific tool here instead of creating a unit test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47251

Files:
  test/clang-tidy/pr37091.cpp


Index: test/clang-tidy/pr37091.cpp
===
--- /dev/null
+++ test/clang-tidy/pr37091.cpp
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// This is a reproducer for PR37091.
+//
+// Verify that no temporary files are left behind by the clang-tidy invocation.
+
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64
+// RUN: rmdir %t


Index: test/clang-tidy/pr37091.cpp
===
--- /dev/null
+++ test/clang-tidy/pr37091.cpp
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// This is a reproducer for PR37091.
+//
+// Verify that no temporary files are left behind by the clang-tidy invocation.
+
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64
+// RUN: rmdir %t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

[David Stenberg via Phabricator via cfe-commits]

dstenb added a comment.

Ping.

We have added a lit reproducer for this now in clang-tools-extra: 
https://reviews.llvm.org/D47251.


Repository:
  rC Clang

https://reviews.llvm.org/D45686



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

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:2137
+  do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+  nextToken();

`tok::r_brace` could be skiped - see comment to line 2143.



Comment at: lib/Format/UnwrappedLineParser.cpp:2143
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+} else {

We have to add `return` after `addUnwrappedLine` as `parseBlock` does consume 
`tok::r_brace`. Without `return` we will consume tokens after `}`. This problem 
will rarely occur as most lines end with `tok::semi` or `tok::r_brace` and it 
will be terminated properly (however maybe not handled properly as we just skip 
every token in `else`) by `if` branch.

Test like:
```
@implementation Foo
- (foo)foo {
}
@end
@implementation Bar
- (bar)bar {
}
@end
```
will distinguish version with `return` from one without. Therefore, I think we 
should add it.


Repository:
  rC Clang

https://reviews.llvm.org/D47095



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

r333060 - [analyzer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[David Carlier via cfe-commits]

Author: devnexen
Date: Tue May 22 21:38:25 2018
New Revision: 333060

URL: http://llvm.org/viewvc/llvm-project?rev=333060&view=rev
Log:
[analyzer] CStringChecker fix for strlcpy when no bytes are copied to the dest 
buffer

Again, strlc* does not return a pointer so the zero size case doest not fit.

Reviewers: NoQ, george.karpenkov

Reviewed by: NoQ

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/bsd-string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=333060&r1=333059&r2=333060&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Tue May 22 
21:38:25 2018
@@ -1652,7 +1652,11 @@ void CStringChecker::evalStrcpyCommon(Ch
 
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
-  StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  if (returnPtr) {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  } else {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *lenValNL);
+  }
   C.addTransition(StateZeroSize);
   return;
 }

Modified: cfe/trunk/test/Analysis/bsd-string.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bsd-string.c?rev=333060&r1=333059&r2=333060&view=diff
==
--- cfe/trunk/test/Analysis/bsd-string.c (original)
+++ cfe/trunk/test/Analysis/bsd-string.c Tue May 22 21:38:25 2018
@@ -38,3 +38,8 @@ void f6() {
   size_t len = strlcat(buf, "defg", 4);
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
+
+int f7() {
+  char buf[8];
+  return strlcpy(buf, "1234567", 0); // no-crash
+}


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

r333059 - This is a test commit.

[David Carlier via cfe-commits]

Author: devnexen
Date: Tue May 22 21:27:39 2018
New Revision: 333059

URL: http://llvm.org/viewvc/llvm-project?rev=333059&view=rev
Log:
This is a test commit.

Modified:
cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt

Modified: cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt?rev=333059&r1=333058&r2=333059&view=diff
==
--- cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt (original)
+++ cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt Tue May 22 21:27:39 
2018
@@ -1,6 +1,6 @@
 # If we don't need RTTI or EH, there's no reason to export anything
 # from the plugin.
-if( NOT MSVC ) # MSVC mangles symbols differently, and
+if( NOT MSVC )  # MSVC mangles symbols differently, and
 # PrintFunctionNames.export contains C++ symbols.
   if( NOT LLVM_REQUIRES_RTTI )
 if( NOT LLVM_REQUIRES_EH )


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

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Eric Liu via Phabricator via cfe-commits]

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

- Include main file in the include stack when mapping headers.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47187

Files:
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -131,9 +131,9 @@
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11",   "-include",
- TestHeaderName, TestFileName};
+std::vector Args = {
+"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
+"-include", TestHeaderName,  TestFileName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 tooling::ToolInvocation Invocation(
 Args,
@@ -666,6 +666,54 @@
  IncludeHeader("\"the/good/header.h\"";
 }
 
+TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  Includes.addMapping(TestHeaderName, "");
+  CollectorOpts.Includes = &Includes;
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader("")),
+   AllOf(QName("Y"), DeclURI(TestHeaderURI),
+ IncludeHeader("";
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = &Includes;
+  TestFileName = testPath("main.h");
+  TestFileURI = URI::createFile(TestFileName).toString();
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = &Includes;
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(IncURI;
+}
+
 TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
   CollectorOpts.CollectIncludePath = true;
   Annotations Header(R"(
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -200,23 +200,31 @@
 /// Gets a canonical include (URI of the header or   or "header") for
 /// header of \p Loc.
 /// Returns None if fails to get include header for \p Loc.
-/// FIXME: we should handle .inc files whose symbols are expected be exported by
-/// their containing headers.
 llvm::Optional
 getIncludeHeader(llvm::StringRef QName, const SourceManager &SM,
  SourceLocation Loc, const SymbolCollector::Options &Opts) {
-  llvm::StringRef FilePath = SM.getFilename(Loc);
-  if (FilePath.empty())
+  std::vector Headers;
+  // Collect the #include stack.
+  while (true) {
+if (!Loc.isValid())
+  break;
+auto FilePath = SM.getFilename(Loc);
+if (FilePath.empty())
+  break;
+Headers.push_back(FilePath);
+if (SM.isInMainFile(Loc))
+  break;
+Loc = SM.getIncludeLoc(SM.getFileID(Loc));
+  }
+  if (Headers.empty())
 return llvm::None;
+  llvm::StringRef Header = Headers[0];
   if (Opts.Includes) {
-llvm::StringRef Mapped = Opts.Includes->mapHeader(FilePath,

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:209
+  while (true) {
+if (!Loc.isValid() || SM.isInMainFile(Loc))
+  break;

sammccall wrote:
> (as above, maybe want to include the main file for simplicity/symmetry)
Thanks! As you pointed out offline, we would need main file for correctness as 
well, as a main file could the exporting header.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47187



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

[PATCH] D44888: [RISCV] Add -mrelax/-mno-relax flags to enable/disable RISCV linker relaxation

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

This is looking good to me, just needs an update to address this 
 request for a test in 
riscv-features.c that demonstrates the default +relax/-relax setting.


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



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

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

[Ian Sunamura via Phabricator via cfe-commits]

kent08ian added a comment.

In https://reviews.llvm.org/D10833#970906, @milianw wrote:

> still looks good to me. can someone else please review and commit this?


Ping


Repository:
  rC Clang

https://reviews.llvm.org/D10833



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

r333080 - Test commit

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Wed May 23 05:48:55 2018
New Revision: 333080

URL: http://llvm.org/viewvc/llvm-project?rev=333080&view=rev
Log:
Test commit

Modified:
cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt

Modified: cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt?rev=333080&r1=333079&r2=333080&view=diff
==
--- cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt (original)
+++ cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt Wed May 23 05:48:55 
2018
@@ -1,7 +1,7 @@
 # If we don't need RTTI or EH, there's no reason to export anything
 # from the plugin.
-if( NOT MSVC )  # MSVC mangles symbols differently, and
-# PrintFunctionNames.export contains C++ symbols.
+if( NOT MSVC ) # MSVC mangles symbols differently, and
+   # PrintFunctionNames.export contains C++ symbols.
   if( NOT LLVM_REQUIRES_RTTI )
 if( NOT LLVM_REQUIRES_EH )
   set(LLVM_EXPORTED_SYMBOL_FILE 
${CMAKE_CURRENT_SOURCE_DIR}/PrintFunctionNames.exports)


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

[PATCH] D47155: [analyzer] Reduce simplifySVal complexity threshold further.

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

baloghadamsoftware added a comment.

Hello! Thank you for addressing this problem. Are these kinds of symbols common 
in real code? For me it seems very artificial. However, I agree with George, it 
would be better to have this value as an analyzer option with a default value 
(of 20).


Repository:
  rC Clang

https://reviews.llvm.org/D47155



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

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

baloghadamsoftware added a comment.

Can we continue the discussion here, please? We should involve Devin and/or 
George as well if we cannot agree ourselves.


https://reviews.llvm.org/D35110



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

[PATCH] D47161: update

[Anastasis via Phabricator via cfe-commits]

gramanas added a comment.

It was a mistake on my part with the arcanist tool. The documentation is fine, 
the only problems I encountered were due to my ignorance of how these systems 
work.


Repository:
  rC Clang

https://reviews.llvm.org/D47161



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

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46602



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Anastasis via Phabricator via cfe-commits]

gramanas updated this revision to Diff 148198.
gramanas added a comment.

Set debug location to the entry block alloca


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+
+// CHECK: [[B:%.*]] = alloca i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -68,6 +68,8 @@
   bool CastToDefaultAddrSpace) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getQuantity());
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(this->Builder.getCurrentDebugLocation());
   if (AllocaAddr)
 *AllocaAddr = Address(Alloca, Align);
   llvm::Value *V = Alloca;
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
@@ -2071,7 +2074,7 @@
 
   // Store the initial value into the alloca.
   if (DoStore)
-EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+   EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
 
   setAddrOfLocalVar(&D, DeclPtr);
 


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+
+// CHECK: [[B:%.*]] = alloca i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -68,6 +68,8 @@
   bool CastToDefaultAddrSpace) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getQuantity());
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(this->Builder.getCurrentDebugLocation());
   if (AllocaAddr)
 *AllocaAddr = Address(Alloca, Align);
   llvm::Value *V = Alloca;
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
@@ -2071,7 +2074,7 @@
 
   // Store the initial value into the alloca.
   if (DoStore)
-EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+   EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
 
   setAddrOfLocalVar(&D, DeclPtr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47256: [clangd] Fix code completion in MACROs with stringification.

[Eric Liu via Phabricator via cfe-commits]

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

Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.

For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".

  void f(const char*, int);
  #define F(x) f(#x, x)
  int x;
  void main() {
F(::^);
  }

To fix this, we only process a sema callback when it gives completion results or
the context supports index-based completion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47256

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -693,6 +693,44 @@
 )cpp");
 }
 
+TEST(CompletionTest, CompleteInMacroWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+}  // namespace ns
+
+int f(int input_num) {
+  F(ns::^)
+}
+)cpp");
+
+  EXPECT_THAT(Results.items,
+  UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
+TEST(CompletionTest, CompleteInMacroAndNamespaceWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+
+int f(int input_num) {
+  F(^)
+}
+}  // namespace ns
+)cpp");
+
+  EXPECT_THAT(Results.items,
+  UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
 TEST(CompletionTest, CompleteInExcludedPPBranch) {
   auto Results = completions(R"cpp(
 int bar(int param_in_bar) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -406,6 +406,50 @@
   return Info.scopesForIndexQuery();
 }
 
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+  switch (K) {
+  case CodeCompletionContext::CCC_TopLevel:
+  case CodeCompletionContext::CCC_ObjCInterface:
+  case CodeCompletionContext::CCC_ObjCImplementation:
+  case CodeCompletionContext::CCC_ObjCIvarList:
+  case CodeCompletionContext::CCC_ClassStructUnion:
+  case CodeCompletionContext::CCC_Statement:
+  case CodeCompletionContext::CCC_Expression:
+  case CodeCompletionContext::CCC_ObjCMessageReceiver:
+  case CodeCompletionContext::CCC_EnumTag:
+  case CodeCompletionContext::CCC_UnionTag:
+  case CodeCompletionContext::CCC_ClassOrStructTag:
+  case CodeCompletionContext::CCC_ObjCProtocolName:
+  case CodeCompletionContext::CCC_Namespace:
+  case CodeCompletionContext::CCC_Type:
+  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+  case CodeCompletionContext::CCC_ParenthesizedExpression:
+  case CodeCompletionContext::CCC_ObjCInterfaceName:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
+return true;
+  case CodeCompletionContext::CCC_Other: // Be conservative.
+  case CodeCompletionContext::CCC_OtherWithMacros:
+  case CodeCompletionContext::CCC_DotMemberAccess:
+  case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCPropertyAccess:
+  case CodeCompletionContext::CCC_MacroName:
+  case CodeCompletionContext::CCC_MacroNameUse:
+  case CodeCompletionContext::CCC_PreprocessorExpression:
+  case CodeCompletionContext::CCC_PreprocessorDirective:
+  case CodeCompletionContext::CCC_NaturalLanguage:
+  case CodeCompletionContext::CCC_SelectorName:
+  case CodeCompletionContext::CCC_TypeQualifiers:
+  case CodeCompletionContext::CCC_ObjCInstanceMessage:
+  case CodeCompletionContext::CCC_ObjCClassMessage:
+  case CodeCompletionContext::CCC_Recovery:
+return false;
+  }
+  llvm_unreachable("unknown code completion context");
+}
+
 // The CompletionRecorder captures Sema code-complete output, including context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -431,12 +475,17 @@
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *InResults,
   unsigned NumResults) override final {
+// If a callback is called without any sema result and the context does not
+// support index-based completion, we simply 

[PATCH] D47183: [clangd] Support multiple sema code completion callbacks.

[Eric Liu via Phabricator via cfe-commits]

ioeric abandoned this revision.
ioeric added a comment.

Dropping this in favor of https://reviews.llvm.org/D47256


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47183



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Anastasis via Phabricator via cfe-commits]

gramanas updated this revision to Diff 148200.
gramanas marked an inline comment as done.
gramanas added a comment.

Remove redundant `this`


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+
+// CHECK: [[B:%.*]] = alloca i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -68,6 +68,8 @@
   bool CastToDefaultAddrSpace) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getQuantity());
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)
 *AllocaAddr = Address(Alloca, Align);
   llvm::Value *V = Alloca;
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
@@ -2071,7 +2074,7 @@
 
   // Store the initial value into the alloca.
   if (DoStore)
-EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+   EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
 
   setAddrOfLocalVar(&D, DeclPtr);
 


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+
+// CHECK: [[B:%.*]] = alloca i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -68,6 +68,8 @@
   bool CastToDefaultAddrSpace) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getQuantity());
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)
 *AllocaAddr = Address(Alloca, Align);
   llvm::Value *V = Alloca;
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
@@ -2071,7 +2074,7 @@
 
   // Store the initial value into the alloca.
   if (DoStore)
-EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+   EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
 
   setAddrOfLocalVar(&D, DeclPtr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added reviewers: aaron.ballman, john.brawn, mehdi_amini, sammccall, 
zturner, bkramer.
lebedev.ri added a comment.

The testcase sounds reasonable. Can't comment on the actual fix.
Adding some more reviewers that potentially worked on that source file.


Repository:
  rC Clang

https://reviews.llvm.org/D45686



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

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/Tooling/CompilationDatabase.cpp:303
+  // Remove temp files.
+  Compilation->CleanupFileList(Compilation->getTempFiles());
+

It seems to me that this would be best called from the destructor of the 
Compilation object.  I realize nothing returns before this, but IMO this seems 
universal enough that if it doesn't break anything, putting this in the 
destructor would be a proper place for cleanup.


Repository:
  rC Clang

https://reviews.llvm.org/D45686



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

[PATCH] D47257: [clang-format] Break template declarations followed by comments

[Krasimir Georgiev via Phabricator via cfe-commits]

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

This patch fixes two bugs in clang-format where the template wrapper doesn't 
skip over
comments causing a long template declaration to not be split into multiple 
lines.
These were latent and exposed by r332436.


Repository:
  rC Clang

https://reviews.llvm.org/D47257

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5521,6 +5521,58 @@
NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test",
+format(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value> /* line */
+void f() {})test",
+format(R"test(
+template ::value>  /* line */
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+   // multiline
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+  // multiline
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template ::value>  // trailing lng
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing lng
+void f() {})test", Style));
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -405,7 +405,7 @@
   // If the template declaration spans multiple lines, force wrap before the
   // function/class declaration
   if (Previous.ClosesTemplateDeclaration &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
   if (State.Column <= NewLineColumn)
@@ -804,7 +804,8 @@
!State.Stack.back().AvoidBinPacking) ||
   Previous.is(TT_BinaryOperator))
 State.Stack.back().BreakBeforeParameter = false;
-  if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
+  if (PreviousNonComment &&
+  PreviousNonComment->isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
   Current.NestingLevel == 0)
 State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5521,6 +5521,58 @@
NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test",
+format(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value> /* line */
+void f() {})test",
+format(R"test(
+template ::value>  /* line */
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+   // multiline
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+  // multiline
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template ::value>  // trailing lng
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing lng
+void f() {})test", Style));
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
Index: lib/Format/ContinuationIndenter.cpp
=

[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333082: Fix duplicate class template definitions problem 
(authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46950?vs=148013&id=148204#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46950

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

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -4070,6 +4070,17 @@
   TemplateParams);
 }
 
+// Returns the definition for a (forward) declaration of a ClassTemplateDecl, if
+// it has any definition in the redecl chain.
+static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) {
+  CXXRecordDecl *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
+  if (!ToTemplatedDef)
+return nullptr;
+  ClassTemplateDecl *TemplateWithDef =
+  ToTemplatedDef->getDescribedClassTemplate();
+  return TemplateWithDef;
+}
+
 Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   // If this record has a definition in the translation unit we're coming from,
   // but this particular declaration is not that definition, import the
@@ -4084,7 +4095,7 @@
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -4103,34 +4114,39 @@
 for (auto *FoundDecl : FoundDecls) {
   if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary))
 continue;
-  
+
   Decl *Found = FoundDecl;
   if (auto *FoundTemplate = dyn_cast(Found)) {
-if (IsStructuralMatch(D, FoundTemplate)) {
-  // The class templates structurally match; call it the same template.
 
-  // We found a forward declaration but the class to be imported has a
-  // definition.
-  // FIXME Add this forward declaration to the redeclaration chain.
-  if (D->isThisDeclarationADefinition() &&
-  !FoundTemplate->isThisDeclarationADefinition())
+// The class to be imported is a definition.
+if (D->isThisDeclarationADefinition()) {
+  // Lookup will find the fwd decl only if that is more recent than the
+  // definition. So, try to get the definition if that is available in
+  // the redecl chain.
+  ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate);
+  if (!TemplateWithDef)
 continue;
+  FoundTemplate = TemplateWithDef; // Continue with the definition.
+}
 
-  Importer.Imported(D->getTemplatedDecl(), 
+if (IsStructuralMatch(D, FoundTemplate)) {
+  // The class templates structurally match; call it the same template.
+
+  Importer.Imported(D->getTemplatedDecl(),
 FoundTemplate->getTemplatedDecl());
   return Importer.Imported(D, FoundTemplate);
-} 
+}
   }
-  
+
   ConflictingDecls.push_back(FoundDecl);
 }
-
+
 if (!ConflictingDecls.empty()) {
   Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary,
- ConflictingDecls.data(), 
+ ConflictingDecls.data(),
  ConflictingDecls.size());
 }
-
+
 if (!Name)
   return nullptr;
   }
Index: unittests/AST/DeclMatcher.h
===
--- unittests/AST/DeclMatcher.h
+++ unittests/AST/DeclMatcher.h
@@ -44,15 +44,23 @@
 using FirstDeclMatcher = DeclMatcher;
 
 template 
-class DeclCounter : public MatchFinder::MatchCallback {
+class DeclCounterWithPredicate : public MatchFinder::MatchCallback {
+  using UnaryPredicate = std::function;
+  UnaryPredicate Predicate;
   unsigned Count = 0;
   void run(const MatchFinder::MatchResult &Result) override {
-  if(Result.Nodes.getNodeAs("")) {
+if (auto N = Result.Nodes.getNodeAs("")) {
+  if (Predicate(N))
 ++Count;
-  }
+}
   }
+
 public:
-  // Returns the number of matched nodes under the tree rooted in `D`.
+  DeclCounterWithPredicate()
+  : Predicate([](const NodeType *) { return true; }) {}
+  DeclCounterWithPredicate(UnaryPredicate P) : Predicate(P) {}
+  // Returns the number of matched nodes which satisfy the predicate under the
+  // tree rooted in `D`.
   template 
   unsigned match(const Decl *D, const MatcherType &AMatcher) {
 MatchFinder Finder;
@@ -62,6 +70,9 @@
   }
 };
 
+template 
+using DeclCounter = DeclCounterWithPredicate;
+
 } // end namespace ast_matchers
 } // end namespace clang
 
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/AS

[clang-tools-extra] r333083 - Replace last DEBUG occurrence with LLVM_DEBUG.

[Nicola Zaghen via cfe-commits]

Author: nzaghen
Date: Wed May 23 06:57:48 2018
New Revision: 333083

URL: http://llvm.org/viewvc/llvm-project?rev=333083&view=rev
Log:
Replace last DEBUG occurrence with LLVM_DEBUG.

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333083&r1=333082&r2=333083&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed May 23 06:57:48 2018
@@ -1004,11 +1004,11 @@ private:
 Scores.symbolScore =
 Scores.filterScore ? Scores.finalScore / Scores.filterScore : 
QualScore;
 
-DEBUG(llvm::dbgs() << "CodeComplete: " << C.Name
-   << (IndexResult ? " (index)" : "")
-   << (SemaResult ? " (sema)" : "") << " = "
-   << Scores.finalScore << "\n"
-   << Quality << Relevance << "\n");
+LLVM_DEBUG(llvm::dbgs()
+   << "CodeComplete: " << C.Name << (IndexResult ? " (index)" : "")
+   << (SemaResult ? " (sema)" : "") << " = " << Scores.finalScore
+   << "\n" 
+   << Quality << Relevance << "\n");
 
 NSema += bool(SemaResult);
 NIndex += bool(IndexResult);


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

[PATCH] D47257: [clang-format] Break template declarations followed by comments

[Sam McCall via Phabricator via cfe-commits]

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

Thanks for chasing this, good detective work!
Changes look plausible and tests are nice, so LG assuming you know what you're 
doing. If you're unsure, Manuel will give you better advice than me :)


Repository:
  rC Clang

https://reviews.llvm.org/D47257



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

[PATCH] D47256: [clangd] Fix code completion in MACROs with stringification.

[Eric Liu via Phabricator via cfe-commits]

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

- Fix broken test


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47256

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -693,6 +693,42 @@
 )cpp");
 }
 
+TEST(CompletionTest, CompleteInMacroWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+}  // namespace ns
+
+int f(int input_num) {
+  F(ns::^)
+}
+)cpp");
+
+  EXPECT_THAT(Results.items,
+  UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
+TEST(CompletionTest, CompleteInMacroAndNamespaceWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+
+int f(int input_num) {
+  F(^)
+}
+}  // namespace ns
+)cpp");
+
+  EXPECT_THAT(Results.items, Contains(Named("X")));
+}
+
 TEST(CompletionTest, CompleteInExcludedPPBranch) {
   auto Results = completions(R"cpp(
 int bar(int param_in_bar) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -406,6 +406,50 @@
   return Info.scopesForIndexQuery();
 }
 
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+  switch (K) {
+  case CodeCompletionContext::CCC_TopLevel:
+  case CodeCompletionContext::CCC_ObjCInterface:
+  case CodeCompletionContext::CCC_ObjCImplementation:
+  case CodeCompletionContext::CCC_ObjCIvarList:
+  case CodeCompletionContext::CCC_ClassStructUnion:
+  case CodeCompletionContext::CCC_Statement:
+  case CodeCompletionContext::CCC_Expression:
+  case CodeCompletionContext::CCC_ObjCMessageReceiver:
+  case CodeCompletionContext::CCC_EnumTag:
+  case CodeCompletionContext::CCC_UnionTag:
+  case CodeCompletionContext::CCC_ClassOrStructTag:
+  case CodeCompletionContext::CCC_ObjCProtocolName:
+  case CodeCompletionContext::CCC_Namespace:
+  case CodeCompletionContext::CCC_Type:
+  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+  case CodeCompletionContext::CCC_ParenthesizedExpression:
+  case CodeCompletionContext::CCC_ObjCInterfaceName:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
+return true;
+  case CodeCompletionContext::CCC_Other: // Be conservative.
+  case CodeCompletionContext::CCC_OtherWithMacros:
+  case CodeCompletionContext::CCC_DotMemberAccess:
+  case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCPropertyAccess:
+  case CodeCompletionContext::CCC_MacroName:
+  case CodeCompletionContext::CCC_MacroNameUse:
+  case CodeCompletionContext::CCC_PreprocessorExpression:
+  case CodeCompletionContext::CCC_PreprocessorDirective:
+  case CodeCompletionContext::CCC_NaturalLanguage:
+  case CodeCompletionContext::CCC_SelectorName:
+  case CodeCompletionContext::CCC_TypeQualifiers:
+  case CodeCompletionContext::CCC_ObjCInstanceMessage:
+  case CodeCompletionContext::CCC_ObjCClassMessage:
+  case CodeCompletionContext::CCC_Recovery:
+return false;
+  }
+  llvm_unreachable("unknown code completion context");
+}
+
 // The CompletionRecorder captures Sema code-complete output, including context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -431,12 +475,17 @@
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *InResults,
   unsigned NumResults) override final {
+// If a callback is called without any sema result and the context does not
+// support index-based completion, we simply skip it to give way to
+// potential future callbacks with results.
+if (NumResults == 0 && !contextAllowsIndex(Context.getKind()))
+  return;
 if (CCSema) {
   log(llvm::formatv(
   "Multiple code complete callbacks (parser backtracked?). "
   "Dropping results from context {0}, keeping results from {1}.",
-  getCompletionKindString(this->CCContext.getKind()),
-  getCompletionKindString(Context.getKind(;
+  getCompletionKindString(Context.getKind()),
+  getCompletionKindString(this->CCContext.getKind(;
   return;
 }
 // Record the completion context.
@@ -719,50 +768,6 @@
   return true;
 }
 
-// Should we perform index-based completion in a context of the specified kind?
-// FIXME: consider al

[PATCH] D47137: [Sparc] Add floating-point register names

[Daniel Cederman via Phabricator via cfe-commits]

dcederman updated this revision to Diff 148205.
dcederman added a comment.
Herald added a subscriber: eraman.

Added test cases, f32-f63,  d0-d31, and q0-q15.


https://reviews.llvm.org/D47137

Files:
  lib/Basic/Targets/Sparc.cpp
  test/CodeGen/sparcv8-inline-asm.c
  test/CodeGen/sparcv9-inline-asm.c

Index: test/CodeGen/sparcv9-inline-asm.c
===
--- /dev/null
+++ test/CodeGen/sparcv9-inline-asm.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple sparcv9-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+void test_gcc_registers(void) {
+register unsigned int regO6 asm("o6") = 0;
+register unsigned int regSP asm("sp") = 1;
+register unsigned int reg14 asm("r14") = 2;
+register unsigned int regI6 asm("i6") = 3;
+register unsigned int regFP asm("fp") = 4;
+register unsigned int reg30 asm("r30") = 5;
+
+register float fQ5 asm("q5") = 6.0;
+register float fD10 asm("d10") = 7.0;
+register float fF20 asm("f20") = 8.0;
+
+register double dQ10 asm("q10") = 9.0;
+register double dD20 asm("d20") = 10.0;
+register double dF40 asm("f40") = 11.0;
+
+register long double qQ10 asm("q10") = 12.0;
+register long double qD20 asm("d20") = 13.0;
+register long double qF40 asm("f40") = 14.0;
+
+// Test remapping register names in register ... asm("rN") statments.
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r14},{r14},{r14}"
+asm volatile("add %0,%1,%2" : : "r" (regO6), "r" (regSP), "r" (reg14));
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r30},{r30},{r30}"
+asm volatile("add %0,%1,%2" : : "r" (regI6), "r" (regFP), "r" (reg30));
+
+// CHECK: call void asm sideeffect "fadds $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("fadds %0,%1,%2" : : "f" (fQ5), "f" (fD10), "f"(fF20));
+
+// CHECK: call void asm sideeffect "faddd $0,$1,$2", "{f40},{f40},{f40}"
+asm volatile("faddd %0,%1,%2" : : "f" (dQ10), "f" (dD20), "f"(dF40));
+
+// CHECK: call void asm sideeffect "faddq $0,$1,$2", "{f40},{f40},{f40}"
+asm volatile("faddq %0,%1,%2" : : "f" (qQ10), "f" (qD20), "f"(qF40));
+
+}
Index: test/CodeGen/sparcv8-inline-asm.c
===
--- test/CodeGen/sparcv8-inline-asm.c
+++ test/CodeGen/sparcv8-inline-asm.c
@@ -1,11 +1,50 @@
 // RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 
 // CHECK: define float @fabsf(float %a)
-// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float %{{.*}}) #1
+// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float %{{.*}})
 float fabsf(float a) {
   float res;
   __asm __volatile__("fabss  %1, %0;"
  : /* reg out*/ "=e"(res)
  : /* reg in */ "f"(a));
   return res;
 }
+
+void test_gcc_registers(void) {
+register unsigned int regO6 asm("o6") = 0;
+register unsigned int regSP asm("sp") = 1;
+register unsigned int reg14 asm("r14") = 2;
+register unsigned int regI6 asm("i6") = 3;
+register unsigned int regFP asm("fp") = 4;
+register unsigned int reg30 asm("r30") = 5;
+
+register float fQ5 asm("q5") = 6.0;
+register float fD10 asm("d10") = 7.0;
+register float fF20 asm("f20") = 8.0;
+
+register double dQ5 asm("q5") = 9.0;
+register double dD10 asm("d10") = 10.0;
+register double dF20 asm("f20") = 11.0;
+
+register long double qQ5 asm("q5") = 12.0;
+register long double qD10 asm("d10") = 13.0;
+register long double qF20 asm("f20") = 14.0;
+
+// Test remapping register names in register ... asm("rN") statments.
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r14},{r14},{r14}"
+asm volatile("add %0,%1,%2" : : "r" (regO6), "r" (regSP), "r" (reg14));
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r30},{r30},{r30}"
+asm volatile("add %0,%1,%2" : : "r" (regI6), "r" (regFP), "r" (reg30));
+
+ // CHECK: call void asm sideeffect "fadds $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("fadds %0,%1,%2" : : "f" (fQ5), "f" (fD10), "f"(fF20));
+
+// CHECK: call void asm sideeffect "faddd $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("faddd %0,%1,%2" : : "f" (dQ5), "f" (dD10), "f"(dF20));
+
+// CHECK: call void asm sideeffect "faddq $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("faddq %0,%1,%2" : : "f" (qQ5), "f" (qD10), "f"(qF20));
+
+}
Index: lib/Basic/Targets/Sparc.cpp
===
--- lib/Basic/Targets/Sparc.cpp
+++ lib/Basic/Targets/Sparc.cpp
@@ -20,24 +20,50 @@
 using namespace clang::targets;
 
 const char *const SparcTargetInfo::GCCRegNames[] = {
+// Integer registers
 "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",  "r8",  "r9",  "r10",
 "r11", "r12", "r13", "r14", "r15", "r16", "r17", "r18", "r19", "r20", "r21",
-"r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"
+"r

r333085 - [clang-format] Break template declarations followed by comments

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed May 23 07:18:19 2018
New Revision: 333085

URL: http://llvm.org/viewvc/llvm-project?rev=333085&view=rev
Log:
[clang-format] Break template declarations followed by comments

Summary:
This patch fixes two bugs in clang-format where the template wrapper doesn't 
skip over
comments causing a long template declaration to not be split into multiple 
lines.
These were latent and exposed by r332436.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

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

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

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=333085&r1=333084&r2=333085&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed May 23 07:18:19 2018
@@ -405,7 +405,7 @@ bool ContinuationIndenter::mustBreak(con
   // If the template declaration spans multiple lines, force wrap before the
   // function/class declaration
   if (Previous.ClosesTemplateDeclaration &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
   if (State.Column <= NewLineColumn)
@@ -804,7 +804,8 @@ unsigned ContinuationIndenter::addTokenO
!State.Stack.back().AvoidBinPacking) ||
   Previous.is(TT_BinaryOperator))
 State.Stack.back().BreakBeforeParameter = false;
-  if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
+  if (PreviousNonComment &&
+  PreviousNonComment->isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
   Current.NestingLevel == 0)
 State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=333085&r1=333084&r2=333085&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 23 07:18:19 2018
@@ -5521,6 +5521,58 @@ TEST_F(FormatTest, WrapsTemplateDeclarat
NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test",
+format(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value> /* line */
+void f() {})test",
+format(R"test(
+template ::value>  /* line */
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+   // multiline
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+  // multiline
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template ::value>  // trailing lng
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing lng
+void f() {})test", Style));
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;


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

[PATCH] D47257: [clang-format] Break template declarations followed by comments

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333085: [clang-format] Break template declarations followed 
by comments (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47257

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


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -405,7 +405,7 @@
   // If the template declaration spans multiple lines, force wrap before the
   // function/class declaration
   if (Previous.ClosesTemplateDeclaration &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
   if (State.Column <= NewLineColumn)
@@ -804,7 +804,8 @@
!State.Stack.back().AvoidBinPacking) ||
   Previous.is(TT_BinaryOperator))
 State.Stack.back().BreakBeforeParameter = false;
-  if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
+  if (PreviousNonComment &&
+  PreviousNonComment->isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
   Current.NestingLevel == 0)
 State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -5521,6 +5521,58 @@
NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test",
+format(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value> /* line */
+void f() {})test",
+format(R"test(
+template ::value>  /* line */
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename aa::value>  // trailing
+   // multiline
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing
+  // multiline
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template ::value>  // trailing lng
+void f() {})test",
+format(R"test(
+template <
+typename aa::value> // trailing lng
+void f() {})test", Style));
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -405,7 +405,7 @@
   // If the template declaration spans multiple lines, force wrap before the
   // function/class declaration
   if (Previous.ClosesTemplateDeclaration &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
   if (State.Column <= NewLineColumn)
@@ -804,7 +804,8 @@
!State.Stack.back().AvoidBinPacking) ||
   Previous.is(TT_BinaryOperator))
 State.Stack.back().BreakBeforeParameter = false;
-  if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
+  if (PreviousNonComment &&
+  PreviousNonComment->isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
   Current.NestingLevel == 0)
 State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -5521,6 +5521,58 @@
NeverBreak);
 }
 
+TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 60;
+  EXPECT_EQ(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test",
+format(R"test(
+// Baseline - no comments.
+template <
+typename aa::value>
+void f() {})test", Style));
+
+  EXPECT_EQ(R"test(
+template <
+typename a

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

[Gabor Marton via Phabricator via cfe-commits]

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

Remove multiline comment and reorder parts of the condition


Repository:
  rC Clang

https://reviews.llvm.org/D47057

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-
DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333086: [ASTImporter] Fix missing implict CXXRecordDecl in… 
(authored by martong, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47057

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


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-
DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333086: [ASTImporter] Fix missing implict CXXRecordDecl in… 
(authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47057?vs=148209&id=148210#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47057

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-
DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1363,6 +1363,22 @@
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@
 
 TEST_P(
 ASTImporterTestBase,
-DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1959,14 +1959,20 @@
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

[David Stenberg via Phabricator via cfe-commits]

dstenb added inline comments.



Comment at: lib/Tooling/CompilationDatabase.cpp:303
+  // Remove temp files.
+  Compilation->CleanupFileList(Compilation->getTempFiles());
+

erichkeane wrote:
> It seems to me that this would be best called from the destructor of the 
> Compilation object.  I realize nothing returns before this, but IMO this 
> seems universal enough that if it doesn't break anything, putting this in the 
> destructor would be a proper place for cleanup.
That sounds cleaner (if it's viable).

I considered that initially, but I was not sure that there are no cases where 
the temporary files outlive a Compilation object, and the "/// Temporary files 
which should be removed on exit." comment for the TempFiles field did not 
really make me more confident, so I went the conservative route instead.

I'll do that change and run some tests to see where that leads.



Repository:
  rC Clang

https://reviews.llvm.org/D45686



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

[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/Tooling/CompilationDatabase.cpp:303
+  // Remove temp files.
+  Compilation->CleanupFileList(Compilation->getTempFiles());
+

dstenb wrote:
> erichkeane wrote:
> > It seems to me that this would be best called from the destructor of the 
> > Compilation object.  I realize nothing returns before this, but IMO this 
> > seems universal enough that if it doesn't break anything, putting this in 
> > the destructor would be a proper place for cleanup.
> That sounds cleaner (if it's viable).
> 
> I considered that initially, but I was not sure that there are no cases where 
> the temporary files outlive a Compilation object, and the "/// Temporary 
> files which should be removed on exit." comment for the TempFiles field did 
> not really make me more confident, so I went the conservative route instead.
> 
> I'll do that change and run some tests to see where that leads.
> 
It just looks off-place for the function named `stripPositionalArgs()`


Repository:
  rC Clang

https://reviews.llvm.org/D45686



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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Haven't reviewed the code yet, just design stuff)
I'm tempted to push back on the idea that we need to store any - "if it's fast 
enough for code complete, it's fast enough for GTD". But that's probably 
oversimplifying - we don't force reuse of a stable preamble for GTD I think. So 
we probably do want some cache.

In https://reviews.llvm.org/D47063#1104495, @malaperle wrote:

> In https://reviews.llvm.org/D47063#1104417, @ilya-biryukov wrote:
>
> > Maybe we can even store enough information to not need the AST for 
> > navigation at all and build it only for features like refactorings.
> >  @sammccall, let me know what are your thoughts on all of this.
>
>
> That's what I was thinking. I think I also had similar discussion with 
> @arphaman. I think storing a limited number of ASTs is a good interim 
> solution.


Agree - it's a good idea but we shouldn't block on it. A general-purpose xrefs 
index may solve this problem (and others) but requires a bunch of design. A 
narrower structure risks building a bunch of complexity for one feature we 
can't reuse for the next.

> Another alternative that I've considered was evicting the ASTs from memory 
> after a certain period of time, e.g. after 30 seconds of inactivity for some 
> file.

We discussed this a bit more offline. A time-based limit reduces idle RAM 
usage, and a weight limit (or just count) reduces peak RAM.
Relatively speaking, idle seems to be more important to our hosted/multiplexed 
use cases, and peak is more important when running on a developer machine.
Weight is probably easier to tune. Time is easier to implement as the TUs are 
independent.

So this gives us some options (assuming we want some caching, but not 
unlimited):

- Evict if old - this helps hosted a lot, and is simple to implement.
- Evict if full (this patch) - this helps standalone, and is more complex.
- Evict if full AND old - this can be tuned nicely for hosted and standalone. 
Most complex, but only a little more than the previous option.
- Evict if full OR old - this puts a hard limit on resource usage and controls 
idle, but doesn't seem useful for hosted as it can't drive idle to zero.

I think the main design decision is whether the cache logic requires TUs to 
interact (vs simple time eviction). If we pay that complexity cost we get a lot 
of flexibility to tweak the cache. It's the hosted stuff that's driving this 
(for Google) right now, but maybe that's just because we're not really 
measuring impact on workstations yet :)
So I think I like this policy as a starting point, but we should plan to bolt 
on time-based-expiry in the near future.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: probinson.
Herald added subscribers: cfe-commits, JDevlieghere, aprantl.

Test is a reduction from running csmith with -gdwarf-5
The checksums are required for all 'files'.
The clang source manager does not provide them for expansions.


Repository:
  rC Clang

https://reviews.llvm.org/D47260

Files:
  test/CodeGen/2018-05-20-ExpansionChecksum.c


Index: test/CodeGen/2018-05-20-ExpansionChecksum.c
===
--- /dev/null
+++ test/CodeGen/2018-05-20-ExpansionChecksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif


Index: test/CodeGen/2018-05-20-ExpansionChecksum.c
===
--- /dev/null
+++ test/CodeGen/2018-05-20-ExpansionChecksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/index/CanonicalIncludes.cpp:54
+  StringRef Header = *I;
+  if (!isLiteralInclude(Header)) {
+// If Header is not expected be included (e.g. .cc file), we fall back to

headers are paths, not , right?



Comment at: clangd/index/CanonicalIncludes.cpp:55
+  if (!isLiteralInclude(Header)) {
+// If Header is not expected be included (e.g. .cc file), we fall back to
+// the declaring header.

This logic could be merged with the .inc logic, right?
"Find the first header such that the extension is not '.inc', and isn't a 
recognized non-header file"



Comment at: clangd/index/CanonicalIncludes.cpp:60
+if (!Ext.empty() && !driver::types::onlyPrecompileType(
+driver::types::lookupTypeForExtension(Ext)))
+  return Headers[0];

lookupTypeForExtension can return TY_INVALID, which you have to check for 
(onlyPrecompileType will assert).

I think we should be conservative and accept TY_INVALID as possible headers - 
our goal here really is to exclude "obvious" non-headers like cpp.

(in this case you can probably drop the explicit empty check)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47187



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: rnk.
probinson added a subscriber: rnk.
probinson added a comment.

+ @rnk who did the initial checksum stuff for CodeView.

I am unclear how the notion of checksums should interact with preprocessed 
files.

Nit: while many tests have dates in the filenames, we no longer use that 
convention.


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D45212: [HIP] Let CUDA toolchain support HIP language mode and amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 19 inline comments as done.
yaxunl added a comment.

In https://reviews.llvm.org/D45212#1105177, @tra wrote:

> Hi,
>
> Sorry about the long silence. I'm back to continue the reviews. I'll handle 
> what I can today and will continue with the rest on Tuesday.
>
> It looks like patch description needs to be updated:
>
> > Use clang-offload-bindler to create binary for device ISA.
>
> I don't see anything related to offload-bundler in this patch any more.


You are right. Using clang-offload-bundler to create binary for device ISA has 
been moved to another patch. Will update the description of this patch.

In https://reviews.llvm.org/D45212#1105282, @tra wrote:

> One more thing -- it would be really good to add some tests to make sure your 
> commands are constructed the way you want.


will do




Comment at: include/clang/Driver/Options.td:582
 def fno_cuda_rdc : Flag<["-"], "fno-cuda-rdc">;
+def hip_device_lib_path_EQ : Joined<["--"], "hip-device-lib-path=">, 
Group,
+  HelpText<"HIP device library path">;

tra wrote:
> I'm not sure about `i_Group`? This will cause this option to be passed to all 
> preprocessor jobs. It will also be passed to host and device side 
> compilations, while you probably only want/need it on device side only.
will change to Link_Group



Comment at: lib/Driver/ToolChains/Cuda.cpp:323
+C.getDriver().Diag(diag::err_drv_no_such_file) << BCName;
+  CmdArgs.push_back(Args.MakeArgString(FullName));
+  return FoundLibDevice;

tra wrote:
> FullName is already result of Args.MakeArgString. You only need to do it once.
will fix



Comment at: lib/Driver/ToolChains/Cuda.cpp:329
+// object file. It calls llvm-link, opt, llc, then lld steps.
+void AMDGCN::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+  const InputInfo &Output,

tra wrote:
> This function is too large to easily see that we're actually constructing 
> sequence of commands.
> I'd probably split construction of individual tool's command line into its 
> own function.
will do



Comment at: lib/Driver/ToolChains/Cuda.cpp:336
+  assert(StringRef(JA.getOffloadingArch()).startswith("gfx") &&
+ " unless gfx processor, backend should be clang");
+  const auto &TC =

tra wrote:
> No need for the leading space in the message.
will fix.



Comment at: lib/Driver/ToolChains/Cuda.cpp:344-345
+  // Add the input bc's created by compile step.
+  for (InputInfoList::const_iterator it = Inputs.begin(), ie = Inputs.end();
+   it != ie; ++it) {
+const InputInfo &II = *it;

tra wrote:
> `for (const InputInfo &it : Inputs)` ?
will fix



Comment at: lib/Driver/ToolChains/Cuda.cpp:350
+
+  std::string GFXNAME = JA.getOffloadingArch();
+

tra wrote:
> All-caps name looks like a macro. Rename to `GfxName` ?
will fix



Comment at: lib/Driver/ToolChains/Cuda.cpp:354-359
+  // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
+  for (auto Arg : Args) {
+if (Arg->getSpelling() == "--hip-device-lib-path=") {
+  LibraryPaths.push_back(Args.MakeArgString(Arg->getValue()));
+}
+  }

tra wrote:
> ```
> for (path : Args.getAllArgValues(...)) {
>LibraryPaths.push_back(Args.MakeArgString(path));
> }
> 
> ```
will fix



Comment at: lib/Driver/ToolChains/Cuda.cpp:375-378
+  addBCLib(C, Args, CmdArgs, LibraryPaths,
+   (Twine("oclc_isa_version_") + StringRef(GFXNAME).drop_front(3) +
+".amdgcn.bc")
+   .str());

tra wrote:
> This is somewhat unreadable. Perhaps you could construct the name in a temp 
> variable.
will do



Comment at: lib/Driver/ToolChains/Cuda.cpp:384
+  const char *ResultingBitcodeF =
+  C.addTempFile(C.getArgs().MakeArgString(TmpName.c_str()));
+  CmdArgs.push_back(ResultingBitcodeF);

tra wrote:
> You don't need to use c_str() for MakeArgString. It will happily accept 
> std::string.
will fix



Comment at: lib/Driver/ToolChains/Cuda.cpp:394
+  // The input to opt is the output from llvm-link.
+  OptArgs.push_back(ResultingBitcodeF);
+  // Pass optimization arg to opt.

tra wrote:
> `BitcodeOutputFile`?
will change



Comment at: lib/Driver/ToolChains/Cuda.cpp:417
+  const char *mcpustr = Args.MakeArgString("-mcpu=" + GFXNAME);
+  OptArgs.push_back(mcpustr);
+  OptArgs.push_back("-o");

tra wrote:
> I think you can get rid of the temp var here without hurting readability.
will do



Comment at: lib/Driver/ToolChains/Cuda.cpp:420
+  std::string OptOutputFileName =
+  C.getDriver().GetTemporaryPath("OPT_OUTPUT", "bc");
+  const char *OptOutputFile =

tra wrote:
> I wonder if we could derive temp file name from the input's name. Th

r333086 - [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

[Gabor Marton via cfe-commits]

Author: martong
Date: Wed May 23 07:24:02 2018
New Revision: 333086

URL: http://llvm.org/viewvc/llvm-project?rev=333086&view=rev
Log:
[ASTImporter] Fix missing implict CXXRecordDecl in 
ClassTemplateSpecializationDecl

Summary:
Currently we do not import the implicit CXXRecordDecl of a
ClassTemplateSpecializationDecl. This patch fixes it.

Reviewers: a.sidorin, xazax.hun, r.stahl

Subscribers: rnkovacs, dkrupp, cfe-commits

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

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

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=333086&r1=333085&r2=333086&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed May 23 07:24:02 2018
@@ -1959,14 +1959,20 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
+  if (Definition && Definition != D &&
+  // In contrast to a normal CXXRecordDecl, the implicit
+  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
+  // The definition of the implicit CXXRecordDecl in this case is the
+  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
+  // condition in order to be able to import the implict Decl.
+  !D->isImplicit()) {
 Decl *ImportedDef = Importer.Import(Definition);
 if (!ImportedDef)
   return nullptr;
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=333086&r1=333085&r2=333086&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Wed May 23 07:24:02 2018
@@ -1363,6 +1363,22 @@ TEST_P(ASTImporterTestBase, ShouldImport
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(
+  struct declToImport {
+  };
+  )",
+  Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier Verifier;
+  // Match the implicit Decl.
+  auto Matcher = cxxRecordDecl(has(cxxRecordDecl()));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase, ShouldImportImplicitCXXRecordDeclOfClassTemplate) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
   template 
   struct declToImport {
   };
@@ -1378,7 +1394,7 @@ TEST_P(ASTImporterTestBase, ShouldImport
 
 TEST_P(
 ASTImporterTestBase,
-
DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   R"(


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

r333082 - Fix duplicate class template definitions problem

[Gabor Marton via cfe-commits]

Author: martong
Date: Wed May 23 06:53:36 2018
New Revision: 333082

URL: http://llvm.org/viewvc/llvm-project?rev=333082&view=rev
Log:
Fix duplicate class template definitions problem

Summary:
We fail to import a `ClassTemplateDecl` if the "To" context already
contains a definition and then a forward decl.  This is because
`localUncachedLookup` does not find the definition.  This is not a
lookup error, the parser behaves differently than assumed in the
importer code.  A `DeclContext` contains one DenseMap (`LookupPtr`)
which maps names to lists.  The list is a special list `StoredDeclsList`
which is optimized to have one element.  During building the initial
AST, the parser first adds the definition to the `DeclContext`.  Then
during parsing the second declaration (the forward decl) the parser
again calls `DeclContext::addDecl` but that will not add a new element
to the `StoredDeclsList` rarther it simply overwrites the old element
with the most recent one.  This patch fixes the error by finding the
definition in the redecl chain.  Added tests for the same issue with
`CXXRecordDecl` and with `ClassTemplateSpecializationDecl`.  These tests
pass and they pass because in `VisitRecordDecl` and in
`VisitClassTemplateSpecializationDecl` we already use
`D->getDefinition()` after the lookup.

Reviewers: a.sidorin, xazax.hun, szepet

Subscribers: rnkovacs, dkrupp, cfe-commits

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

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

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=333082&r1=333081&r2=333082&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed May 23 06:53:36 2018
@@ -4070,6 +4070,17 @@ ASTNodeImporter::VisitTemplateTemplatePa
   TemplateParams);
 }
 
+// Returns the definition for a (forward) declaration of a ClassTemplateDecl, 
if
+// it has any definition in the redecl chain.
+static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) {
+  CXXRecordDecl *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
+  if (!ToTemplatedDef)
+return nullptr;
+  ClassTemplateDecl *TemplateWithDef =
+  ToTemplatedDef->getDescribedClassTemplate();
+  return TemplateWithDef;
+}
+
 Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   // If this record has a definition in the translation unit we're coming from,
   // but this particular declaration is not that definition, import the
@@ -4084,7 +4095,7 @@ Decl *ASTNodeImporter::VisitClassTemplat
 
 return Importer.Imported(D, ImportedDef);
   }
-  
+
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -4103,34 +4114,39 @@ Decl *ASTNodeImporter::VisitClassTemplat
 for (auto *FoundDecl : FoundDecls) {
   if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary))
 continue;
-  
+
   Decl *Found = FoundDecl;
   if (auto *FoundTemplate = dyn_cast(Found)) {
-if (IsStructuralMatch(D, FoundTemplate)) {
-  // The class templates structurally match; call it the same template.
 
-  // We found a forward declaration but the class to be imported has a
-  // definition.
-  // FIXME Add this forward declaration to the redeclaration chain.
-  if (D->isThisDeclarationADefinition() &&
-  !FoundTemplate->isThisDeclarationADefinition())
+// The class to be imported is a definition.
+if (D->isThisDeclarationADefinition()) {
+  // Lookup will find the fwd decl only if that is more recent than the
+  // definition. So, try to get the definition if that is available in
+  // the redecl chain.
+  ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate);
+  if (!TemplateWithDef)
 continue;
+  FoundTemplate = TemplateWithDef; // Continue with the definition.
+}
 
-  Importer.Imported(D->getTemplatedDecl(), 
+if (IsStructuralMatch(D, FoundTemplate)) {
+  // The class templates structurally match; call it the same template.
+
+  Importer.Imported(D->getTemplatedDecl(),
 FoundTemplate->getTemplatedDecl());
   return Importer.Imported(D, FoundTemplate);
-} 
+}
   }
-  
+
   ConflictingDecls.push_back(FoundDecl);
 }
-
+
 if (!ConflictingDecls.empty()) {
   Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary,
- ConflictingDecls.data(), 
+ ConflictingDecls.data(),
  Conflic

[PATCH] D45212: Add HIP toolchain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 148216.
yaxunl marked 19 inline comments as done.
yaxunl retitled this revision from "[HIP] Let CUDA toolchain support HIP 
language mode and amdgpu" to "Add HIP toolchain".
yaxunl edited the summary of this revision.
yaxunl added a comment.
Herald added a subscriber: mgorny.

Revised by Artem's comments.


https://reviews.llvm.org/D45212

Files:
  include/clang/Driver/Options.td
  lib/Driver/CMakeLists.txt
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/HIP.cpp
  lib/Driver/ToolChains/HIP.h
  test/Driver/Inputs/hip_multiple_inputs/lib1/lib1.bc
  test/Driver/Inputs/hip_multiple_inputs/lib2/lib2.bc
  test/Driver/hip-toolchain.hip

Index: test/Driver/hip-toolchain.hip
===
--- /dev/null
+++ test/Driver/hip-toolchain.hip
@@ -0,0 +1,84 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   --hip-device-lib=lib1.bc --hip-device-lib=lib2.bc \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib1 \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib2 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-o" [[OPT_BC_DEV1:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx803" "-o" [[OBJ_DEV1:".*-gfx803-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV1:.*out]]" [[OBJ_DEV1]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC]]
+
+// CHECK: [[LLVM_LINK]] [[A_BC]] [[B_BC]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
+
+// CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-o" [[OPT_BC_DEV2:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx900" "-o" [[OBJ_DEV2:".*-gfx900-.*o"]]
+
+// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[OBJ_DEV2]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64--linux-gnu" 
+// CHECK-SAME: "-aux-triple" "amdgcn-amd-amdhsa" "-emit-obj"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-SAME: "-aux-triple" "amdgcn-amd-amdhsa" "-emit-obj"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip"
+// CHECK-SAME: {{.*}} "-o" [[B_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC]]
+
+// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
+// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*o]]"
+
+// CHECK: [[LD:".*ld.lld"]] {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]]
+// CHECK-SAME: {{.*}} "-T" "{{.*}}.lk"
Index: lib/Driver/ToolChains/HIP.h
===
--- /dev/null
+++ lib/Driver/ToolChains/HIP.h
@@ -0,0 +1,123 @@
+//===--- HIP.h - HIP ToolChain Implementations --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructur

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 148218.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- addressed comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47187

Files:
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -131,9 +131,9 @@
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11",   "-include",
- TestHeaderName, TestFileName};
+std::vector Args = {
+"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
+"-include", TestHeaderName,  TestFileName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 tooling::ToolInvocation Invocation(
 Args,
@@ -666,6 +666,70 @@
  IncludeHeader("\"the/good/header.h\"";
 }
 
+TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  Includes.addMapping(TestHeaderName, "");
+  CollectorOpts.Includes = &Includes;
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader("")),
+   AllOf(QName("Y"), DeclURI(TestHeaderURI),
+ IncludeHeader("";
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = &Includes;
+  TestFileName = testPath("main.h");
+  TestFileURI = URI::createFile(TestFileName).toString();
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = &Includes;
+  TestFileName = testPath("no_ext_main");
+  TestFileURI = URI::createFile(TestFileName).toString();
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = &Includes;
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(IncURI;
+}
+
 TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
   CollectorOpts.CollectIncludePath = true;
   Annotations Header(R"(
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -200,23 +200,31 @@
 /// Gets a canonical include (URI of the header or   or "header") for
 /// header of \p Loc.
 /// Returns None if fails to get include header for \p Loc.
-/// FIXME: we should handle .inc files whose symbols are expected be exported by
-/// their conta

[PATCH] D47262: [VFS] Implement getRealPath in InMemoryFileSystem.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: bkramer.

Repository:
  rC Clang

https://reviews.llvm.org/D47262

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
   NormalizedFS.getCurrentWorkingDirectory().get()));
 }
 
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+  SmallString<16> Path;
+  EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+  auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+  };
+
+  FS.setCurrentWorkingDirectory("a");
+  EXPECT_EQ(GetRealPath("b"), "a/b");
+  EXPECT_EQ(GetRealPath("../b"), "b");
+  EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+  FS.setCurrentWorkingDirectory("/a");
+  EXPECT_EQ(GetRealPath("b"), "/a/b");
+  EXPECT_EQ(GetRealPath("../b"), "/b");
+  EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
 TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
   FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
   auto Stat = FS.status("/a");
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
   return {};
 }
 
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+  auto CWD = getCurrentWorkingDirectory();
+  if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+  Path.toVector(Output);
+  if (auto EC = makeAbsolute(Output))
+return EC;
+  llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+  return {};
+}
+
 } // namespace vfs
 } // namespace clang
 
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -374,6 +374,14 @@
   llvm::ErrorOr getCurrentWorkingDirectory() const override {
 return WorkingDirectory;
   }
+  /// Canonicalizes \p Path by combining with the current working
+  /// directory and normalizing the path (e.g. remove dots). If the current
+  /// working directory is not set, this returns errc::operation_not_permitted.
+  ///
+  /// This doesn't resolve symlinks as they are not supported in in-memory file
+  /// system.
+  std::error_code getRealPath(const Twine &Path,
+  SmallVectorImpl &Output) const override;
 
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
 };


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
   NormalizedFS.getCurrentWorkingDirectory().get()));
 }
 
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+  SmallString<16> Path;
+  EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+  auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+  };
+
+  FS.setCurrentWorkingDirectory("a");
+  EXPECT_EQ(GetRealPath("b"), "a/b");
+  EXPECT_EQ(GetRealPath("../b"), "b");
+  EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+  FS.setCurrentWorkingDirectory("/a");
+  EXPECT_EQ(GetRealPath("b"), "/a/b");
+  EXPECT_EQ(GetRealPath("../b"), "/b");
+  EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
 TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
   FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
   auto Stat = FS.status("/a");
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
   return {};
 }
 
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+  auto CWD = getCurrentWorkingDirectory();
+  if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+  Path.toVector(Output);
+  if (auto EC = makeAbsolute(Output))
+return EC;
+  llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+  return {};
+}
+
 } // namespace vfs
 } // namespace clang
 
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -374,6 +374,14 @@
   llvm::ErrorOr getCurrentWorkingDirectory() const o

r333092 - [clang-format] fix buildbots after r333085

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed May 23 08:21:33 2018
New Revision: 333092

URL: http://llvm.org/viewvc/llvm-project?rev=333092&view=rev
Log:
[clang-format] fix buildbots after r333085

Old gcc versions don't like raw string literals in macros.

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

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=333092&r1=333091&r2=333092&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 23 08:21:33 2018
@@ -5524,53 +5524,52 @@ TEST_F(FormatTest, WrapsTemplateDeclarat
 TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
   Style.ColumnLimit = 60;
-  EXPECT_EQ(R"test(
-// Baseline - no comments.
-template <
-typename aa::value>
-void f() {})test",
-format(R"test(
-// Baseline - no comments.
-template <
-typename aa::value>
-void f() {})test", Style));
+  EXPECT_EQ("// Baseline - no comments.\n"
+"template <\n"
+"typename aa::value>\n"
+"void f() {}",
+format("// Baseline - no comments.\n"
+   "template <\n"
+   "typename 
aa::value>\n"
+   "void f() {}",
+   Style));
 
-  EXPECT_EQ(R"test(
-template <
-typename aa::value>  // trailing
-void f() {})test",
-format(R"test(
-template <
-typename aa::value> // trailing
-void f() {})test", Style));
+  EXPECT_EQ("template <\n"
+"typename aa::value>  // trailing\n"
+"void f() {}",
+format("template <\n"
+   "typename aa::value> // 
trailing\n"
+   "void f() {}",
+   Style));
 
-  EXPECT_EQ(R"test(
-template <
-typename aa::value> /* line */
-void f() {})test",
-format(R"test(
-template ::value>  /* line */
-void f() {})test", Style));
+  EXPECT_EQ(
+  "template <\n"
+  "typename aa::value> /* line */\n"
+  "void f() {}",
+  format("template ::value>  /* line 
*/\n"
+ "void f() {}",
+ Style));
 
-  EXPECT_EQ(R"test(
-template <
-typename aa::value>  // trailing
-   // multiline
-void f() {})test",
-format(R"test(
-template <
-typename aa::value> // trailing
-  // multiline
-void f() {})test", Style));
+  EXPECT_EQ(
+  "template <\n"
+  "typename aa::value>  // trailing\n"
+  "   // multiline\n"
+  "void f() {}",
+  format("template <\n"
+ "typename aa::value> // trailing\n"
+ "  // multiline\n"
+ "void f() {}",
+ Style));
 
-  EXPECT_EQ(R"test(
-template ::value>  // trailing lng
-void f() {})test",
-format(R"test(
-template <
-typename aa::value> // trailing lng
-void f() {})test", Style));
+  EXPECT_EQ(
+  "template ::value>  // trailing lng\n"
+  "void f() {}",
+  format(
+  "template <\n"
+  "typename aa::value> // trailing lng\n"
+  "void f() {}",
+  Style));
 }
 
 TEST_F(FormatTest, WrapsTemplateParameters) {


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

[PATCH] D47190: [Driver] Add -ivfsoverlay-lib option to load VFS from shared library

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic updated this revision to Diff 148225.
asavonic added a comment.

Fixed warning about void* to function pointer cast.


Repository:
  rC Clang

https://reviews.llvm.org/D47190

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/HeaderSearchOptions.h
  lib/Frontend/CompilerInvocation.cpp
  unittests/Frontend/CMakeLists.txt
  unittests/Frontend/CompilerInstanceTest.cpp
  unittests/Frontend/VfsTestLib.cpp

Index: unittests/Frontend/VfsTestLib.cpp
===
--- /dev/null
+++ unittests/Frontend/VfsTestLib.cpp
@@ -0,0 +1,31 @@
+//===- unittests/Frontend/VfsTestLib.cpp - CI tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/ADT/SmallString.h"
+
+#ifdef _WIN32
+#define EXPORT __declspec(dllexport)
+#else
+#define EXPORT
+#endif
+
+extern "C" EXPORT clang::vfs::FileSystem *__clang_create_vfs() {
+  auto *FS = new clang::vfs::InMemoryFileSystem();
+
+  llvm::SmallString<128> FileName("virtual.file");
+  std::error_code EC = llvm::sys::fs::make_absolute(FileName);
+  if (EC) {
+return nullptr;
+  }
+
+  FS->addFile(FileName, (time_t)0, llvm::MemoryBuffer::getMemBuffer("foobar"));
+
+  return FS;
+}
Index: unittests/Frontend/CompilerInstanceTest.cpp
===
--- unittests/Frontend/CompilerInstanceTest.cpp
+++ unittests/Frontend/CompilerInstanceTest.cpp
@@ -9,8 +9,10 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "gtest/gtest.h"
 
@@ -71,4 +73,37 @@
   ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
 }
 
+static std::string LibPath(const std::string Name = "VfsTestLib") {
+  const std::vector &Argvs =
+  testing::internal::GetArgvs();
+  const char *Argv0 = Argvs.size() > 0 ? Argvs[0].c_str() : "FrontendTests";
+  llvm::SmallString<256> Buf(llvm::sys::path::parent_path(Argv0));
+  llvm::sys::path::append(Buf, (Name + LTDL_SHLIB_EXT).c_str());
+  return Buf.str();
+}
+
+TEST(CompilerInstance, VFSOverlayLibrary) {
+  // Create a CompilerInvocation that uses this overlay library.
+  const std::string VFSArg = "-ivfsoverlay-lib" + LibPath();
+  const char *Args[] = {"clang", VFSArg.c_str(), "-xc", "-"};
+
+  IntrusiveRefCntPtr Diags =
+  CompilerInstance::createDiagnostics(new DiagnosticOptions());
+
+  std::shared_ptr CInvok =
+  createInvocationFromCommandLine(Args, Diags);
+
+  if (!CInvok)
+FAIL() << "could not create compiler invocation";
+
+  CompilerInstance Instance;
+  Instance.setDiagnostics(Diags.get());
+  Instance.setInvocation(CInvok);
+  Instance.createFileManager();
+
+  // Check if the virtual file exists which means that our VFS is used by the
+  // CompilerInstance.
+  ASSERT_TRUE(Instance.getFileManager().getFile("virtual.file"));
+}
+
 } // anonymous namespace
Index: unittests/Frontend/CMakeLists.txt
===
--- unittests/Frontend/CMakeLists.txt
+++ unittests/Frontend/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Support
   )
+list(APPEND LLVM_OPTIONAL_SOURCES VfsTestLib.cpp)
 
 add_clang_unittest(FrontendTests
   ASTUnitTest.cpp
@@ -21,3 +22,15 @@
   clangCodeGen
   clangFrontendTool
   )
+
+add_library(VfsTestLib SHARED VfsTestLib.cpp)
+target_link_libraries(VfsTestLib clangBasic)
+set_target_properties(VfsTestLib
+  PROPERTIES PREFIX ""
+  SUFFIX ${LTDL_SHLIB_EXT}
+  )
+set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
+set_output_directory(VfsTestLib BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
+get_target_property(test_suite_folder FrontendTests FOLDER)
+set_property(TARGET VfsTestLib PROPERTY FOLDER "${test_suite_folder}")
+add_dependencies(FrontendTests VfsTestLib)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -66,6 +66,7 @@
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
@@ -1841,6 +1842,9 @@
 
   for (const auto *A : Args.filtered(OPT_ivfsoverlay))
 Opts.AddVFSOverlayFile(A->getValue());
+
+  for (const Arg *A : Args.filtered(OPT_ivfsoverlay_lib))
+Opts.AddVFSOverlayLib(A->getValue());
 }
 
 void CompilerInvocation::setLangDefaults(LangO

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 148226.
avt77 added a comment.

- switched to llvm::sort  - from std::sort (tnx to mgrang)
- added a new counter (hope it will improve the output numbers)
- slightly changed a couple of existing counters
- seriously improved debug/monitor logging


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123);
+  char R2 = foo('d', 1234);
+  int R3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
 //
 // The LLVM Compiler Infrastructure
 //
@@ -9,7 +10,6 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
 
+#define DEBUG_TYPE "templateinst"
 using namespace clang;
 
 static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/

[PATCH] D47137: [Sparc] Add floating-point register names

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/Sparc.cpp:32
+"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", 
"f32",
+"f33", "f34", "f35", "f36", "f37", "f38", "f39", "f40", "f41", "f42", 
"f43",
+"f44", "f45", "f46", "f47", "f48", "f49", "f50", "f51", "f52", "f53", 
"f54",

There's no such register as f33 (nor any odd-numbered reg above f32).



Comment at: lib/Basic/Targets/Sparc.cpp:52
+
+// Double precision floating-point register
+{{"d0"}, "f0"},   {{"d1"}, "f2"},   {{"d2"}, "f4"},   {{"d3"}, "f6"},

AFAICT, gcc doesn't actually accept "d" and "q" aliases in its inline asm, so 
probably no point in LLVM doing so either.


https://reviews.llvm.org/D47137



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 148228.
trixirt added a comment.

Rename test to dwarf5-expansion-checksum.c


Repository:
  rC Clang

https://reviews.llvm.org/D47260

Files:
  test/CodeGen/dwarf5-expansion-checksum.c


Index: test/CodeGen/dwarf5-expansion-checksum.c
===
--- /dev/null
+++ test/CodeGen/dwarf5-expansion-checksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif


Index: test/CodeGen/dwarf5-expansion-checksum.c
===
--- /dev/null
+++ test/CodeGen/dwarf5-expansion-checksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40221: [clang-format] Parse blocks in braced lists

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton edited reviewers, added: klimek; removed: djasper.
benhamilton added a comment.

@djasper isn't available to review.

At a high level, this seems good, but I'd like @klimek to take a look.




Comment at: lib/Format/UnwrappedLineParser.cpp:1320
+// \endcode
+bool UnwrappedLineParser::tryToParseBlock() {
+  // Consume the leading ^.

Is it standard to return a value from these `tryToParseFoo()` methods, even if 
nobody uses it?

I think we should either check the return value somewhere, or make this return 
`void`.



Comment at: lib/Format/UnwrappedLineParser.cpp:1324-1327
+  if (!Style.isCpp()) {
+// Blocks are only supported in C++ and Objective-C.
+return false;
+  }

Style: Remove curly braces for one-line if blocks.



https://reviews.llvm.org/D40221



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

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:2137
+  do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+  nextToken();

jolesiak wrote:
> `tok::r_brace` could be skiped - see comment to line 2143.
Done.



Comment at: lib/Format/UnwrappedLineParser.cpp:2143
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+} else {

jolesiak wrote:
> We have to add `return` after `addUnwrappedLine` as `parseBlock` does consume 
> `tok::r_brace`. Without `return` we will consume tokens after `}`. This 
> problem will rarely occur as most lines end with `tok::semi` or 
> `tok::r_brace` and it will be terminated properly (however maybe not handled 
> properly as we just skip every token in `else`) by `if` branch.
> 
> Test like:
> ```
> @implementation Foo
> - (foo)foo {
> }
> @end
> @implementation Bar
> - (bar)bar {
> }
> @end
> ```
> will distinguish version with `return` from one without. Therefore, I think 
> we should add it.
Done, test added.


Repository:
  rC Clang

https://reviews.llvm.org/D47095



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

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 148231.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.

@jolesiak


Repository:
  rC Clang

https://reviews.llvm.org/D47095

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -328,7 +328,14 @@
"}\n"
"+ (id)init;\n"
"@end");
-
+  verifyFormat("@interface Foo\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n"
+   "@implementation Bar\n"
+   "- (void)bar {\n"
+   "}\n"
+   "@end");
   Style.ColumnLimit = 40;
   verifyFormat("@interface c () <\n"
"c, c,\n"
@@ -998,6 +1005,59 @@
   verifyFormat("MACRO(new:)\n");
   verifyFormat("MACRO(delete:)\n");
   verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+  verifyFormat("@implementation Foo\n"
+   "// Testing\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+
+  verifyFormat("@interface Foo\n"
+   "// Testing\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end\n");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,24 @@
   // "} n, m;" will end up in one unwrapped line.
 }
 
+void UnwrappedLineParser::parseObjCMethod() {
+  assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) &&
+ "'(' or identifier expected.");
+  do {
+if (FormatTok->Tok.is(tok::semi)) {
+  nextToken();
+  addUnwrappedLine();
+  return;
+} else if (FormatTok->Tok.is(tok::l_brace)) {
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+  return;
+} else {
+  nextToken();
+}
+  } while (!eof());
+}
+
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do {
@@ -2157,6 +2175,9 @@
   // Ignore stray "}". parseStructuralElement doesn't consume them.
   nextToken();
   addUnwrappedLine();
+} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+  nextToken();
+  parseObjCMethod();
 } else {
   parseStructuralElement();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Dave Green via Phabricator via cfe-commits]

dmgreen created this revision.
dmgreen added reviewers: SjoerdMeijer, hfinkel, tyler.nowicki, anemet, alexfh.
Herald added a subscriber: zzheng.

This adds support for the unroll_and_jam pragma, to go with 
https://reviews.llvm.org/D41953. The name of
the pragma is copied from the Intel compiler, and most of the code works the 
same as
for unroll.

I have some doubts whether this will ever be used sensibly in the real world, 
but can
be useful for testing and was not difficult to put together.


https://reviews.llvm.org/D47267

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

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam 4
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives 'unroll_and_jam(disable)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma clang loop unroll_and_jam(disable)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{incompatible directives 'unroll_and_jam(full)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam(4)
+#pragma clang loop unroll_and_jam(full)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, unroll_and_jam, unroll_and_jam_count, or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+voi

[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h

[David Kreitzer via Phabricator via cfe-commits]

DavidKreitzer added a comment.

Hi Craig, just one comment on the details. Everything else looks good.




Comment at: lib/Headers/x86intrin.h:47
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__POPCNT__)
-#include 

I see that you are removing this popcntintrin.h inclusion without adding it to 
immintrin.h. Is that because you are relying on the inclusion from smmintrin.h?

If so, won't that cause a problem on Windows with -mpopcnt for targets that 
don't include smmintrin.h?


https://reviews.llvm.org/D47182



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

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

> I have some doubts whether this will ever be used sensibly in the real world, 
> but can
>  be useful for testing and was not difficult to put together.

I definitely support having pragmas for these kinds of loop transformations. In 
my experience, they are used.


https://reviews.llvm.org/D47267



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

[PATCH] D47268: [CUDA] Fixed the list of GPUs supported by CUDA-9

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added reviewers: jlebar, klimek.
Herald added subscribers: bixia, sanjoy.

Removed sm_20 as it is not supported by CUDA-9.
Added sm_37.


https://reviews.llvm.org/D47268

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -164,8 +164,8 @@
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";
   if (FS.exists(FilePath)) {
 for (const char *GpuArchName :
- {"sm_20", "sm_30", "sm_32", "sm_35", "sm_50", "sm_52", "sm_53",
-   "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
+ {"sm_30", "sm_32", "sm_35", "sm_37", "sm_50", "sm_52", "sm_53",
+  "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
   const CudaArch GpuArch = StringToCudaArch(GpuArchName);
   if (Version >= MinVersionForCudaArch(GpuArch) &&
   Version <= MaxVersionForCudaArch(GpuArch))


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -164,8 +164,8 @@
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";
   if (FS.exists(FilePath)) {
 for (const char *GpuArchName :
- {"sm_20", "sm_30", "sm_32", "sm_35", "sm_50", "sm_52", "sm_53",
-   "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
+ {"sm_30", "sm_32", "sm_35", "sm_37", "sm_50", "sm_52", "sm_53",
+  "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
   const CudaArch GpuArch = StringToCudaArch(GpuArchName);
   if (Version >= MinVersionForCudaArch(GpuArch) &&
   Version <= MaxVersionForCudaArch(GpuArch))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47137: [Sparc] Add floating-point register names

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: lib/Basic/Targets/Sparc.cpp:52
+
+// Double precision floating-point register
+{{"d0"}, "f0"},   {{"d1"}, "f2"},   {{"d2"}, "f4"},   {{"d3"}, "f6"},

jyknight wrote:
> AFAICT, gcc doesn't actually accept "d" and "q" aliases in its inline asm, so 
> probably no point in LLVM doing so either.
Ah you're right; trying to use it gives "invalid register name". Having said 
that, maybe it's a nice thing to support anyway?


https://reviews.llvm.org/D47137



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

After thinking about this a bit...  The # directive will cause the filename to 
be identified as the source for the subsequent lines.  That means it will show 
up as the original source file in the line table.
However, the compiler doesn't have the actual source file of the header (or the 
original source file, for that matter) and so cannot compute a correct checksum 
for it.  That means, it should omit the checksum, for any file identified with 
a # directive.
I haven't looked at how CodeView handles this situation; for DWARF v5 I made 
inconsistent use of a checksum into an assertion offense.
Should we reconsider that?  Or have the # directive cause Clang to omit 
checksums for *all* files, even the one it is actually reading?
@aprantl, @rnk, thoughts?


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Based on the cfe-dev discussion 
 we'll want to 
handle the case of NSInteger with `%z` format on Darwin separately from other 
attempts at portability warnings in printf formats. I'll therefore re-post this 
patch as-is and CC all of you.


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

r333098 - [CUDA] Fixed the list of GPUs supported by CUDA-9.

[Artem Belevich via cfe-commits]

Author: tra
Date: Wed May 23 09:45:23 2018
New Revision: 333098

URL: http://llvm.org/viewvc/llvm-project?rev=333098&view=rev
Log:
[CUDA] Fixed the list of GPUs supported by CUDA-9.

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

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

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=333098&r1=333097&r2=333098&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Wed May 23 09:45:23 2018
@@ -164,8 +164,8 @@ CudaInstallationDetector::CudaInstallati
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";
   if (FS.exists(FilePath)) {
 for (const char *GpuArchName :
- {"sm_20", "sm_30", "sm_32", "sm_35", "sm_50", "sm_52", "sm_53",
-   "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
+ {"sm_30", "sm_32", "sm_35", "sm_37", "sm_50", "sm_52", "sm_53",
+  "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
   const CudaArch GpuArch = StringToCudaArch(GpuArchName);
   if (Version >= MinVersionForCudaArch(GpuArch) &&
   Version <= MaxVersionForCudaArch(GpuArch))


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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Having taken a closer look, I think the cache can be simplified/separated a bit 
more cleanly by returning shared pointers and not allowing lookups, instead 
restoring limited ownership in CppFile...

Happy to discuss more, esp if you might disagree :)




Comment at: clangd/ClangdUnit.h:132
 
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.

This may be change aversion, but I'm not sure this class does enough after this 
change - it doesn't store the inputs or the outputs/cache, so it kind of seems 
like it wants to be a function.

I guess the motivation here is that storing the outputs means dealing with the 
cache, and the cache is local to TUScheduler.
But `CppFile` is only used in TUScheduler, so we could move this there too? It 
feels like expanding the scope more than I'd like.

The upside is that I think it's a more natural division of responsibility: 
`CppFile` could continue to be the "main" holder of the `shared_ptr` 
(which we don't limit, but share), and instead of `Optional` it'd 
have a `weak_ptr` which is controlled and can be refreshed through 
the cache.

As discussed offline, the cache could look something like:
```
class Cache {
   shared_ptr put(ParsedAST);
   void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
   void hintUnused(ParsedAST*); // optional, releases when client abandons
}

shared_ptr CppFile::getAST() {
  shared_ptr AST = WeakAST.lock();
  if (AST)
Cache.hintUsed(AST.get());
  else
WeakAST = AST = Cache.put(build...);
  return AST;
}
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



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

[PATCH] D47268: [CUDA] Fixed the list of GPUs supported by CUDA-9

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333098: [CUDA] Fixed the list of GPUs supported by CUDA-9. 
(authored by tra, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47268?vs=148232&id=148236#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47268

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -164,8 +164,8 @@
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";
   if (FS.exists(FilePath)) {
 for (const char *GpuArchName :
- {"sm_20", "sm_30", "sm_32", "sm_35", "sm_50", "sm_52", "sm_53",
-   "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
+ {"sm_30", "sm_32", "sm_35", "sm_37", "sm_50", "sm_52", "sm_53",
+  "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
   const CudaArch GpuArch = StringToCudaArch(GpuArchName);
   if (Version >= MinVersionForCudaArch(GpuArch) &&
   Version <= MaxVersionForCudaArch(GpuArch))


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -164,8 +164,8 @@
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";
   if (FS.exists(FilePath)) {
 for (const char *GpuArchName :
- {"sm_20", "sm_30", "sm_32", "sm_35", "sm_50", "sm_52", "sm_53",
-   "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
+ {"sm_30", "sm_32", "sm_35", "sm_37", "sm_50", "sm_52", "sm_53",
+  "sm_60", "sm_61", "sm_62", "sm_70", "sm_72"}) {
   const CudaArch GpuArch = StringToCudaArch(GpuArchName);
   if (Version >= MinVersionForCudaArch(GpuArch) &&
   Version <= MaxVersionForCudaArch(GpuArch))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

This is straightforward in that it clones the implementation of `#pragma 
unroll` for `unroll_and_jam`.

Hence, it also shares the same problem of clang's LoopHints, namely that the 
order of loop transformations (if there are multiple on the same loop: loop 
distribution, vectorization, etc) is defined by the order of the passes in the 
pass pipeline, which should be an implementation detail.

I am currently working on this topic. Could we maybe disable the `#pragma clang 
loop unroll_and_jam` spelling ftm to avoid compatibility issues? However, the 
problem already exists for the other loop hints, so I will have to ensure 
compatibility with those anyway.


https://reviews.llvm.org/D47267



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

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D46943#1109199, @ioeric wrote:

> > - Symbols store their paths as URIs ⇒ we need to parse them in order to 
> > apply heuristics. We could avoid that by writing a version of 
> > header-matching that also works on URIs, but that would mean more 
> > complexity.
>
> Yeah, this is a bit unfortunate. It's probably OK to parse URIs; they are not 
> that expensive after all (we might want to do some measurement though).


Yeah. I really wish we had microbenchmarks for things like completion.

>> - Merging quality signals from headers now requires an extra paramater: name 
>> of the source file. I wonder if it's worth extracting builders for symbol 
>> qualities into separate classes to keep the current nice signatures, i.e. 
>> `merge(Symbol& IndexResult)`.
> 
> I'm not very familiar with `SymbolQualitySignals`. But if we decide to use 
> main file name as a signal, it might make sense to pass it in through the 
> constructor?

That's possible, but that essentially turns `SymbolQualitySignals` from a data 
class to a stateful builder.

>> - How should we match the header with the main file?  Our options are:
>>   - (proposed by Eric) use main file regex from clang-format for that. I'm 
>> not entirely sure it suits us well, since the regex is designed to work on 
>> paths inside #include directive, but we're getting ours from the Symbols and 
>> Sema AST Decls. Moreover, it means we're gonna read .clang-format files to 
>> get that style.
> 
> I think the ".clang-format problem" is not specific to the header matching 
> here. We would eventually need proper format style support in clangd anyway, 
> as clangd provides formatting features (e.g. reformat and include insertion).

Yeah, `IncludeMainRegex` does look like a useful setting from clang-format. And 
maybe using clang-format settings is a good idea there. I'm just a bit weary of 
adding this logic in this change along with the completion changes.
So I'd go with a simple heuristic for starters to solve a problem at hand. 
Happy to improve it to use clang-format regex with a follow-up change if 
everyone agrees that's a good idea. I mostly feel uneasy about the added 
complexity to the current change.




Comment at: clangd/MatchingHeader.cpp:44
+  auto HeaderNoExt = llvm::sys::path::filename(Header);
+  return SourceNoExt.equals_lower(HeaderNoExt);
+}

ioeric wrote:
> Why `equals_lower`?
A former-windows-developer habbit. I don't think it hurts in any way if we do 
`equals_lower` here, we'll merely work in those strange cases where the 
extensions are not lower-case.
This is also consistent with isHeaderFile/isSourceFile (moved from ClangdServer)




Comment at: clangd/MatchingHeader.h:1
+//===--- MatchingHeader.h - Match source and header files *- 
C++-*-===//
+//

ioeric wrote:
> I wonder if we could merge this into Headers.h
Thanks, I totally forgot we have `Headers.h`.  Will move the helpers there.



Comment at: clangd/MatchingHeader.h:24
+/// header for a \p Source.
+bool isMainHeaderFor(PathRef Header, PathRef Source);
+

ioeric wrote:
> We might want to briefly explain what a matching header is and what the 
> heuristics are.
My idea was to not elaborate before we agree on what those heuristics should be.
Given the heuristics are so simple and there are suggestions to change them, 
documenting the current behavior seems like a bad idea. I'd rather keep them a 
black box for now.
Does that make sense? Maybe add a comment that the heuristics are likely to 
change, so the users shouldn't rely on them too much?



Comment at: clangd/Quality.cpp:28-29
+struct DeclFiles {
+  bool AllDeclsInMainFile = false;
+  bool HasDeclsInMatchingHeader = false;
+};

ioeric wrote:
> Could we merge these two flags into something like 
> `IsDeclaredInMainHeaderOrFile`?
The two flags are built differently.
**Any** decl in the matching header gives a boost. Otherwise, **all** decls 
should be in the main file to get a boost.
The second one is built differently for the reasons outlined in the previous 
comments on why it might not be the best idea to boost completion items that 
have one of the inside the current file:
- It gives inconsistent ranking for different completion points (before/after 
declaration)
- The fact that a function has definition in the current file does not 
necessarily mean I want to call it more often than the others.



Comment at: clangd/Quality.cpp:40
+  assert(MainFile);
+  for (auto *Redecl : D->redecls()) {
+auto Loc = SM.getSpellingLoc(Redecl->getLocation());

ioeric wrote:
> I wonder if it's still necessary to check all `redecls`. Would it be 
> sufficient to check `D`, if `D` is the decl we referencing to?
I don't think it's sufficient. How could we compute the flags that this 
function returns by looking at just

[clang-tools-extra] r333100 - [Documentation] Move some Clang-tidy changes to proper sections.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed May 23 10:25:22 2018
New Revision: 333100

URL: http://llvm.org/viewvc/llvm-project?rev=333100&view=rev
Log:
[Documentation] Move some Clang-tidy changes to proper sections.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=333100&r1=333099&r2=333100&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed May 23 10:25:22 2018
@@ -99,10 +99,10 @@ Improvements to clang-tidy
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
-- New alias :doc:`fuchsia-header-anon-namespaces
-  ` to 
:doc:`google-build-namespaces
-  `
-  added.
+- New :doc:`cppcoreguidelines-narrowing-conversions
+  `_ check
+
+  Checks for narrowing conversions, e. g. ``int i = 0; i += 0.1;``.
 
 - New :doc:`fuchsia-multiple-inheritance
   ` check.
@@ -180,6 +180,11 @@ Improvements to clang-tidy
 - The `AnalyzeTemporaryDtors` option was removed, since the corresponding
   `cfg-temporary-dtors` option of the Static Analyzer now defaults to `true`.
 
+- New alias :doc:`fuchsia-header-anon-namespaces
+  ` to 
:doc:`google-build-namespaces
+  `
+  added.
+
 - New alias :doc:`hicpp-avoid-goto
   ` to :doc:`cppcoreguidelines-avoid-goto
   `
@@ -241,11 +246,6 @@ Improvements to clang-tidy
 
 - The 'google-runtime-member-string-references' check was removed.
 
-- New `cppcoreguidelines-narrowing-conversions
-  
`_
 check
-
-  Checks for narrowing conversions, e.g. ``int i = 0; i += 0.1;``.
-
 Improvements to include-fixer
 -
 


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

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Bevin,

Could you please address these comments?




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89
 SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
+StateMgr(stateMgr), ArrayIndexTy(context.getSignedSizeType()),
 ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}

As you correctly pointed, ssize_t is 32-bit on 32-bit systems. Therefore, it is 
too short. So, we can leave this line as-is.



Comment at: test/Analysis/index-type.c:13
   char arr[X86_ARRAY_SIZE];
-  char *ptr = arr + UINT_MAX/2;
+  char *ptr = arr + UINT_MAX/4;
   ptr += 2;  // index shouldn't overflow

We don't need to fix the test - it is correct. We have to fix the type instead.



Comment at: test/Analysis/index-type.c:25
+void testOutOfBounds() {
+  // not out of bounds
+  buf[SIZE-1] = 1; // no-warning

The comments should be normal sentences: "Not out of bounds."


https://reviews.llvm.org/D46944



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

[libcxx] r333103 - Teach __libcpp_is_floating_point that __fp16 and _Float16 are

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed May 23 10:31:09 2018
New Revision: 333103

URL: http://llvm.org/viewvc/llvm-project?rev=333103&view=rev
Log:
Teach __libcpp_is_floating_point that __fp16 and _Float16 are
floating-point types.

rdar://problem/40377353

Added:
libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp
Modified:
libcxx/trunk/include/type_traits

Modified: libcxx/trunk/include/type_traits
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=333103&r1=333102&r2=333103&view=diff
==
--- libcxx/trunk/include/type_traits (original)
+++ libcxx/trunk/include/type_traits Wed May 23 10:31:09 2018
@@ -733,6 +733,10 @@ _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR boo
 // is_floating_point
 
 template  struct __libcpp_is_floating_point  : public 
false_type {};
+template <>  struct __libcpp_is_floating_point<__fp16>  : public 
true_type {};
+#ifdef __FLT16_MANT_DIG__
+template <>  struct __libcpp_is_floating_point<_Float16>: public 
true_type {};
+#endif
 template <>  struct __libcpp_is_floating_point   : public 
true_type {};
 template <>  struct __libcpp_is_floating_point  : public 
true_type {};
 template <>  struct __libcpp_is_floating_point : public 
true_type {};

Added: libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp?rev=333103&view=auto
==
--- libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp Wed May 23 
10:31:09 2018
@@ -0,0 +1,22 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// 
+//
+// Test that is_floating_point::value is true when T=__fp16 or T=_Float16.
+
+#include 
+
+int main() {
+  static_assert(std::is_floating_point<__fp16>::value, "");
+#ifdef __FLT16_MANT_DIG__
+  static_assert(std::is_floating_point<_Float16>::value, "");
+#endif
+  return 0;
+}


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

r333094 - Test Commit. Fix namespace comment

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Wed May 23 08:49:12 2018
New Revision: 333094

URL: http://llvm.org/viewvc/llvm-project?rev=333094&view=rev
Log:
Test Commit. Fix namespace comment

Signed-off-by: Mikhail Ramalho 

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h?rev=333094&r1=333093&r2=333094&view=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
 Wed May 23 08:49:12 2018
@@ -85,8 +85,8 @@ private:
 bool Assumption);
 };
 
-} // end GR namespace
+} // end namespace ento
 
-} // end clang namespace
+} // end namespace clang
 
 #endif


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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a subscriber: sammccall.
leonardchan added inline comments.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+  "Fixed point types are only allowed in C">;
 

rsmith wrote:
> Diagnostics should not be capitalized. Also, we generally allow conforming C 
> extensions to be used in other languages unless there is a really good reason 
> not to.
We decided not to allow fixed point types in other languages because there is 
no specification provided in N1169 for addressing some features in other 
languages. Using C++ as an example, N1169 does not provide recommended 
characters when name mangling so we do not allow this in C++.



Comment at: lib/AST/ItaniumMangle.cpp:2552
+  case BuiltinType::ULongAccum:
+llvm_unreachable("Fixed point types are disabled for c++");
   case BuiltinType::Half:

rsmith wrote:
> Please check what GCC uses to mangle these, and follow suit; if GCC doesn't 
> have a mangling, you can use a vendor mangling (`u6_Accum`) or produce an 
> error for now, but please open an issue at 
> https://github.com/itanium-cxx-abi/cxx-abi/ to pick a real mangling.
It seems that GCC uses the characters for each fixed point type's corresponding 
integral type (https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/mangle.c). 
Will follow up on this if we end up enabling fixed point types for C++.



Comment at: lib/CodeGen/CGDebugInfo.cpp:678-681
+  case BuiltinType::UShortAccum:
+  case BuiltinType::UAccum:
+  case BuiltinType::ULongAccum:
 Encoding = llvm::dwarf::DW_ATE_unsigned;

rsmith wrote:
> @echristo @dblaikie Is this appropriate?
My bad, this should be changed to `DW_ATE_signed_fixed` and 
`DW_ATE_unsigned_fixed`



Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+  llvm_unreachable("No USR name mangling for fixed point types.");
 case BuiltinType::Float16:

rsmith wrote:
> leonardchan wrote:
> > phosek wrote:
> > > We need some solution for fixed point types.
> > Added character ~ to indicate fixed point type followed by string detailing 
> > the type. I have not added a test to it because logically, I do not think 
> > we will ever reach that point. This logic is implemented in the VisitType 
> > method, which mostly gets called by visitors to c++ nodes like 
> > VisitTemplateParameterList, but we have disabled the use of fixed point 
> > types in c++. VisitType does get called in VisitFunctionDecl but the 
> > function exits early since we are not reading c++ (line 
> > lib/Index/USRGeneration.cpp:238).
> @rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme 
> documented anywhere?)
I chatted with @sammccall about this who said it was ok to add these types if 
no one opposed this. I posted this on cfe-dev also and it seemed that no one 
spoke up about it, so I thought this was ok.

I also couldn't find any standard or documentation about reserving characters 
for USR. It doesn't seem that USR is also parsed in any way, so I don't think 
I'm breaking anything (running ninja check-all passes).

And unless we also do enable this for C++, this code actually may not be run 
since this method will not be visited if limited to C.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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