[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 117956.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.

Fixes according to review comments. Reuse ast matchers in LoopUnrolling.cpp. 
Avoid some recursion (however the isChanged() is still recursive but it is very 
small and simple).


Repository:
  rL LLVM

https://reviews.llvm.org/D37897

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  test/Analysis/global-vars.c

Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+  int i;
+  for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+  }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+  int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -99,7 +99,10 @@
 declRefExpr(to(varDecl(VarNodeMatcher)),
   binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="),
hasOperatorName("/="), hasOperatorName("*="),
-   hasOperatorName("-=")),
+   hasOperatorName("-="), hasOperatorName("%="),
+   hasOperatorName("<<="), hasOperatorName(">>="),
+   hasOperatorName("&="), hasOperatorName("|="),
+   hasOperatorName("^=")),
  hasLHS(ignoringParenImpCasts(
  declRefExpr(to(varDecl(VarNodeMatcher)));
 }
@@ -283,5 +286,16 @@
 return false;
   return true;
 }
+
+bool isVarChanged(const FunctionDecl *FD, const VarDecl *VD) {
+  if (!FD->getBody())
+return false;
+  auto Match = match(
+  stmt(hasDescendant(stmt(anyOf(
+  callByRef(equalsNode(VD)), getAddrTo(equalsNode(VD)),
+  assignedToRef(equalsNode(VD)), changeIntBoundNode(equalsNode(VD)),
+  *FD->getBody(), FD->getASTContext());
+  return !Match.empty();
 }
-}
+} // namespace ento
+} // namespace clang
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,86 @@
 // Utility methods.
 //===--===//
 
+static bool isChanged(const Decl *D, const VarDecl *VD) {
+  if (const auto *FD = dyn_cast(D)) {
+return isVarChanged(FD, VD);
+  }
+  if (const auto *Rec = dyn_cast(D)) {
+for (const auto *RecChild : Rec->decls()) {
+  if (isChanged(RecChild, VD))
+return true;
+}
+  }
+  return false;
+}
+
+/** Get initial state for global static variable */
+ProgramStateRef ExprEngine::getInitialStateForGlobalStaticVar(
+const LocationContext *LCtx, ProgramStateRef State, const VarDecl *VD) {
+  // Is variable changed anywhere in TU?
+  for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) {
+if (isChanged(D, VD))
+  return State;
+  }
+
+  // What is the initialized value?
+  llvm::APSInt InitVal;
+  if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+  return State;
+  } else {
+InitVal = 0;
+  }
+
+  const MemRegion *R = State->getRegion(VD, LCtx);
+  if (!R)
+return State;
+  SVal V = State->getSVal(loc::MemRegionVal(R));
+  SVal Constraint_untested =
+  evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+  Optional Constraint =
+  Constraint_untested.getAs();
+  if (!Constraint)
+return State;
+  return State->assume(*Constraint, true);
+}
+
+static void getGlobalStaticVars(const Stmt *FuncBody,
+llvm::SmallSet *Vars) {
+  std::stack Children;
+  Children.push(FuncBody);
+  while (!Children.empty()) {
+const Stmt *Child = Children.top();
+Children.pop();
+if (!Child)
+  continue;
+for (const Stmt *C : Child->children()) {
+  Children.push(C);
+}
+if (const DeclRefExpr *D = dyn_cast(Child)) {
+  const VarDecl *VD = dyn_cast(D->getDecl());
+  if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+  VD->getType()->isIntegerType() &&
+  VD->getStorageClass() == SC_Static &&
+  !VD->getType()->isPointe

r315045 - [CodeGen] Emit a helper function for __builtin_os_log_format to reduce

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Oct  6 00:12:46 2017
New Revision: 315045

URL: http://llvm.org/viewvc/llvm-project?rev=315045&view=rev
Log:
[CodeGen] Emit a helper function for __builtin_os_log_format to reduce
code size.

Currently clang expands a call to __builtin_os_log_format into a long
sequence of instructions at the call site, causing code size to
increase in some cases.

This commit attempts to reduce code size by emitting a helper function
that can be shared by calls to __builtin_os_log_format with similar
formats and arguments. The helper function has linkonce_odr linkage to
enable the linker to merge identical functions across translation units.
Attribute 'noinline' is attached to the helper function at -Oz so that
the inliner doesn't inline functions that can potentially be merged.

This commit also fixes a bug where the generated IR writes past the end
of the buffer when "%m" is the last specifier appearing in the format
string passed to __builtin_os_log_format.

Original patch by Duncan Exon Smith.

rdar://problem/34065973
rdar://problem/34196543

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/builtins.c
cfe/trunk/test/CodeGenObjC/os_log.m

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=315045&r1=315044&r2=315045&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  6 00:12:46 2017
@@ -663,6 +663,179 @@ Value *CodeGenFunction::EmitCheckedArgFo
   return ArgValue;
 }
 
+/// Get the argument type for arguments to os_log_helper.
+static CanQualType getOSLogArgType(ASTContext &C, int Size) {
+  QualType UnsignedTy = C.getIntTypeForBitwidth(Size * 8, /*Signed=*/false);
+  return C.getCanonicalType(UnsignedTy);
+}
+
+llvm::Function *CodeGenFunction::generateBuiltinOSLogHelperFunction(
+const analyze_os_log::OSLogBufferLayout &Layout,
+CharUnits BufferAlignment) {
+  ASTContext &Ctx = getContext();
+
+  llvm::SmallString<64> Name;
+  {
+raw_svector_ostream OS(Name);
+OS << "__os_log_helper";
+OS << "_" << BufferAlignment.getQuantity();
+OS << "_" << int(Layout.getSummaryByte());
+OS << "_" << int(Layout.getNumArgsByte());
+for (const auto &Item : Layout.Items)
+  OS << "_" << int(Item.getSizeByte()) << "_"
+ << int(Item.getDescriptorByte());
+  }
+
+  if (llvm::Function *F = CGM.getModule().getFunction(Name))
+return F;
+
+  llvm::SmallVector Params;
+  Params.emplace_back(Ctx, nullptr, SourceLocation(), 
&Ctx.Idents.get("buffer"),
+  Ctx.VoidPtrTy, ImplicitParamDecl::Other);
+
+  for (unsigned int I = 0, E = Layout.Items.size(); I < E; ++I) {
+char Size = Layout.Items[I].getSizeByte();
+if (!Size)
+  continue;
+
+Params.emplace_back(Ctx, nullptr, SourceLocation(),
+&Ctx.Idents.get(std::string("arg") + 
std::to_string(I)),
+getOSLogArgType(Ctx, Size), ImplicitParamDecl::Other);
+  }
+
+  FunctionArgList Args;
+  for (auto &P : Params)
+Args.push_back(&P);
+
+  // The helper function has linkonce_odr linkage to enable the linker to merge
+  // identical functions. To ensure the merging always happens, 'noinline' is
+  // attached to the function when compiling with -Oz.
+  const CGFunctionInfo &FI =
+  CGM.getTypes().arrangeBuiltinFunctionDeclaration(Ctx.VoidTy, Args);
+  llvm::FunctionType *FuncTy = CGM.getTypes().GetFunctionType(FI);
+  llvm::Function *Fn = llvm::Function::Create(
+  FuncTy, llvm::GlobalValue::LinkOnceODRLinkage, Name, &CGM.getModule());
+  Fn->setVisibility(llvm::GlobalValue::HiddenVisibility);
+  CGM.SetLLVMFunctionAttributes(nullptr, FI, Fn);
+  CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Fn);
+
+  // Attach 'noinline' at -Oz.
+  if (CGM.getCodeGenOpts().OptimizeSize == 2)
+Fn->addFnAttr(llvm::Attribute::NoInline);
+
+  auto NL = ApplyDebugLocation::CreateEmpty(*this);
+  IdentifierInfo *II = &Ctx.Idents.get(Name);
+  FunctionDecl *FD = FunctionDecl::Create(
+  Ctx, Ctx.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), 
II,
+  Ctx.VoidTy, nullptr, SC_PrivateExtern, false, false);
+
+  StartFunction(FD, Ctx.VoidTy, Fn, FI, Args);
+
+  // Create a scope with an artificial location for the body of this function.
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
+
+  CharUnits Offset;
+  Address BufAddr(Builder.CreateLoad(GetAddrOfLocalVar(&Params[0]), "buf"),
+  BufferAlignment);
+  Builder.CreateStore(Builder.getInt8(Layout.getSummaryByte()),
+  Builder.CreateConstByteGEP(BufAddr, Offset++, 
"summary"));
+  Builder.CreateStore(Builder.getInt8(Layout.getNumArgsByte()),
+  Builder.CreateConstByteGEP(BufAddr,

[PATCH] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315045: [CodeGen] Emit a helper function for 
__builtin_os_log_format to reduce (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D38606?vs=117955&id=117957#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38606

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGen/builtins.c
  cfe/trunk/test/CodeGenObjC/os_log.m

Index: cfe/trunk/test/CodeGen/builtins.c
===
--- cfe/trunk/test/CodeGen/builtins.c
+++ cfe/trunk/test/CodeGen/builtins.c
@@ -378,229 +378,385 @@
 #ifdef __x86_64__
 
 // CHECK-LABEL: define void @test_builtin_os_log
-// CHECK: (i8* [[BUF:%.*]], i32 [[I:%.*]], i8* [[DATA:%.*]])
+// CHECK: (i8* %[[BUF:.*]], i32 %[[I:.*]], i8* %[[DATA:.*]])
 void test_builtin_os_log(void *buf, int i, const char *data) {
   volatile int len;
-  // CHECK: store i8* [[BUF]], i8** [[BUF_ADDR:%.*]], align 8
-  // CHECK: store i32 [[I]], i32* [[I_ADDR:%.*]], align 4
-  // CHECK: store i8* [[DATA]], i8** [[DATA_ADDR:%.*]], align 8
+  // CHECK: %[[BUF_ADDR:.*]] = alloca i8*, align 8
+  // CHECK: %[[I_ADDR:.*]] = alloca i32, align 4
+  // CHECK: %[[DATA_ADDR:.*]] = alloca i8*, align 8
+  // CHECK: %[[LEN:.*]] = alloca i32, align 4
+  // CHECK: store i8* %[[BUF]], i8** %[[BUF_ADDR]], align 8
+  // CHECK: store i32 %[[I]], i32* %[[I_ADDR]], align 4
+  // CHECK: store i8* %[[DATA]], i8** %[[DATA_ADDR]], align 8
 
-  // CHECK: store volatile i32 34
+  // CHECK: store volatile i32 34, i32* %[[LEN]]
   len = __builtin_os_log_format_buffer_size("%d %{public}s %{private}.16P", i, data, data);
 
-  // CHECK: [[BUF2:%.*]] = load i8*, i8** [[BUF_ADDR]]
-  // CHECK: [[SUMMARY:%.*]] = getelementptr i8, i8* [[BUF2]], i64 0
-  // CHECK: store i8 3, i8* [[SUMMARY]]
-  // CHECK: [[NUM_ARGS:%.*]] = getelementptr i8, i8* [[BUF2]], i64 1
-  // CHECK: store i8 4, i8* [[NUM_ARGS]]
-  //
-  // CHECK: [[ARG1_DESC:%.*]] = getelementptr i8, i8* [[BUF2]], i64 2
-  // CHECK: store i8 0, i8* [[ARG1_DESC]]
-  // CHECK: [[ARG1_SIZE:%.*]] = getelementptr i8, i8* [[BUF2]], i64 3
-  // CHECK: store i8 4, i8* [[ARG1_SIZE]]
-  // CHECK: [[ARG1:%.*]] = getelementptr i8, i8* [[BUF2]], i64 4
-  // CHECK: [[ARG1_INT:%.*]] = bitcast i8* [[ARG1]] to i32*
-  // CHECK: [[I2:%.*]] = load i32, i32* [[I_ADDR]]
-  // CHECK: store i32 [[I2]], i32* [[ARG1_INT]]
-
-  // CHECK: [[ARG2_DESC:%.*]] = getelementptr i8, i8* [[BUF2]], i64 8
-  // CHECK: store i8 34, i8* [[ARG2_DESC]]
-  // CHECK: [[ARG2_SIZE:%.*]] = getelementptr i8, i8* [[BUF2]], i64 9
-  // CHECK: store i8 8, i8* [[ARG2_SIZE]]
-  // CHECK: [[ARG2:%.*]] = getelementptr i8, i8* [[BUF2]], i64 10
-  // CHECK: [[ARG2_PTR:%.*]] = bitcast i8* [[ARG2]] to i8**
-  // CHECK: [[DATA2:%.*]] = load i8*, i8** [[DATA_ADDR]]
-  // CHECK: store i8* [[DATA2]], i8** [[ARG2_PTR]]
-
-  // CHECK: [[ARG3_DESC:%.*]] = getelementptr i8, i8* [[BUF2]], i64 18
-  // CHECK: store i8 17, i8* [[ARG3_DESC]]
-  // CHECK: [[ARG3_SIZE:%.*]] = getelementptr i8, i8* [[BUF2]], i64 19
-  // CHECK: store i8 4, i8* [[ARG3_SIZE]]
-  // CHECK: [[ARG3:%.*]] = getelementptr i8, i8* [[BUF2]], i64 20
-  // CHECK: [[ARG3_INT:%.*]] = bitcast i8* [[ARG3]] to i32*
-  // CHECK: store i32 16, i32* [[ARG3_INT]]
-
-  // CHECK: [[ARG4_DESC:%.*]] = getelementptr i8, i8* [[BUF2]], i64 24
-  // CHECK: store i8 49, i8* [[ARG4_DESC]]
-  // CHECK: [[ARG4_SIZE:%.*]] = getelementptr i8, i8* [[BUF2]], i64 25
-  // CHECK: store i8 8, i8* [[ARG4_SIZE]]
-  // CHECK: [[ARG4:%.*]] = getelementptr i8, i8* [[BUF2]], i64 26
-  // CHECK: [[ARG4_PTR:%.*]] = bitcast i8* [[ARG4]] to i8**
-  // CHECK: [[DATA3:%.*]] = load i8*, i8** [[DATA_ADDR]]
-  // CHECK: store i8* [[DATA3]], i8** [[ARG4_PTR]]
-
+  // CHECK: %[[V1:.*]] = load i8*, i8** %[[BUF_ADDR]]
+  // CHECK: %[[V2:.*]] = load i32, i32* %[[I_ADDR]]
+  // CHECK: %[[V3:.*]] = load i8*, i8** %[[DATA_ADDR]]
+  // CHECK: %[[V4:.*]] = ptrtoint i8* %[[V3]] to i64
+  // CHECK: %[[V5:.*]] = load i8*, i8** %[[DATA_ADDR]]
+  // CHECK: %[[V6:.*]] = ptrtoint i8* %[[V5]] to i64
+  // CHECK: call void @__os_log_helper_1_3_4_4_0_8_34_4_17_8_49(i8* %[[V1]], i32 %[[V2]], i64 %[[V4]], i32 16, i64 %[[V6]])
   __builtin_os_log_format(buf, "%d %{public}s %{private}.16P", i, data, data);
 }
 
-// CHECK-LABEL: define void @test_builtin_os_log_errno
-// CHECK: (i8* [[BUF:%.*]], i8* [[DATA:%.*]])
-void test_builtin_os_log_errno(void *buf, const char *data) {
-  volatile int len;
-  // CHECK: store i8* [[BUF]], i8** [[BUF_ADDR:%.*]], align 8
-  // CHECK: store i8* [[DATA]], i8** [[DATA_ADDR:%.*]], align 8
-
-  // CHECK: store volatile i32 2
-  len = __builtin_os_log_format_buffer_size("%S");
-
-  // CHECK: [[BUF2:%.*]] = load i8*, i8** [[BUF_ADDR]]
-  // CHECK: [[SUMMARY:%.*]] = getelementptr i8, i8* [[BUF2]], i64 0
-  // CHECK: store i8 2, i8* [[SUMMARY]]
-  // CHECK: [[NUM_ARGS:%.*]] = getelementptr i8, i8* [[BUF2]

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

[Daniel Marjamäki via Phabricator via cfe-commits]

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



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107
+/** Recursively check if variable is changed in code. */
+static bool isChanged(const Stmt *S, const VarDecl *VD, bool Write) {
+  if (!S)

xazax.hun wrote:
> Usually, we do not like bug recursions since it might eat up the stack. 
> Also, do you consider the case when the variable is passed by reference to a 
> function in another translation unit? 
I rewrote the code to reuse the ast matchers in LoopUnroll.. and that is not 
recursive. I rewrote the getGlobalStaticVars (this code is longer and in my 
opinion less readable but it's not recursive).



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:169
+  SVal Constraint_untested =
+  evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());

xazax.hun wrote:
> Does this work for non-integer typed e.g. structs? 
Currently static struct objects are unaffected by these changes. There is no 
regression.

Reviews are taking too long time. I am not against getting reviews but I am 
against waiting for reviews for months, ideally people would only need to wait 
for at most a week to get a review. This is why I don't want to handle structs 
now -- I am not eager to prolong the review process for this patch.



Repository:
  rL LLVM

https://reviews.llvm.org/D37897



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

[PATCH] D20689: [clang-tidy] Suspicious Call Argument checker

[Barancsuk Lilla via Phabricator via cfe-commits]

barancsuk added a comment.

@alexfh, have you had a chance to look at the results yet?


https://reviews.llvm.org/D20689



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

r315046 - Fix check strings in test case and use llvm::to_string instead of

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Oct  6 00:47:47 2017
New Revision: 315046

URL: http://llvm.org/viewvc/llvm-project?rev=315046&view=rev
Log:
Fix check strings in test case and use llvm::to_string instead of
std::to_string.

These changes were needed to fix bots that started failing after
r315045.

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGenObjC/os_log.m

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=315046&r1=315045&r2=315046&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  6 00:47:47 2017
@@ -30,6 +30,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/ConvertUTF.h"
 #include 
 
@@ -698,9 +699,10 @@ llvm::Function *CodeGenFunction::generat
 if (!Size)
   continue;
 
-Params.emplace_back(Ctx, nullptr, SourceLocation(),
-&Ctx.Idents.get(std::string("arg") + 
std::to_string(I)),
-getOSLogArgType(Ctx, Size), ImplicitParamDecl::Other);
+Params.emplace_back(
+Ctx, nullptr, SourceLocation(),
+&Ctx.Idents.get(std::string("arg") + llvm::to_string(I)),
+getOSLogArgType(Ctx, Size), ImplicitParamDecl::Other);
   }
 
   FunctionArgList Args;

Modified: cfe/trunk/test/CodeGenObjC/os_log.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/os_log.m?rev=315046&r1=315045&r2=315046&view=diff
==
--- cfe/trunk/test/CodeGenObjC/os_log.m (original)
+++ cfe/trunk/test/CodeGenObjC/os_log.m Fri Oct  6 00:47:47 2017
@@ -19,10 +19,10 @@ extern __attribute__((visibility("defaul
 void *test_builtin_os_log(void *buf) {
   return __builtin_os_log_format(buf, "capabilities: %@", GenString());
 
-  // CHECK: %[[CALL:.*]] = tail call %[[V0:.*]]* (...) @GenString()
-  // CHECK: %[[V0]] = bitcast %[[V0]]* %[[CALL]] to i8*
+  // CHECK: %[[CALL:.*]] = tail call %[[TY0:.*]]* (...) @GenString()
+  // CHECK: %[[V0:.*]] = bitcast %[[TY0]]* %[[CALL]] to i8*
   // CHECK: %[[V1:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* 
%[[V0]])
-  // CHECK: %[[V2:.*]] = ptrtoint %[[V0]]* %[[CALL]] to i64
+  // CHECK: %[[V2:.*]] = ptrtoint %[[TY0]]* %[[CALL]] to i64
   // CHECK: store i8 2, i8* %[[BUF]], align 1
   // CHECK: %[[NUMARGS_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
   // CHECK: store i8 1, i8* %[[NUMARGS_I]], align 1
@@ -43,13 +43,13 @@ void *test_builtin_os_log(void *buf) {
   // CHECK-O0: %[[BUF_ADDR:.*]] = alloca i8*, align 8
   // CHECK-O0: store i8* %[[BUF]], i8** %[[BUF_ADDR]], align 8
   // CHECK-O0: %[[V0:.*]] = load i8*, i8** %[[BUF_ADDR]], align 8
-  // CHECK-O0: %[[CALL:.*]] = call %[[V0]]* (...) @GenString()
-  // CHECK-O0: %[[V1:.*]] = bitcast %[[V0]]* %[[CALL]] to i8*
+  // CHECK-O0: %[[CALL:.*]] = call %[[TY0:.*]]* (...) @GenString()
+  // CHECK-O0: %[[V1:.*]] = bitcast %[[TY0]]* %[[CALL]] to i8*
   // CHECK-O0: %[[V2:.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
%[[V1]])
   // CHECK-O0: %[[V3:.*]] = bitcast i8* %[[V2]] to %[[V0]]*
-  // CHECK-O0: %[[V4:.*]] = ptrtoint %[[V0]]* %[[V3]] to i64
+  // CHECK-O0: %[[V4:.*]] = ptrtoint %[[TY0]]* %[[V3]] to i64
   // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V4]])
-  // CHECK-O0: %[[V5:.*]] = bitcast %[[V0]]* %[[V3]] to i8*
+  // CHECK-O0: %[[V5:.*]] = bitcast %[[TY0]]* %[[V3]] to i8*
   // CHECK-O0-NOT call void (...) @clang.arc.use({{.*}}
   // CHECK-O0: call void @objc_release(i8* %[[V5]])
   // CHECK-O0: ret i8* %[[V0]]


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

r315047 - Fix one more check string after r315045.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Oct  6 01:05:34 2017
New Revision: 315047

URL: http://llvm.org/viewvc/llvm-project?rev=315047&view=rev
Log:
Fix one more check string after r315045.

Modified:
cfe/trunk/test/CodeGenObjC/os_log.m

Modified: cfe/trunk/test/CodeGenObjC/os_log.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/os_log.m?rev=315047&r1=315046&r2=315047&view=diff
==
--- cfe/trunk/test/CodeGenObjC/os_log.m (original)
+++ cfe/trunk/test/CodeGenObjC/os_log.m Fri Oct  6 01:05:34 2017
@@ -33,7 +33,7 @@ void *test_builtin_os_log(void *buf) {
   // CHECK: %[[ARGDATA_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 4
   // CHECK: %[[ARGDATACAST_I:.*]] = bitcast i8* %[[ARGDATA_I]] to i64*
   // CHECK: store i64 %[[V2]], i64* %[[ARGDATACAST_I]], align 1
-  // CHECK: tail call void (...) @clang.arc.use(%[[V0]]* %[[CALL]])
+  // CHECK: tail call void (...) @clang.arc.use(%[[TY0]]* %[[CALL]])
   // CHECK: tail call void @objc_release(i8* %[[V0]])
   // CHECK: ret i8* %[[BUF]]
 


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

r315048 - Refine generation of TBAA information in clang

[Ivan A. Kosarev via cfe-commits]

Author: kosarev
Date: Fri Oct  6 01:17:48 2017
New Revision: 315048

URL: http://llvm.org/viewvc/llvm-project?rev=315048&view=rev
Log:
Refine generation of TBAA information in clang

This patch is an attempt to clarify and simplify generation and
propagation of TBAA information. The idea is to pack all values
that describe a memory access, namely, base type, access type and
offset, into a single structure. This is supposed to make further
changes, such as adding support for unions and array members,
easier to prepare and review.

DecorateInstructionWithTBAA() is no more responsible for
converting types to tags. These implicit conversions not only
complicate reading the code, but also suggest assigning scalar
access tags while we generally prefer full-size struct-path tags.

TBAAPathTag is replaced with TBAAAccessInfo; the latter is now
the type of the keys of the cache map that translates access
descriptors to metadata nodes.

Fixed a bug with writing to a wrong map in
getTBAABaseTypeMetadata() (former getTBAAStructTypeInfo()).

We now check for valid base access types every time we
dereference a field. The original code only checks the top-level
base type. See isValidBaseType() / isTBAAPathStruct() calls.

Some entities have been renamed to sound more adequate and less
confusing/misleading in presence of path-aware TBAA information.

Now we do not lookup twice for the same cache entry in
getAccessTagInfo().

Refined relevant comments and descriptions.

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

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
cfe/trunk/lib/CodeGen/CodeGenTBAA.h
cfe/trunk/test/CodeGen/tbaa-for-vptr.cpp

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=315048&r1=315047&r2=315048&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Fri Oct  6 01:17:48 2017
@@ -205,7 +205,7 @@ namespace {
 addr = CGF.Builder.CreateStructGEP(addr, 0, CharUnits());
 
   return LValue::MakeAddr(addr, getValueType(), CGF.getContext(),
-  LVal.getBaseInfo(), LVal.getTBAAAccessType());
+  LVal.getBaseInfo(), LVal.getTBAAInfo());
 }
 
 /// \brief Emits atomic load.
@@ -1425,8 +1425,7 @@ llvm::Value *AtomicInfo::EmitAtomicLoadO
   // Other decoration.
   if (IsVolatile)
 Load->setVolatile(true);
-  TBAAAccessInfo TBAAInfo(LVal.getTBAAAccessType());
-  CGF.CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
+  CGF.CGM.DecorateInstructionWithTBAA(Load, LVal.getTBAAInfo());
   return Load;
 }
 
@@ -1942,8 +1941,7 @@ void CodeGenFunction::EmitAtomicStore(RV
 // Other decoration.
 if (IsVolatile)
   store->setVolatile(true);
-TBAAAccessInfo TBAAInfo(dest.getTBAAAccessType());
-CGM.DecorateInstructionWithTBAA(store, TBAAInfo);
+CGM.DecorateInstructionWithTBAA(store, dest.getTBAAInfo());
 return;
   }
 

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315048&r1=315047&r2=315048&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct  6 01:17:48 2017
@@ -1174,8 +1174,7 @@ LValue CodeGenFunction::EmitLValue(const
   llvm::Value *V = LV.getPointer();
   Scope.ForceCleanup({&V});
   return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
-  getContext(), LV.getBaseInfo(),
-  LV.getTBAAAccessType());
+  getContext(), LV.getBaseInfo(), 
LV.getTBAAInfo());
 }
 // FIXME: Is it possible to create an ExprWithCleanups that produces a
 // bitfield lvalue or some other non-simple lvalue?
@@ -1514,7 +1513,7 @@ llvm::Value *CodeGenFunction::EmitLoadOf
 
   // Atomic operations have to be done on integral types.
   LValue AtomicLValue =
-  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
+  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
   if (Ty->isAtomicType() || LValueIsSuitableForInlineAtomic(AtomicLValue)) {
 return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
@@ -1525,11 +1524,10 @@ llvm::Value *CodeGenFunction::EmitLoadOf
 Load->getContext(), 
llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
 Load->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
-  if (TBAAInfo.AccessType) {
-if (BaseInfo.getMayAlias())
-  T

[PATCH] D37826: Refine generation of TBAA information in clang

[Ivan A. Kosarev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315048: Refine generation of TBAA information in clang 
(authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D37826?vs=117823&id=117961#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37826

Files:
  cfe/trunk/lib/CodeGen/CGAtomic.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGValue.h
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
  cfe/trunk/lib/CodeGen/CodeGenTBAA.h
  cfe/trunk/test/CodeGen/tbaa-for-vptr.cpp

Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -594,6 +594,12 @@
   return TBAA->getTBAAStructInfo(QTy);
 }
 
+llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
+  if (!TBAA)
+return nullptr;
+  return TBAA->getBaseTypeInfo(QTy);
+}
+
 llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
   if (!TBAA)
 return nullptr;
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -1174,8 +1174,7 @@
   llvm::Value *V = LV.getPointer();
   Scope.ForceCleanup({&V});
   return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
-  getContext(), LV.getBaseInfo(),
-  LV.getTBAAAccessType());
+  getContext(), LV.getBaseInfo(), LV.getTBAAInfo());
 }
 // FIXME: Is it possible to create an ExprWithCleanups that produces a
 // bitfield lvalue or some other non-simple lvalue?
@@ -1514,7 +1513,7 @@
 
   // Atomic operations have to be done on integral types.
   LValue AtomicLValue =
-  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
+  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
   if (Ty->isAtomicType() || LValueIsSuitableForInlineAtomic(AtomicLValue)) {
 return EmitAtomicLoad(AtomicLValue, Loc).getScalarVal();
   }
@@ -1525,11 +1524,10 @@
 Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
 Load->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
-  if (TBAAInfo.AccessType) {
-if (BaseInfo.getMayAlias())
-  TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
-CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
-  }
+
+  if (BaseInfo.getMayAlias())
+TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
 
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
@@ -1596,7 +1594,7 @@
   Value = EmitToMemory(Value, Ty);
 
   LValue AtomicLValue =
-  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo.AccessType);
+  LValue::MakeAddr(Addr, Ty, getContext(), BaseInfo, TBAAInfo);
   if (Ty->isAtomicType() ||
   (!isInit && LValueIsSuitableForInlineAtomic(AtomicLValue))) {
 EmitAtomicStore(RValue::get(Value), AtomicLValue, isInit);
@@ -1610,11 +1608,10 @@
   llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
 Store->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
-  if (TBAAInfo.AccessType) {
-if (BaseInfo.getMayAlias())
-  TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
-CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
-  }
+
+  if (BaseInfo.getMayAlias())
+TBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
 }
 
 void CodeGenFunction::EmitStoreOfScalar(llvm::Value *value, LValue lvalue,
@@ -3753,30 +3750,40 @@
 
   LValue LV = MakeAddrLValue(addr, type, FieldBaseInfo);
   LV.getQuals().addCVRQualifiers(cvr);
-  if (TBAAPath) {
+
+  // Fields of may_alias structs act like 'char' for TBAA purposes.
+  // FIXME: this should get propagated down through anonymous structs
+  // and unions.
+  if (mayAlias) {
+LV.setTBAAInfo(CGM.getTBAAMayAliasAccessInfo());
+  } else if (TBAAPath) {
+// If no base type been assigned for the base access, then try to generate
+// one for this base lvalue.
+TBAAAccessInfo TBAAInfo = base.getTBAAInfo();
+if (!TBAAInfo.BaseType) {
+TBAAInfo.BaseType = CGM.getTBAABaseTypeInfo(base.getType());
+assert(!TBAAInfo.Offset &&
+   "Nonzero offset for an access with no base type!");
+}
+
+// Adjust offset to be relative to the base type.
 const ASTRecordLayout &Layout =
 getContext().getASTRecordLayout(field->getParent());
-// Set the base type to be the base type of the base LValue and
-// update offset to be relative to the base type.
 unsigned Cha

r315049 - Fix one more check string after r315045.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Oct  6 01:19:31 2017
New Revision: 315049

URL: http://llvm.org/viewvc/llvm-project?rev=315049&view=rev
Log:
Fix one more check string after r315045.

Modified:
cfe/trunk/test/CodeGenObjC/os_log.m

Modified: cfe/trunk/test/CodeGenObjC/os_log.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/os_log.m?rev=315049&r1=315048&r2=315049&view=diff
==
--- cfe/trunk/test/CodeGenObjC/os_log.m (original)
+++ cfe/trunk/test/CodeGenObjC/os_log.m Fri Oct  6 01:19:31 2017
@@ -46,7 +46,7 @@ void *test_builtin_os_log(void *buf) {
   // CHECK-O0: %[[CALL:.*]] = call %[[TY0:.*]]* (...) @GenString()
   // CHECK-O0: %[[V1:.*]] = bitcast %[[TY0]]* %[[CALL]] to i8*
   // CHECK-O0: %[[V2:.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
%[[V1]])
-  // CHECK-O0: %[[V3:.*]] = bitcast i8* %[[V2]] to %[[V0]]*
+  // CHECK-O0: %[[V3:.*]] = bitcast i8* %[[V2]] to %[[TY0]]*
   // CHECK-O0: %[[V4:.*]] = ptrtoint %[[TY0]]* %[[V3]] to i64
   // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V4]])
   // CHECK-O0: %[[V5:.*]] = bitcast %[[TY0]]* %[[V3]] to i8*


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

Re: [clang-tools-extra] r314989 - [clangd] Added async API to run code completion.

[Ilya Biryukov via cfe-commits]

Hi Douglas,

Fixed in r315028.
Sorry for the inconvenience.

On Thu, Oct 5, 2017 at 11:55 PM, Yung, Douglas 
wrote:

> Hi Ilya,
>
> Your change has broken the build on the PS4 Windows bot.
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_
> 64-scei-ps4-windows10pro-fast/builds/12525
>
> Can you take a look and fix it?
>
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of
> > Ilya Biryukov via cfe-commits
> > Sent: Thursday, October 05, 2017 10:04
> > To: cfe-commits@lists.llvm.org
> > Subject: [clang-tools-extra] r314989 - [clangd] Added async API to run
> code
> > completion.
> >
> > Author: ibiryukov
> > Date: Thu Oct  5 10:04:13 2017
> > New Revision: 314989
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=314989&view=rev
> > Log:
> > [clangd] Added async API to run code completion.
> >
> > Summary:
> > ClangdServer now provides async code completion API.
> > It is still used synchronously by ClangdLSPServer, more work is needed to
> > allow processing other requests in parallel while completion (or any
> other
> > request) is running.
> >
> > Reviewers: klimek, bkramer, krasimir
> >
> > Reviewed By: klimek
> >
> > Subscribers: cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D38583
> >
> > Modified:
> > clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> > clang-tools-extra/trunk/clangd/ClangdServer.cpp
> > clang-tools-extra/trunk/clangd/ClangdServer.h
> > clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
> >
> > Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> > extra/trunk/clangd/ClangdLSPServer.cpp?rev=314989&r1=314988&r2=314989&
> view=dif
> > f
> > 
> ==
> > --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
> > +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct  5
> > +++ 10:04:13 2017
> > @@ -149,6 +149,9 @@ void ClangdLSPServer::onCompletion(TextD
> > .codeComplete(Params.textDocument.uri.file,
> >   Position{Params.position.line,
> >Params.position.character})
> > +   .get() // FIXME(ibiryukov): This could be made async
> if we
> > +  // had an API that would allow to attach
> callbacks
> > to
> > +  // futures returned by ClangdServer.
> > .Value;
> >
> >std::string Completions;
> >
> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> > extra/trunk/clangd/ClangdServer.cpp?rev=314989&
> r1=314988&r2=314989&view=diff
> > 
> ==
> > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct  5 10:04:13
> > +++ 2017
> > @@ -194,18 +194,19 @@ std::future ClangdServer::forceRep
> >   std::move(TaggedFS));  }
> >
> > -Tagged>
> > +std::future>>
> >  ClangdServer::codeComplete(PathRef File, Position Pos,
> > llvm::Optional OverridenContents,
> > IntrusiveRefCntPtr
> *UsedFS) {
> > -  std::string DraftStorage;
> > -  if (!OverridenContents) {
> > +  std::string Contents;
> > +  if (OverridenContents) {
> > +Contents = *OverridenContents;
> > +  } else {
> >  auto FileContents = DraftMgr.getDraft(File);
> >  assert(FileContents.Draft &&
> > "codeComplete is called for non-added document");
> >
> > -DraftStorage = std::move(*FileContents.Draft);
> > -OverridenContents = DraftStorage;
> > +Contents = std::move(*FileContents.Draft);
> >}
> >
> >auto TaggedFS = FSProvider.getTaggedFileSystem(File);
> > @@ -215,12 +216,36 @@ ClangdServer::codeComplete(PathRef File,
> >std::shared_ptr Resources = Units.getFile(File);
> >assert(Resources && "Calling completion on non-added file");
> >
> > -  auto Preamble = Resources->getPossiblyStalePreamble();
> > -  std::vector Result = clangd::codeComplete(
> > -  File, Resources->getCompileCommand(),
> > -  Preamble ? &Preamble->Preamble : nullptr, *OverridenContents, Pos,
> > -  TaggedFS.Value, PCHs, SnippetCompletions, Logger);
> > -  return make_tagged(std::move(Result), TaggedFS.Tag);
> > +  using PackagedTask =
> > +  std::packaged_task>()>;
> > +
> > +  // Remember the current Preamble and use it when async task starts
> > executing.
> > +  // At the point when async task starts executing, we may have a
> > + different  // Preamble in Resources. However, we assume the Preamble
> > + that we obtain here  // is reusable in completion more often.
> > +  std::shared_ptr Preamble =
> > +  Resources->getPossiblyStalePreamble

[PATCH] D31538: [analyzer] MisusedMovedObjectChecker: Fix a false positive on state-resetting a base-class sub-object.

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

Hello Artem!

Could you please commit these changes? (And https://reviews.llvm.org/D31541 as 
well.) Thanks in advance!


https://reviews.llvm.org/D31538



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

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM. Only a few code style-related comments.




Comment at: include/clang/Lex/Preprocessor.h:102
+bool ReachedEOFWhileSkipping;
+SourceLocation HashToken;
+SourceLocation IfTokenLoc;

Maybe rename to `HashTokenLoc` for consistencty with `IfTokenLoc`?



Comment at: include/clang/Lex/Preprocessor.h:333
 
+PreambleSkipInfo SkipInfo;
+

Maybe move closer to other fields to avoid mixing data and functions?



Comment at: include/clang/Lex/Preprocessor.h:1859
   /// the caller can lex the first valid token.
-  void SkipExcludedConditionalBlock(const Token &HashToken,
+  void SkipExcludedConditionalBlock(SourceLocation HashToken,
 SourceLocation IfTokenLoc,

Maybe rename to `HashTokenLoc` for consistencty with `IfTokenLoc`?



Comment at: lib/Lex/PPDirectives.cpp:353
 /// the caller can lex the first valid token.
-void Preprocessor::SkipExcludedConditionalBlock(const Token &HashToken,
+void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashToken,
 SourceLocation IfTokenLoc,

Maybe rename to `HashTokenLoc` for consistencty with `IfTokenLoc`?


https://reviews.llvm.org/D38578



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

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Also, one more important thing. I'll unaccept revision for now, before getting 
feedback on this one.




Comment at: include/clang/Lex/Preprocessor.h:333
 
+PreambleSkipInfo SkipInfo;
+

ilya-biryukov wrote:
> Maybe move closer to other fields to avoid mixing data and functions?
Maybe initialize `SkipInfo` with default values? Or even better: forbid default 
constructor and put it into `llvm::Optional`?

It's a public member, so I guess there's a huge potential some code will 
eventually get into undefined behavior when using it.


https://reviews.llvm.org/D38578



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

[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv created this revision.

All attributes have a source range associated with it. However, implicit
attributes are added by the compiler, and not added because the user
wrote something in the input. So no token type should be set to
CXCursor_*Attr.

The problem was visible when a class gets marked by e.g.
MSInheritanceAttr, which has the full CXXRecordDecl's range as its
own range. The effect of marking that range as CXCursor_UnexposedAttr
was that all cursors for the record decl, including all child decls,
would become CXCursor_UnexposedAttr.


https://reviews.llvm.org/D38615

Files:
  test/Index/annotate-tokens-unexposed.cpp
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1772,7 +1772,7 @@
 
 bool CursorVisitor::VisitAttributes(Decl *D) {
   for (const auto *I : D->attrs())
-if (Visit(MakeCXCursor(I, D, TU)))
+if (!I->isImplicit() && Visit(MakeCXCursor(I, D, TU)))
 return true;
 
   return false;
Index: test/Index/annotate-tokens-unexposed.cpp
===
--- /dev/null
+++ test/Index/annotate-tokens-unexposed.cpp
@@ -0,0 +1,20 @@
+// RUN: c-index-test -test-annotate-tokens=%s:1:1:16:1 %s -target 
x86_64-pc-windows-msvc | FileCheck %s
+class Foo
+{
+public:
+void step(int v);
+Foo();
+};
+
+void bar()
+{
+// Introduce a MSInheritanceAttr node on the CXXRecordDecl for Foo. The
+// existance of this attribute should not mark all cursors for tokens in
+// Foo as UnexposedAttr.
+&Foo::step;
+}
+
+Foo::Foo()
+{}
+
+// CHECK-NOT: UnexposedAttr=


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1772,7 +1772,7 @@
 
 bool CursorVisitor::VisitAttributes(Decl *D) {
   for (const auto *I : D->attrs())
-if (Visit(MakeCXCursor(I, D, TU)))
+if (!I->isImplicit() && Visit(MakeCXCursor(I, D, TU)))
 return true;
 
   return false;
Index: test/Index/annotate-tokens-unexposed.cpp
===
--- /dev/null
+++ test/Index/annotate-tokens-unexposed.cpp
@@ -0,0 +1,20 @@
+// RUN: c-index-test -test-annotate-tokens=%s:1:1:16:1 %s -target x86_64-pc-windows-msvc | FileCheck %s
+class Foo
+{
+public:
+void step(int v);
+Foo();
+};
+
+void bar()
+{
+// Introduce a MSInheritanceAttr node on the CXXRecordDecl for Foo. The
+// existance of this attribute should not mark all cursors for tokens in
+// Foo as UnexposedAttr.
+&Foo::step;
+}
+
+Foo::Foo()
+{}
+
+// CHECK-NOT: UnexposedAttr=
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv updated this revision to Diff 117975.
erikjv added a comment.

Added more context to the diff


https://reviews.llvm.org/D38615

Files:
  test/Index/annotate-tokens-unexposed.cpp
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1772,7 +1772,7 @@
 
 bool CursorVisitor::VisitAttributes(Decl *D) {
   for (const auto *I : D->attrs())
-if (Visit(MakeCXCursor(I, D, TU)))
+if (!I->isImplicit() && Visit(MakeCXCursor(I, D, TU)))
 return true;
 
   return false;
Index: test/Index/annotate-tokens-unexposed.cpp
===
--- /dev/null
+++ test/Index/annotate-tokens-unexposed.cpp
@@ -0,0 +1,20 @@
+// RUN: c-index-test -test-annotate-tokens=%s:1:1:16:1 %s -target 
x86_64-pc-windows-msvc | FileCheck %s
+class Foo
+{
+public:
+void step(int v);
+Foo();
+};
+
+void bar()
+{
+// Introduce a MSInheritanceAttr node on the CXXRecordDecl for Foo. The
+// existance of this attribute should not mark all cursors for tokens in
+// Foo as UnexposedAttr.
+&Foo::step;
+}
+
+Foo::Foo()
+{}
+
+// CHECK-NOT: UnexposedAttr=


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1772,7 +1772,7 @@
 
 bool CursorVisitor::VisitAttributes(Decl *D) {
   for (const auto *I : D->attrs())
-if (Visit(MakeCXCursor(I, D, TU)))
+if (!I->isImplicit() && Visit(MakeCXCursor(I, D, TU)))
 return true;
 
   return false;
Index: test/Index/annotate-tokens-unexposed.cpp
===
--- /dev/null
+++ test/Index/annotate-tokens-unexposed.cpp
@@ -0,0 +1,20 @@
+// RUN: c-index-test -test-annotate-tokens=%s:1:1:16:1 %s -target x86_64-pc-windows-msvc | FileCheck %s
+class Foo
+{
+public:
+void step(int v);
+Foo();
+};
+
+void bar()
+{
+// Introduce a MSInheritanceAttr node on the CXXRecordDecl for Foo. The
+// existance of this attribute should not mark all cursors for tokens in
+// Foo as UnexposedAttr.
+&Foo::step;
+}
+
+Foo::Foo()
+{}
+
+// CHECK-NOT: UnexposedAttr=
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38617: Set PreprocessorOpts.GeneratePreamble=true in PrecompiledPreamble.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

It was previsouly set only in ASTUnit, but it should be set for all client of
PrecompiledPreamble.


https://reviews.llvm.org/D38617

Files:
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -234,6 +234,8 @@
   FrontendOpts.OutputFile = PreamblePCHFile->getFilePath();
   PreprocessorOpts.PrecompiledPreambleBytes.first = 0;
   PreprocessorOpts.PrecompiledPreambleBytes.second = false;
+  // Inform preprocessor to record conditional stack when building the 
preamble.
+  PreprocessorOpts.GeneratePreamble = true;
 
   // Create the compiler instance to use for building the precompiled preamble.
   std::unique_ptr Clang(
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1698,7 +1698,6 @@
   PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
   PPOpts.RemappedFilesKeepOriginalName = RemappedFilesKeepOriginalName;
   PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
-  PPOpts.GeneratePreamble = PrecompilePreambleAfterNParses != 0;
   PPOpts.SingleFileParseMode = SingleFileParse;
   
   // Override the resources path.


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -234,6 +234,8 @@
   FrontendOpts.OutputFile = PreamblePCHFile->getFilePath();
   PreprocessorOpts.PrecompiledPreambleBytes.first = 0;
   PreprocessorOpts.PrecompiledPreambleBytes.second = false;
+  // Inform preprocessor to record conditional stack when building the preamble.
+  PreprocessorOpts.GeneratePreamble = true;
 
   // Create the compiler instance to use for building the precompiled preamble.
   std::unique_ptr Clang(
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1698,7 +1698,6 @@
   PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
   PPOpts.RemappedFilesKeepOriginalName = RemappedFilesKeepOriginalName;
   PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
-  PPOpts.GeneratePreamble = PrecompilePreambleAfterNParses != 0;
   PPOpts.SingleFileParseMode = SingleFileParse;
   
   // Override the resources path.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38617: Set PreprocessorOpts.GeneratePreamble=true in PrecompiledPreamble.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This commit will unbreak clangd from all the same issues that were fixed by 
conditional stack commits.


https://reviews.llvm.org/D38617



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

[PATCH] D38134: [OpenCL] Emit enqueued block as kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:144
+  if (auto *I = dyn_cast(V)) {
+// If the block literal is emitted as an instruction, it is an alloca
+// and the block invoke function is stored to GEP of this alloca.

Why do we need to replace original block calls with the kernels? I think in 
case of calling a block we could use the original block function and only for 
enqueue use the kernel that would call the block function inside. The pointer 
to the kernel wrapper could be passed as an additional parameter to 
`enqueue_kernel` calls. We won't need to iterate through all IR then.



Comment at: lib/CodeGen/TargetInfo.cpp:8927
+llvm::Function *
+TargetCodeGenInfo::createEnqueuedBlockKernel(CodeGenFunction &CGF,
+ llvm::Function *Invoke,

Could you add some comments please?



Comment at: lib/CodeGen/TargetInfo.cpp:8949
+  Builder.restoreIP(IP);
+  return F;
+}

Wondering if we should add the kernel metadata (w/o args) since it was used for 
long time to indicate the kernel.



Comment at: lib/CodeGen/TargetInfo.h:35
 class Decl;
+class ASTContext;
 

Do we need this?



Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:9
 
-// N.B. The check here only exists to set BL_GLOBAL
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) 
addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ 
i32, i32, i8 addrspace(4)* } addrspace(1)* 
[[BL_GLOBAL:@__block_literal_global(\.[0-9]+)?]] to void (i8 addrspace(3)*) 
addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: %struct.__opencl_block_literal_generic = type { i32, i32, i8 
addrspace(4)* }
+

Can we check generated kernel function too?


https://reviews.llvm.org/D38134



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

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.

With enabled CINDEXTEST_CODE_COMPLETE_PATTERNS env option (which enables 
IncludeCodePatterns in completion options) code completion after colon 
currently suggests access modifiers with 2 completion chunks which is incorrect.

Example:
class A : B
{
}

Currently we get 'NotImplemented:{TypedText public}{Colon :} (40)'
but the correct line is just 'NotImplemented:{TypedText public} (40)'

The fix introduces more specific scope that occurs between ':' and '{'
It allows us to determine when we don't need to add ':' as a second chunk to 
the public/protected/private access modifiers.


https://reviews.llvm.org/D38618

Files:
  include/clang/Sema/Scope.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-super.cpp


Index: test/Index/complete-super.cpp
===
--- test/Index/complete-super.cpp
+++ test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test 
-code-completion-at=%s:10:12 %s | FileCheck 
-check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1647,21 +1647,22 @@
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope = !(S->getFlags() & 
Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // protected:
 Builder.AddTypedTextChunk("protected");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // private:
 Builder.AddTypedTextChunk("private");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
   }
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3193,6 +3193,8 @@
   }
 
   if (Tok.is(tok::colon)) {
+ParseScope ClassScope(this, Scope::ClassScope|Scope::DeclScope | 
Scope::ClassInheritanceScope);
+
 ParseBaseClause(TagDecl);
 if (!Tok.is(tok::l_brace)) {
   bool SuggestFixIt = false;
Index: include/clang/Sema/Scope.h
===
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -124,6 +124,9 @@
 
 /// We are currently in the filter expression of an SEH except block.
 SEHFilterScope = 0x20,
+
+/// We are between inheritance colon and the real class/struct definition 
scope
+ClassInheritanceScope = 0x40,
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit


Index: test/Index/complete-super.cpp
===
--- test/Index/complete-super.cpp
+++ test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:10:12 %s | FileCheck -check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1647,21 +1647,22 @@
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope = !(S->getFlags() & Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-  

[clang-tools-extra] r315055 - [clangd] Add textDocument/signatureHelp

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Oct  6 04:54:17 2017
New Revision: 315055

URL: http://llvm.org/viewvc/llvm-project?rev=315055&view=rev
Log:
[clangd] Add textDocument/signatureHelp

Summary:
Makes clangd respond to a client's "textDocument/signatureHelp" request by
presenting function/method overloads.

Patch by Raoul Wols.

Reviewers: bkramer, ilya-biryukov, krasimir

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Added:
clang-tools-extra/trunk/test/clangd/signature-help.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
clang-tools-extra/trunk/test/clangd/formatting.test
clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
clang-tools-extra/trunk/test/clangd/initialize-params.test

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=315055&r1=315054&r2=315055&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Oct  6 04:54:17 2017
@@ -48,6 +48,7 @@ void ClangdLSPServer::onInitialize(Strin
   "documentOnTypeFormattingProvider": 
{"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
   "codeActionProvider": true,
   "completionProvider": {"resolveProvider": false, 
"triggerCharacters": [".",">",":"]},
+  "signatureHelpProvider": {"triggerCharacters": ["(",","]},
   "definitionProvider": true
 }}})");
   if (IP.rootUri && !IP.rootUri->file.empty())
@@ -166,6 +167,18 @@ void ClangdLSPServer::onCompletion(TextD
   R"(,"result":[)" + Completions + R"(]})");
 }
 
+void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams Params,
+  StringRef ID, JSONOutput &Out) {
+  const auto SigHelp = SignatureHelp::unparse(
+  Server
+  .signatureHelp(
+  Params.textDocument.uri.file,
+  Position{Params.position.line, Params.position.character})
+  .Value);
+  Out.writeMessage(R"({"jsonrpc":"2.0","id":)" + ID.str() + R"(,"result":)" +
+   SigHelp + "}");
+}
+
 void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams Params,
StringRef ID, JSONOutput &Out) {
 

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=315055&r1=315054&r2=315055&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Oct  6 04:54:17 2017
@@ -67,6 +67,8 @@ private:
 JSONOutput &Out) override;
   void onCompletion(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) override;
+  void onSignatureHelp(TextDocumentPositionParams Params, StringRef ID,
+   JSONOutput &Out) override;
   void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) override;
   void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID,

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=315055&r1=315054&r2=315055&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Oct  6 04:54:17 2017
@@ -248,6 +248,35 @@ ClangdServer::codeComplete(PathRef File,
   return Future;
 }
 
+Tagged
+ClangdServer::signatureHelp(PathRef File, Position Pos,
+llvm::Optional OverridenContents,
+IntrusiveRefCntPtr *UsedFS) {
+  std::string DraftStorage;
+  if (!OverridenContents) {
+auto FileContents = DraftMgr.getDraft(File);
+assert(FileContents.Draft &&
+   "signatureHelp is called for non-added document");
+
+DraftStorage = std::move(*FileContents.Draft);
+OverridenContents = DraftStorage;
+  }
+
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  if (UsedFS)
+*UsedFS = TaggedFS.Value;
+
+  std::shared_ptr Resources = Units.getFile(File);
+  assert(Res

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315055: [clangd] Add textDocument/signatureHelp (authored by 
ibiryukov).

Changed prior to commit:
  https://reviews.llvm.org/D38048?vs=117580&id=117983#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38048

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
  clang-tools-extra/trunk/clangd/ProtocolHandlers.h
  clang-tools-extra/trunk/test/clangd/formatting.test
  clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
  clang-tools-extra/trunk/test/clangd/initialize-params.test
  clang-tools-extra/trunk/test/clangd/signature-help.test

Index: clang-tools-extra/trunk/test/clangd/signature-help.test
===
--- clang-tools-extra/trunk/test/clangd/signature-help.test
+++ clang-tools-extra/trunk/test/clangd/signature-help.test
@@ -0,0 +1,42 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+
+# Start a session.
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+# Modify the document.
+Content-Length: 333
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"void foo(int x, int y);\nvoid foo(int x, float y);\nvoid foo(float x, int y);\nvoid foo(float x, float y);\nvoid bar(int x, int y = 0);\nvoid bar(float x = 0, int y = 42);\nint main() { foo("}}}
+
+# Ask for signature help.
+Content-Length: 151
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":8,"character":9}}}
+# CHECK: {"jsonrpc":"2.0","id":1,"result":{"activeSignature":0,"activeParameter":0,"signatures":[
+# CHECK-DAG: {"label":"foo(float x, float y) -> void","parameters":[{"label":"float x"},{"label":"float y"}]}
+# CHECK-DAG: {"label":"foo(float x, int y) -> void","parameters":[{"label":"float x"},{"label":"int y"}]}
+# CHECK-DAG: {"label":"foo(int x, float y) -> void","parameters":[{"label":"int x"},{"label":"float y"}]}
+# CHECK-DAG: {"label":"foo(int x, int y) -> void","parameters":[{"label":"int x"},{"label":"int y"}]}
+# CHECK: ]}
+
+# Modify the document
+Content-Length: 333
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":2,"text":"void foo(int x, int y);\nvoid foo(int x, float y);\nvoid foo(float x, int y);\nvoid foo(float x, float y);\nvoid bar(int x, int y = 0);\nvoid bar(float x = 0, int y = 42);\nint main() { bar("}}}
+
+# Ask for signature help (this checks default argument handling).
+Content-Length: 151
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":8,"character":9}}}
+# CHECK: {"jsonrpc":"2.0","id":2,"result":{"activeSignature":0,"activeParameter":0,"signatures":[
+# CHECK-DAG: {"label":"bar(int x, int y = 0) -> void","parameters":[{"label":"int x"},{"label":"int y = 0"}]}
+# CHECK-DAG: {"label":"bar(float x = 0, int y = 42) -> void","parameters":[{"label":"float x = 0"},{"label":"int y = 42"}]}
+# CHECK: ]}
+
+# Shutdown.
+Content-Length: 49
+
+{"jsonrpc":"2.0","id":10,"method":"shutdown"}
Index: clang-tools-extra/trunk/test/clangd/initialize-params.test
===
--- clang-tools-extra/trunk/test/clangd/initialize-params.test
+++ clang-tools-extra/trunk/test/clangd/initialize-params.test
@@ -5,14 +5,15 @@
 Content-Length: 143
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 466
+# CHECK: Content-Length: 535
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
 # CHECK:   "documentRangeFormattingProvider": true,
 # CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
 # CHECK:   "codeActionProvider": true,
 # CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
+# CHECK:   "signatureHelpProvider": {"triggerCharacters": ["(",","]},
 # CHECK:   "definitionProvider": true
 # CHECK: }}}
 #
Index: clang-tools-extra/trunk/test/clangd/formatting.test
==

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D35082#890162, @rjmccall wrote:

> Okay.  I think I see your point about why it would be better to have a 
> canonical __private address space that is different from the implicit address 
> space 0.  I assume this means that there should basically never be pointers, 
> references, or l-values in address space 0 in OpenCL.


If you mean 0 is `private` address space then no. There can be private AS 
pointers. But if you mean 0 is no address space then this doesn't exist in 
OpenCL. I think the right design in the first place  would have been to keep 
address spaces in AST just as they are in the source code and add explicit 
address spaces to all types in IR instead. In this case absence of any address 
spaces in AST would signal implicit AS to be used. This would make some Sema 
checks a bit more complicated though, but the design would become a lot 
cleaner. Unfortunately I think it would not be worth doing this big change now.

> You will lose a significant amount of representational efficiency by doing 
> this, but it's probably not overwhelming.
> 
> I know you aren't implementing OpenCL C++ yet, so most of the cases where 
> temporaries are introduced aren't meaningful to you, but you will at least 
> need to consider compound literals.  I suspect the right rule is that 
> file-scope literals should be inferred as being in __global or __constant 
> memory, depending on whether they're const, and local-scope literals should 
> be inferred as __private.
> 
> I'll try to review your patch tomorrow.


https://reviews.llvm.org/D35082



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

[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@francisco.lopes, I've surely noticed there are a bunch of places where 
`signatureHelp` does not show up while I was playing around with this change.
It would be great to have it available in more cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D38048



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

Patch to Bugzilla 31373

[Erik Viktorsson via cfe-commits]

Committing a patch to Bugzilla 
31373
A novice programmer so hopefully it complies with the coding policy.

I had to disable an assert in lib/CodeGen/CGExpr.cpp since it requires that all 
expressions are marked as Used or referenced, which is not possible if we want 
the ShouldDiagnoseUnusedDecl to return true (thus trigger the 
warn_unused_variable  warning).

The reason I removed the assert statement is because it causes five tests to 
fail. These test are the following:


*   clang -cc1 -triple i386-unknown-unknown -mllvm -inline-threshold=1024 
-O3 -emit-llvm temp-order.cpp

*   clang -cc1 -debug-info-kind=limited -std=c++11 -emit-llvm 
debug-info-scope.cpp

*   clang -cc1 -std=c++1z -triple x86_64-apple-macosx10.7.0 -emit-llvm 
cxx1z-init-statement.cpp

*   clang -cc1 -triple x86_64-apple-darwin10 -emit-llvm condition.cpp

*   clang -cc1 -emit-llvm cxx-condition.cpp

/E


CGExpr.patch
Description: CGExpr.patch


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

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM.
Note there's a new LSP method handler added upstream 
(`textDocument/signatureHelp`), we should add it to this change before 
submitting.




Comment at: clangd/ClangdLSPServer.h:47
   // Implement ProtocolCallbacks.
-  void onInitialize(StringRef ID, InitializeParams IP,
-JSONOutput &Out) override;
-  void onShutdown(JSONOutput &Out) override;
-  void onDocumentDidOpen(DidOpenTextDocumentParams Params,
- JSONOutput &Out) override;
-  void onDocumentDidChange(DidChangeTextDocumentParams Params,
-   JSONOutput &Out) override;
-  void onDocumentDidClose(DidCloseTextDocumentParams Params,
-  JSONOutput &Out) override;
-  void onDocumentOnTypeFormatting(DocumentOnTypeFormattingParams Params,
-  StringRef ID, JSONOutput &Out) override;
-  void onDocumentRangeFormatting(DocumentRangeFormattingParams Params,
- StringRef ID, JSONOutput &Out) override;
-  void onDocumentFormatting(DocumentFormattingParams Params, StringRef ID,
-JSONOutput &Out) override;
-  void onCodeAction(CodeActionParams Params, StringRef ID,
-JSONOutput &Out) override;
-  void onCompletion(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput &Out) override;
-  void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput &Out) override;
-  void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID,
-JSONOutput &Out) override;
+  void onInitialize(Ctx &C, InitializeParams &Params) override;
+  void onShutdown(Ctx &C, NoParams &Params) override;

sammccall wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Maybe there's a way to have a typed return value instead of `Ctx`?
> > > This would give an interface that's harder to misuse: one can't forget to 
> > > call `reply` or call it twice.
> > > 
> > > Here's on design that comes to mind.
> > > Notification handlers could have `void` return, normal requests can 
> > > return `Optional` or `Optional` (with result json).
> > > `Optional` is be used to indicate whether error occurred while processing 
> > > the request.
> > > 
> > After putting more thought into it, `Ctx`-based API seems to have an 
> > advantage: it will allow us to easily implement async responses.
> > E.g., we can run code completion on a background thread and continue 
> > processing other requests. When completion is ready, we will simply call 
> > `Ctx.reply` to return results to the language client.
> > 
> > To make that possible, can we allow moving `RequestContext` and pass it 
> > by-value instead of by-ref?
> Yeah I thought about returning a value... it certainly reads more nicely, but 
> I don't think we're ready to do a good job in this patch:
>  - return value should be an object ready to be serialized (rather than a 
> JSON string) - I don't want to bring that in scope here, but it might affect 
> the details of the API
>  - there's several cases we know about (return object, no reply, error reply) 
> and some we're not sure about (async as you mention - any multiple 
> responses)? I think this needs some design, and I don't yet know the project 
> well enough to drive it.
> 
> I've switched to passing Ctx by value as you suggest (though it's certainly 
> cheap enough to copy, too).
Yeah, copy is also fine there performance-wise. 

I do think move-only interface fits slightly better. We can check a whole bunch 
of invariants if `Ctx` is move-only (i.e., that request wasn't dropped without 
response or `reply` was not called twice).



Comment at: clangd/ClangdLSPServer.h:48
+  void onInitialize(Ctx &C, InitializeParams &Params) override;
+  void onShutdown(Ctx &C, NoParams &Params) override;
+  void onDocumentDidOpen(Ctx &C, DidOpenTextDocumentParams &Params) override;

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe there's a way to get rid of `NoParams`?
> > E.g. by adding a overload to `HandlerRegisterer`?
> Even if there was, I think I prefer the regularity (changed this to 
> ShutdownParams - oops!).
> 
> Otherwise the signature's dependent on some combination of {current LSP, 
> whether we actually implement the options, whether we've defined any 
> extensions}. It's harder to remember, means changing lots of places when 
> these factors change, and complicates the generic code.
> 
> Maybe I've just spent too long around Stubby, though - WDYT?
All those are good points. We also have only one method that does not have 
params currently, so it's probably not even worth additional complexity in the 
implementation.


https://reviews.llvm.org/D38464



___
cfe-commit

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

[Alexandru Octavian Buțiu via Phabricator via cfe-commits]

predator5047 added a comment.

In https://reviews.llvm.org/D38284#888124, @aaron.ballman wrote:

> Aside from a small nit, this LGTM, thanks!


Can you commit it?  I don't have commit rights.


https://reviews.llvm.org/D38284



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

[clang-tools-extra] r315057 - Fix nested namespaces in google-readability-nested-namespace-comments.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Oct  6 05:57:28 2017
New Revision: 315057

URL: http://llvm.org/viewvc/llvm-project?rev=315057&view=rev
Log:
Fix nested namespaces in google-readability-nested-namespace-comments.

Fixes PR34701.

Patch by Alexandru Octavian Buțiu.

Added:

clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=315057&r1=315056&r2=315057&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp 
Fri Oct  6 05:57:28 2017
@@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentC
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$",
+  "namespace( +([a-zA-Z0-9_:]+))?\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase),
   ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
   SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
@@ -56,6 +56,15 @@ static std::string getNamespaceComment(c
   return Fix;
 }
 
+static std::string getNamespaceComment(const std::string &NameSpaceName,
+   bool InsertLineBreak) {
+  std::string Fix = "// namespace ";
+  Fix.append(NameSpaceName);
+  if (InsertLineBreak)
+Fix.append("\n");
+  return Fix;
+}
+
 void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ND = Result.Nodes.getNodeAs("namespace");
   const SourceManager &Sources = *Result.SourceManager;
@@ -74,11 +83,38 @@ void NamespaceCommentCheck::check(const
   SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
   SourceLocation Loc = AfterRBrace;
   Token Tok;
+  SourceLocation LBracketLocation = ND->getLocation();
+  SourceLocation NestedNamespaceBegin = LBracketLocation;
+
+  // Currently for nested namepsace (n1::n2::...) the AST matcher will match 
foo
+  // then bar instead of a single match. So if we got a nested namespace we 
have
+  // to skip the next ones.
+  for (const auto &EndOfNameLocation : Ends) {
+if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin,
+  EndOfNameLocation))
+  return;
+  }
+  while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) ||
+ !Tok.is(tok::l_brace)) {
+LBracketLocation = LBracketLocation.getLocWithOffset(1);
+  }
+
+  auto TextRange =
+  Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin, 
LBracketLocation),
+Sources, getLangOpts());
+  auto NestedNamespaceName =
+  Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
+  bool IsNested = NestedNamespaceName.contains(':');
+
+  if (IsNested)
+Ends.push_back(LBracketLocation);
+
   // Skip whitespace until we find the next token.
   while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||
  Tok.is(tok::semi)) {
 Loc = Loc.getLocWithOffset(1);
   }
+
   if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
 return;
 
@@ -98,10 +134,14 @@ void NamespaceCommentCheck::check(const
   StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : "";
   StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";
 
-  // Check if the namespace in the comment is the same.
-  if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
-  (ND->getNameAsString() == NamespaceNameInComment &&
-   Anonymous.empty())) {
+  if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
+// C++17 nested namespace.
+return;
+  } else if ((ND->isAnonymousNamespace() &&
+  NamespaceNameInComment.empty()) ||
+ (ND->getNameAsString() == NamespaceNameInComment &&
+  Anonymous.empty())) {
+// Check if the namespace in the comment is the same.
 // FIXME: Maybe we need a strict mode, where we always fix namespace
 // comments with different format.
 return;
@@ -131,13 +171,16 @@ void NamespaceCommentCheck::check(const
   std::string NamespaceName =
   ND->isAnonymousNamespace()
   ? "anonymous namespace"
-  : ("namespace '" + ND->getNameAsString() + "'");
+  : ("namespace '" + NestedNamespaceName.str() + "'");
 
   diag(AfterRBrace, Messa

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

[Aaron Ballman via Phabricator via cfe-commits]

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

I've commit in r315057. Thank you!


https://reviews.llvm.org/D38284



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

Re: Patch to Bugzilla 31373

[Roman Lebedev via cfe-commits]

On Fri, Oct 6, 2017 at 3:15 PM, Erik Viktorsson via cfe-commits
 wrote:
Hi.

> Committing a patch to Bugzilla 31373
>
> A novice programmer so hopefully it complies with the coding policy.
I think it may be better to put it to Phabricator, see
https://llvm.org/docs/Phabricator.html#id3

Please do note that you should generate the diff with context (-U99),
and should put the maillist (cfe-dev in this case) as Subscriber.

>
> I had to disable an assert in lib/CodeGen/CGExpr.cpp since it requires that
> all expressions are marked as Used or referenced, which is not possible if
> we want the ShouldDiagnoseUnusedDecl to return true (thus trigger the
> warn_unused_variable  warning).
>
>
>
> The reason I removed the assert statement is because it causes five tests to
> fail. These test are the following:
>
>
>
> ·   clang -cc1 -triple i386-unknown-unknown -mllvm
> -inline-threshold=1024 -O3 -emit-llvm temp-order.cpp
>
> ·   clang -cc1 -debug-info-kind=limited -std=c++11 -emit-llvm
> debug-info-scope.cpp
>
> ·   clang -cc1 -std=c++1z -triple x86_64-apple-macosx10.7.0 -emit-llvm
> cxx1z-init-statement.cpp
>
> ·   clang -cc1 -triple x86_64-apple-darwin10 -emit-llvm condition.cpp
>
> ·   clang -cc1 -emit-llvm cxx-condition.cpp
>
>
>
> /E
Roman.

> ___
> 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] D38113: OpenCL: Assume functions are convergent

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


https://reviews.llvm.org/D38113



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

[PATCH] D38425: [clangd] Document highlights for clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for the delay.

The patch does not apply cleanly on top of the current head. Mostly conflicts 
with `switchHeaderSource` for me.
Could you please rebase your change with head and resubmit it?

Another note: current implementation does not seem to handle macros at all (it 
will only highlight definition of a macro, not its usages).
Do you have an idea on how to support them?




Comment at: clangd/ClangdUnit.cpp:784
+/// Finds declarations locations that a given source location refers to.
+class TargetDeclarationFinder : public index::IndexDataConsumer {
+  std::vector DeclarationLocations;

This `IndexDataConsumer` effectively does the same thing as 
`DeclarationLocationsFinder`.
We should reuse the code between those two.

To do that we could:
1. Change `DeclarationLocationsFinder` to return a list of `Decl*`s and 
`MacroDef*`s that are under caret.
2. In `findDefinition` we should return locations of found `Decl` and 
`MacroDef`.
3. In `documentHighlights` we would rerun `DocumentHighlightsFinder`to capture 
locations of `Decls` referenced in step 1.

Do you think this approach would work?



Comment at: clangd/ClangdUnit.cpp:1017
+
+  auto DeclLocationsFinder = std::make_shared(
+  llvm::errs(), SourceLocationBeg, AST.getASTContext(),

I wonder if we really need to rerun indexer twice here. Probably ok for the 
first version, but we should definitely think about a faster way to get the 
`Decl` under cursor than visiting the whole AST. Not sure if it's easy to do 
with clang's AST, though, maybe we'll need a separate index for that.



Comment at: clangd/Protocol.cpp:782
+llvm::raw_string_ostream(Result) << llvm::format(
+R"({"range": %s, "kind": %d})", Range::unparse(DH.range).c_str(), 
DH.kind);
+return Result;

It should work without `c_str()`, simply pass result of `unparse` (i.e., 
`std::string`) to `llvm::format`.
Generally, avoid calling `c_str()` (and `str()` on `StringRef` for that matter).



Comment at: clangd/Protocol.h:431
+ */
+struct DocumentHighlight{
+  /**

There seems to be a missing brace before `{`. I generally run this simple 
script before submission to make sure my code is always formatted:
```
find "$CLANG_TOOLS_EXTRA_PATH/clangd" -name "*.cpp" -or -name "*.h" -exec 
clang-format -i --style=LLVM {} \;
find "$CLANG_TOOLS_EXTRA_PATH/unittests/clangd" -name "*.cpp" -or -name "*.h" 
-exec clang-format -i --style=LLVM {} \ ;
```


https://reviews.llvm.org/D38425



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

[clang-tools-extra] r315059 - Fixing the command line for a test and switching from tabs to spaces.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Oct  6 06:14:28 2017
New Revision: 315059

URL: http://llvm.org/viewvc/llvm-project?rev=315059&view=rev
Log:
Fixing the command line for a test and switching from tabs to spaces.

Modified:

clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315059&r1=315058&r2=315059&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
 Fri Oct  6 06:14:28 2017
@@ -1,12 +1,10 @@
-// RUN: %check_clang_tidy %s google-readability-namespace-comments %t -- 
-std=c++17
+// RUN: %check_clang_tidy %s google-readability-namespace-comments %t -- -- 
-std=c++17
 
 namespace n1::n2 {
 namespace n3 {
-   
-   // So that namespace is not empty.
-   void f();
-   
-   
+  // So that namespace is not empty.
+  void f();
+
 // CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with
 // CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here
 // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated 
with a closing comment [google-readability-namespace-comments]


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

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv updated this revision to Diff 117986.

https://reviews.llvm.org/D38578

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/Index/preamble-conditionals-inverted.cpp
  test/Index/preamble-conditionals-skipping.cpp

Index: test/Index/preamble-conditionals-skipping.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-skipping.cpp
@@ -0,0 +1,16 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \
+// RUN:   local -std=c++14 %s 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "error:"
+
+#ifdef MYCPLUSPLUS
+extern "C" {
+#endif
+
+#ifdef MYCPLUSPLUS
+}
+#endif
+
+int main()
+{
+return 0;
+}
Index: test/Index/preamble-conditionals-inverted.cpp
===
--- test/Index/preamble-conditionals-inverted.cpp
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -3,6 +3,8 @@
 // RUN: | FileCheck %s --implicit-check-not "error:"
 #ifdef FOO_H
 
-void foo();
+void foo() {}
 
 #endif
+
+int foo() { return 0; }
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -2407,6 +2407,17 @@
 
   if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
 assert(!IsModule);
+auto SkipInfo = PP.getPreambleSkipInfo();
+if (SkipInfo.hasValue()) {
+  Record.push_back(true);
+  AddSourceLocation(SkipInfo->HashTokenLoc, Record);
+  AddSourceLocation(SkipInfo->IfTokenLoc, Record);
+  Record.push_back(SkipInfo->FoundNonSkipPortion);
+  Record.push_back(SkipInfo->FoundElse);
+  AddSourceLocation(SkipInfo->ElseLoc, Record);
+} else {
+  Record.push_back(false);
+}
 for (const auto &Cond : PP.getPreambleConditionalStack()) {
   AddSourceLocation(Cond.IfLoc, Record);
   Record.push_back(Cond.WasSkipping);
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2995,16 +2995,28 @@
 
 case PP_CONDITIONAL_STACK:
   if (!Record.empty()) {
+unsigned Idx = 0, End = Record.size() - 1;
+bool ReachedEOFWhileSkipping = Record[Idx++];
+llvm::Optional SkipInfo;
+if (ReachedEOFWhileSkipping) {
+  SourceLocation HashToken = ReadSourceLocation(F, Record, Idx);
+  SourceLocation IfTokenLoc = ReadSourceLocation(F, Record, Idx);
+  bool FoundNonSkipPortion = Record[Idx++];
+  bool FoundElse = Record[Idx++];
+  SourceLocation ElseLoc = ReadSourceLocation(F, Record, Idx);
+  SkipInfo.emplace(HashToken, IfTokenLoc, FoundNonSkipPortion,
+   FoundElse, ElseLoc);
+}
 SmallVector ConditionalStack;
-for (unsigned Idx = 0, N = Record.size() - 1; Idx < N; /* in loop */) {
+while (Idx < End) {
   auto Loc = ReadSourceLocation(F, Record, Idx);
   bool WasSkipping = Record[Idx++];
   bool FoundNonSkip = Record[Idx++];
   bool FoundElse = Record[Idx++];
   ConditionalStack.push_back(
   {Loc, WasSkipping, FoundNonSkip, FoundElse});
 }
-PP.setReplayablePreambleConditionalStack(ConditionalStack);
+PP.setReplayablePreambleConditionalStack(ConditionalStack, SkipInfo);
   }
   break;
 
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -544,6 +544,13 @@
"CurPPLexer is null when calling replayPreambleConditionalStack.");
 CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack());
 PreambleConditionalStack.doneReplaying();
+if (PreambleConditionalStack.reachedEOFWhileSkipping())
+  SkipExcludedConditionalBlock(
+  PreambleConditionalStack.SkipInfo->HashTokenLoc,
+  PreambleConditionalStack.SkipInfo->IfTokenLoc,
+  PreambleConditionalStack.SkipInfo->FoundNonSkipPortion,
+  PreambleConditionalStack.SkipInfo->FoundElse,
+  PreambleConditionalStack.SkipInfo->ElseLoc);
   }
 }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -350,16 +350,19 @@
 /// If ElseOk is true, then \#else directives are ok, if not, then we have
 /// already seen one so a \#else directive is a duplicate.  When this returns,
 /// the caller can lex the first valid token.
-void Preprocessor::SkipExcludedConditionalBlock(const Token &HashToken,
+void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
   

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D38578



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

Re: [clang-tools-extra] r315057 - Fix nested namespaces in google-readability-nested-namespace-comments.

[Alexander Kornienko via cfe-commits]

On 6 Oct 2017 14:59, "Aaron Ballman via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

Author: aaronballman
Date: Fri Oct  6 05:57:28 2017
New Revision: 315057

URL: http://llvm.org/viewvc/llvm-project?rev=315057&view=rev
Log:
Fix nested namespaces in google-readability-nested-namespace-comments.

Fixes PR34701.

Patch by Alexandru Octavian Buțiu.

Added:
clang-tools-extra/trunk/test/clang-tidy/google-readability-
nested-namespace-comments.cpp


Can we rename the test so it starts with the check name? E.g.
"google-readability-namespace-comments-cxx17" or something else that makes
sense?

Modified:
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h

Modified: clang-tools-extra/trunk/clang-tidy/readability/
NamespaceCommentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=
315057&r1=315056&r2=315057&view=diff

==
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
Fri Oct  6 05:57:28 2017
@@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentC
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)?
*"
-  "namespace( +([a-zA-Z0-9_]+))?\\.?
*(\\*/)?$",
+  "namespace( +([a-zA-Z0-9_:]+))?\\.?
*(\\*/)?$",
   llvm::Regex::IgnoreCase),
   ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
   SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
@@ -56,6 +56,15 @@ static std::string getNamespaceComment(c
   return Fix;
 }

+static std::string getNamespaceComment(const std::string &NameSpaceName,
+   bool InsertLineBreak) {
+  std::string Fix = "// namespace ";
+  Fix.append(NameSpaceName);
+  if (InsertLineBreak)
+Fix.append("\n");
+  return Fix;
+}
+
 void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ND = Result.Nodes.getNodeAs("namespace");
   const SourceManager &Sources = *Result.SourceManager;
@@ -74,11 +83,38 @@ void NamespaceCommentCheck::check(const
   SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
   SourceLocation Loc = AfterRBrace;
   Token Tok;
+  SourceLocation LBracketLocation = ND->getLocation();
+  SourceLocation NestedNamespaceBegin = LBracketLocation;
+
+  // Currently for nested namepsace (n1::n2::...) the AST matcher will
match foo
+  // then bar instead of a single match. So if we got a nested namespace
we have
+  // to skip the next ones.
+  for (const auto &EndOfNameLocation : Ends) {
+if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin,
+  EndOfNameLocation))
+  return;
+  }
+  while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts())
||
+ !Tok.is(tok::l_brace)) {
+LBracketLocation = LBracketLocation.getLocWithOffset(1);
+  }
+
+  auto TextRange =
+  Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin,
LBracketLocation),
+Sources, getLangOpts());
+  auto NestedNamespaceName =
+  Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
+  bool IsNested = NestedNamespaceName.contains(':');
+
+  if (IsNested)
+Ends.push_back(LBracketLocation);
+
   // Skip whitespace until we find the next token.
   while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||
  Tok.is(tok::semi)) {
 Loc = Loc.getLocWithOffset(1);
   }
+
   if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
 return;

@@ -98,10 +134,14 @@ void NamespaceCommentCheck::check(const
   StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] :
"";
   StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";

-  // Check if the namespace in the comment is the same.
-  if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
-  (ND->getNameAsString() == NamespaceNameInComment &&
-   Anonymous.empty())) {
+  if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
+// C++17 nested namespace.
+return;
+  } else if ((ND->isAnonymousNamespace() &&
+  NamespaceNameInComment.empty()) ||
+ (ND->getNameAsString() == NamespaceNameInComment &&
+  Anonymous.empty())) {
+// Check if the namespace in the comment is the same.
 // FIXME: Maybe we need a strict mode, where we always fix
namespace
 // comments with different format.
 return;
@@ -131,13 +171,16 @@ void NamespaceCommentCheck::check(const
   std::string Names

[clang-tools-extra] r315060 - Renaming a test to start with the name of the check based on post-commit review feedback; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Oct  6 06:27:59 2017
New Revision: 315060

URL: http://llvm.org/viewvc/llvm-project?rev=315060&view=rev
Log:
Renaming a test to start with the name of the check based on post-commit review 
feedback; NFC.

Added:

clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17
  - copied unchanged from r315059, 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
Removed:

clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp

Removed: 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315059&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
 (removed)
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s google-readability-namespace-comments %t -- -- 
-std=c++17
-
-namespace n1::n2 {
-namespace n3 {
-  // So that namespace is not empty.
-  void f();
-
-// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with
-// CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here
-// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated 
with a closing comment [google-readability-namespace-comments]
-// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1::n2' starts here
-}}
-// CHECK-FIXES: }  // namespace n3
-// CHECK-FIXES: }  // namespace n1::n2
-


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

Re: [clang-tools-extra] r315057 - Fix nested namespaces in google-readability-nested-namespace-comments.

[Aaron Ballman via cfe-commits]

On Fri, Oct 6, 2017 at 9:27 AM, Alexander Kornienko  wrote:
>
>
> On 6 Oct 2017 14:59, "Aaron Ballman via cfe-commits"
>  wrote:
>
> Author: aaronballman
> Date: Fri Oct  6 05:57:28 2017
> New Revision: 315057
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315057&view=rev
> Log:
> Fix nested namespaces in google-readability-nested-namespace-comments.
>
> Fixes PR34701.
>
> Patch by Alexandru Octavian Buțiu.
>
> Added:
>
> clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
>
>
> Can we rename the test so it starts with the check name? E.g.
> "google-readability-namespace-comments-cxx17" or something else that makes
> sense?

Sure! I've commit in r315060.

~Aaron

>
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
> clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=315057&r1=315056&r2=315057&view=diff
> ==
> --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
> Fri Oct  6 05:57:28 2017
> @@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentC
>   ClangTidyContext *Context)
>  : ClangTidyCheck(Name, Context),
>NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)?
> *"
> -  "namespace( +([a-zA-Z0-9_]+))?\\.?
> *(\\*/)?$",
> +  "namespace( +([a-zA-Z0-9_:]+))?\\.?
> *(\\*/)?$",
>llvm::Regex::IgnoreCase),
>ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
>SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
> @@ -56,6 +56,15 @@ static std::string getNamespaceComment(c
>return Fix;
>  }
>
> +static std::string getNamespaceComment(const std::string &NameSpaceName,
> +   bool InsertLineBreak) {
> +  std::string Fix = "// namespace ";
> +  Fix.append(NameSpaceName);
> +  if (InsertLineBreak)
> +Fix.append("\n");
> +  return Fix;
> +}
> +
>  void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
>const auto *ND = Result.Nodes.getNodeAs("namespace");
>const SourceManager &Sources = *Result.SourceManager;
> @@ -74,11 +83,38 @@ void NamespaceCommentCheck::check(const
>SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
>SourceLocation Loc = AfterRBrace;
>Token Tok;
> +  SourceLocation LBracketLocation = ND->getLocation();
> +  SourceLocation NestedNamespaceBegin = LBracketLocation;
> +
> +  // Currently for nested namepsace (n1::n2::...) the AST matcher will
> match foo
> +  // then bar instead of a single match. So if we got a nested namespace we
> have
> +  // to skip the next ones.
> +  for (const auto &EndOfNameLocation : Ends) {
> +if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin,
> +  EndOfNameLocation))
> +  return;
> +  }
> +  while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts())
> ||
> + !Tok.is(tok::l_brace)) {
> +LBracketLocation = LBracketLocation.getLocWithOffset(1);
> +  }
> +
> +  auto TextRange =
> +  Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin,
> LBracketLocation),
> +Sources, getLangOpts());
> +  auto NestedNamespaceName =
> +  Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
> +  bool IsNested = NestedNamespaceName.contains(':');
> +
> +  if (IsNested)
> +Ends.push_back(LBracketLocation);
> +
>// Skip whitespace until we find the next token.
>while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||
>   Tok.is(tok::semi)) {
>  Loc = Loc.getLocWithOffset(1);
>}
> +
>if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
>  return;
>
> @@ -98,10 +134,14 @@ void NamespaceCommentCheck::check(const
>StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] :
> "";
>StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";
>
> -  // Check if the namespace in the comment is the same.
> -  if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
> -  (ND->getNameAsString() == NamespaceNameInComment &&
> -   Anonymous.empty())) {
> +  if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
> +// C++17 nested namespace.
> +return;
> +  } else if ((ND->isAnonymousNamespace() &&
> +  NamespaceNameInComment.empty()) ||
> + (ND->getNameAsString() == NamespaceNameInComment &&
> +  Anon

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv added a comment.

lgtm, but someone else will probably have to review it too.




Comment at: lib/Parse/ParseDeclCXX.cpp:3196
   if (Tok.is(tok::colon)) {
+ParseScope ClassScope(this, Scope::ClassScope|Scope::DeclScope | 
Scope::ClassInheritanceScope);
+

InheritanceScope instead of ClassScope? I mean, it's not really the scope of 
the class, that would be inside the class decl.
Also, clang-format this line.


https://reviews.llvm.org/D38618



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

[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov resigned from this revision.
ilya-biryukov added a comment.

I'm not actually familiar with `CXCursor`, so can't really help with reviewing 
this.


https://reviews.llvm.org/D38615



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

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

baloghadamsoftware added a comment.

It seems that Artem's suggestion is not enough (or I misunderstood it). So two 
options remain: either we drop this and revert to the local solution in the 
Iterator Checkers or we extend the type when rearranging the comparison. Which 
way to go?


https://reviews.llvm.org/D35109



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

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

[Ivan Donchevskii via Phabricator via cfe-commits]

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

Fix according to the review comment


https://reviews.llvm.org/D38618

Files:
  include/clang/Sema/Scope.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-super.cpp


Index: test/Index/complete-super.cpp
===
--- test/Index/complete-super.cpp
+++ test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test 
-code-completion-at=%s:10:12 %s | FileCheck 
-check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1647,21 +1647,22 @@
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope = !(S->getFlags() & 
Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // protected:
 Builder.AddTypedTextChunk("protected");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // private:
 Builder.AddTypedTextChunk("private");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
   }
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3193,6 +3193,9 @@
   }
 
   if (Tok.is(tok::colon)) {
+ParseScope InheritanceScope(this, Scope::ClassScope | Scope::DeclScope |
+  Scope::ClassInheritanceScope);
+
 ParseBaseClause(TagDecl);
 if (!Tok.is(tok::l_brace)) {
   bool SuggestFixIt = false;
Index: include/clang/Sema/Scope.h
===
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -124,6 +124,9 @@
 
 /// We are currently in the filter expression of an SEH except block.
 SEHFilterScope = 0x20,
+
+/// We are between inheritance colon and the real class/struct definition 
scope
+ClassInheritanceScope = 0x40,
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit


Index: test/Index/complete-super.cpp
===
--- test/Index/complete-super.cpp
+++ test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:10:12 %s | FileCheck -check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1647,21 +1647,22 @@
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope = !(S->getFlags() & Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // protected:
 Builder.AddTypedTextChunk("protected");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK

[PATCH] D38596: Implement attribute target multiversioning

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Decl.h:2226
+  MultiVersionKind getMultiVersionKind() const {
+return static_cast(this->MultiVersion);
+  }

Drop the `this->`



Comment at: include/clang/AST/Decl.h:2229
+  void setMultiVersionKind(MultiVersionKind MV) {
+this->MultiVersion = static_cast(MV);
+  }

Same



Comment at: include/clang/Basic/Attr.td:1847
+
+  bool operator ==(const ParsedTargetAttr &Other) const {
+return std::tie(Features, Architecture) ==

`operator==` instead of `operator ==`



Comment at: include/clang/Basic/Attr.td:1852
+
+  bool operator !=(const ParsedTargetAttr &Other) const {
+return !(*this == Other);

Same



Comment at: include/clang/Basic/Attr.td:1898
+  llvm::raw_svector_ostream OS(Buffer);
+  auto Info = parse();
+

Don't use `auto` here.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9301
+def err_target_causes_illegal_multiversioning
+: Error<"function redeclaration causes a multiversioned function, but a "
+"previous declaration lacks a 'target' attribute">;

'causes' seems a bit strange; how about `function redeclaration declares a 
multiversioned function,`? (Or something else, I'm not tied to my wording.)



Comment at: include/clang/Sema/Sema.h:1937
+  // Checks to see if attribute 'target' results in a valid situation.
+  bool CheckMultiVersionedDecl(FunctionDecl *NewFD, FunctionDecl *OldFD);
+  // Helper function for CheckMultiVersionDecl to ensure that a declaration

It's a bit strange that this function is called "Check" but it mutates the 
arguments.



Comment at: lib/Basic/Targets/X86.cpp:1402
+
+  for (auto &Str : TargetArray) {
+if (Lhs == Str)

`const auto &`



Comment at: lib/CodeGen/CodeGenFunction.h:3707
+llvm::Function *Function;
+ResolverOption(TargetAttr::ParsedTargetAttr &&AttrInfo,// can be moved?
+   llvm::Function *Function)

The comment doesn't inspire confidence with the trailing question mark. ;-)



Comment at: lib/Sema/SemaDecl.cpp:9241
+bool Sema::CheckMultiVersionOption(const FunctionDecl *FD) {
+  auto *TA = FD->getAttr();
+

`const auto *`



Comment at: lib/Sema/SemaDecl.cpp:9245
+return true;
+  auto ParseInfo = TA->parse();
+

Don't use `auto` here.



Comment at: lib/Sema/SemaDecl.cpp:9259
+
+  for (auto &Feature : ParseInfo.Features) {
+auto BareFeat = StringRef{Feature}.substr(1);

`const auto &`



Comment at: lib/Sema/SemaDecl.cpp:9281
+  // for each new architecture.
+  auto Arch = Context.getTargetInfo().getTriple().getArch();
+  if (Arch != llvm::Triple::x86_64 && Arch != llvm::Triple::x86) {

Don't use `auto` here.



Comment at: lib/Sema/SemaDecl.cpp:9287
+
+  bool result = true;
+  for (const auto *FD : OldFD->redecls())

Should be named `Result`



Comment at: lib/Sema/SemaDecl.cpp:9295
+bool Sema::CheckMultiVersionedDecl(FunctionDecl *NewFD, FunctionDecl *OldFD) {
+  // The first Decl is easy, it is either the 'All' case, or 'None'.
+  if (!OldFD) {

Remove the comma after `case`.



Comment at: lib/Sema/SemaDecl.cpp:9297-9300
+if (NewFD->hasAttr())
+  NewFD->setMultiVersionKind(FunctionDecl::MultiVersionKind::All);
+else
+  NewFD->setMultiVersionKind(FunctionDecl::MultiVersionKind::None);

Might be more succinctly written with `?:` (unsure which looks better, try it 
and see).



Comment at: lib/Sema/SemaDecl.cpp:9304
+
+  auto *TA = NewFD->getAttr();
+  switch (OldFD->getMultiVersionKind()) {

`const auto *` -- might as well hoist this to the top of the function and 
remove the call to `hasAttr<>()` above.



Comment at: lib/Sema/SemaDecl.cpp:9306
+  switch (OldFD->getMultiVersionKind()) {
+  default:
+llvm_unreachable("Invalid State for Multiversioning.");

I suspect this will give warnings about having a `default` label in a 
fully-covered switch.



Comment at: lib/Sema/SemaDecl.cpp:9344
+}
+// If this doesn't convert it to a multi-version, this is clearly a 'all'
+// case.

a 'all' -> an 'all'



Comment at: lib/Sema/SemaDecl.cpp:9366
+  case FunctionDecl::MultiVersionKind::Partial:
+// A partial not adding a Target Attribute is fine, everything stays as
+// partial.

Attribute -> attribute



Comment at: lib/Sema/SemaDecl.cpp:9372
+}
+// A part

[PATCH] D33537: [clang-tidy] Exception Escape Checker

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

baloghadamsoftware added a comment.

I think that the acceptable false positive rate of a compiler warning is much 
lower than that of a Tidy check. So I understand that the fronted patch will 
not handle the indirect cases (which are the most important in my opinion) or 
the cases where there are multiple throws in a try block. However, it is nearly 
impossible to remove the most straightforward cases from the check and also in 
today's Tidy checks or Static Analyzer checkers there is some overlapping with 
warnings. I think that maybe we should consider making the error locations and 
error text to overlap with the warning so an advanced tool (such as 
CodeChecker) could unify them.


https://reviews.llvm.org/D33537



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

[PATCH] D34508: [Analyzer] Bug Reporter Visitor to Display Values of Variables - PRELIMINARY!

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

baloghadamsoftware added a comment.

I am considering to restrict the assumptions to nodes marked as interesting and 
to the location of the bug. However, I have difficulties with the latter, it 
seems that the bug location itself is not part of the bug path.


https://reviews.llvm.org/D34508



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

[clang-tools-extra] r315065 - [clangd] Run clang-format on the source code. NFC.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Oct  6 07:39:39 2017
New Revision: 315065

URL: http://llvm.org/viewvc/llvm-project?rev=315065&view=rev
Log:
[clangd] Run clang-format on the source code. NFC.

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

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=315065&r1=315064&r2=315065&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Oct  6 07:39:39 2017
@@ -398,12 +398,11 @@ llvm::Optional ClangdServer::switc
   return NewPath.str().str(); // First str() to convert from SmallString to
   // StringRef, second to convert from 
StringRef
   // to std::string
-
+
 // Also check NewExt in upper-case, just in case.
 llvm::sys::path::replace_extension(NewPath, NewExt.upper());
 if (FS->exists(NewPath))
   return NewPath.str().str();
-
   }
 
   return llvm::None;


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

[PATCH] D38627: [clangd] Added a move-only function helpers.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

They are now used in ClangdScheduler instead of deferred std::async
computations.
The results of `std::async` are much less effective and do not provide
a good abstraction for similar purposes, i.e. for storing additional callbacks
to clangd async tasks. The actual callback API will follow a bit later.


https://reviews.llvm.org/D38627

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Function.h

Index: clangd/Function.h
===
--- /dev/null
+++ clangd/Function.h
@@ -0,0 +1,126 @@
+//===--- Function.h - Utility callable wrappers  -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
+
+namespace clang {
+namespace clangd {
+
+/// A move-only type-erasing function wrapper. Similar to `std::function`, but
+/// allows to store move-only callables.
+template  class UniqueFunction;
+
+template  class UniqueFunction {
+public:
+  UniqueFunction() = default;
+
+  UniqueFunction(UniqueFunction const &) = delete;
+  UniqueFunction &operator=(UniqueFunction const &) = delete;
+
+  UniqueFunction(UniqueFunction &&) noexcept = default;
+  UniqueFunction &operator=(UniqueFunction &&) noexcept = default;
+
+  template 
+  UniqueFunction(Callable Func)
+  : CallablePtr(llvm::make_unique<
+FunctionCallImpl::type>>(
+std::forward(Func))) {}
+
+  operator bool() { return CallablePtr; }
+
+  Ret operator()(Args... As) {
+assert(CallablePtr);
+CallablePtr->Call(std::forward(As)...);
+  }
+
+private:
+  class FunctionCallBase {
+  public:
+virtual ~FunctionCallBase() = default;
+virtual Ret Call(Args... As) = 0;
+  };
+
+  template 
+  class FunctionCallImpl final : public FunctionCallBase {
+static_assert(
+std::is_same::type>::value,
+"FunctionCallImpl must be instanstiated with std::decay'ed types");
+
+  public:
+FunctionCallImpl(Callable Func) : Func(std::move(Func)) {}
+
+Ret Call(Args... As) override { return Func(std::forward(As)...); }
+
+  private:
+Callable Func;
+  };
+
+  std::unique_ptr CallablePtr;
+};
+
+/// Stores a callable object (Func) and arguments (Args) and allows to call the
+/// callable with provided arguments later using `operator ()`. The arguments
+/// are std::forward'ed into the callable in the body of `operator()`. Therefore
+/// `operator()` can only be called once, as some of the arguments could be
+/// std::move'ed into the callable on first call.
+template  struct ForwardBinder {
+  using Tuple = std::tuple::type,
+   typename std::decay::type...>;
+  Tuple FuncWithArguments;
+#ifndef NDEBUG
+  bool WasCalled = false;
+#endif
+
+public:
+  ForwardBinder(Tuple FuncWithArguments)
+  : FuncWithArguments(std::move(FuncWithArguments)) {}
+
+private:
+  template 
+  auto CallImpl(llvm::integer_sequence Seq,
+RestArgs... Rest)
+  -> decltype(std::get<0>(this->FuncWithArguments)(std::forward(
+  std::get(this->FuncWithArguments))...)) {
+return std::get<0>(this->FuncWithArguments)(
+std::forward(std::get(this->FuncWithArguments))...,
+std::forward(Rest)...);
+  }
+
+public:
+  template 
+  auto operator()(RestArgs... Rest)
+  -> decltype(CallImpl(llvm::index_sequence_for(),
+   std::forward(Rest)...)) {
+
+#ifndef NDEBUG
+assert(!WasCalled && "Can only call result of BindWithForward once.");
+WasCalled = true;
+#endif
+return CallImpl(llvm::index_sequence_for(),
+std::forward(Rest)...);
+  }
+};
+
+/// Creates an object that stores a callable (\p F) and first arguments to the
+/// callable (\p As) and allows to call \p F with \Args at a later point.
+/// Similar to std::bind, but also works with move-only \p F and \p As.
+///
+/// The returned object can only be called once, as \p As are std::forwarded'ed
+/// (therefore can be std::move`d) into \p F for the call.
+template 
+ForwardBinder BindWithForward(Func F, Args... As) {
+  return ForwardBinder(
+  std::make_tuple(std::forward(F), std::forward(As)...));
+}
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 
 #include "ClangdUnit.h"
+#include "Function.h"
 #include "Protocol.h"
 
 #include 
@@ -132,9 +133,8 @@
 
 {
   std::lock_guard Lock(Mutex);
-  RequestQueue.push_front(std::async(std::launch::deferred,
- std::for

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

Hello Daniel!

It is a great feature to add, thanks for working on this!
I have just small comments (rather questions) on the code.




Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:155
+  Children.push(FuncBody);
+  while (!Children.empty()) {
+const Stmt *Child = Children.top();

I think instead of this logic it would be better to use ConstStmtVisitor. In 
this case it does quite the same thing in a (maybe?) more structured manner. 
What do you think?



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:168
+  VD->getStorageClass() == SC_Static &&
+  !VD->getType()->isPointerType()) {
+Vars->insert(VD);

Is it possible that a type is an IntegerType and a  PointerType at the same 
time? If these are excluding cases then the check for !isPointer could be 
removed.


Repository:
  rL LLVM

https://reviews.llvm.org/D37897



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

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D34512#877643, @xazax.hun wrote:

> - Unittests now creates temporary files at the correct location
> - Temporary files are also cleaned up when the process is terminated
> - Absolute paths are handled correctly by the library


I think there is an issue with this change.

First, the function map generation writes absolute paths to the map file. Now 
when the `parseCrossTUIndex` function parses it, it will no longer prepend the 
paths with the CTUDir and therefore expect the precompiled AST files at the 
exact path of the source file. This seems like a bad requirement, because the 
user would have to overwrite his source files.

If I understand your intention correctly, a fix would be to replace the 
`is_absolute` check with a check for `CTUDir == ""` in the `parseCrossTUIndex` 
function.


Repository:
  rL LLVM

https://reviews.llvm.org/D34512



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

[PATCH] D38628: Remove unneeded typename from test

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.

https://reviews.llvm.org/D38628

Files:
  test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp


Index: test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
===
--- test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
+++ test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
@@ -18,5 +18,5 @@
 int main()
 {
 typedef std::pair T;
-typename std::tuple_element<2, T>::type foo; // expected-error@utility:* 
{{Index out of bounds in std::tuple_element>}}
+std::tuple_element<2, T>::type foo; // expected-error@utility:* {{Index 
out of bounds in std::tuple_element>}}
 }


Index: test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
===
--- test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
+++ test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
@@ -18,5 +18,5 @@
 int main()
 {
 typedef std::pair T;
-typename std::tuple_element<2, T>::type foo; // expected-error@utility:* {{Index out of bounds in std::tuple_element>}}
+std::tuple_element<2, T>::type foo; // expected-error@utility:* {{Index out of bounds in std::tuple_element>}}
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38362: Mark tests as unsupported in C++98 as well

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 118005.
rogfer01 retitled this revision from "Mark test as unsupported in C++98 as 
well" to "Mark tests as unsupported in C++98 as well".
rogfer01 added a comment.

ChangeLog:

- I wanted to mark two tests but forgot to add one in the previous change


https://reviews.llvm.org/D38362

Files:
  test/std/re/re.alg/re.alg.match/exponential.pass.cpp
  test/std/re/re.alg/re.alg.search/exponential.pass.cpp


Index: test/std/re/re.alg/re.alg.search/exponential.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/exponential.pass.cpp
+++ test/std/re/re.alg/re.alg.search/exponential.pass.cpp
@@ -9,7 +9,7 @@
 
 // 
 // UNSUPPORTED: libcpp-no-exceptions
-// UNSUPPORTED: c++03
+// UNSUPPORTED: c++98, c++03
 
 // template 
 // bool
Index: test/std/re/re.alg/re.alg.match/exponential.pass.cpp
===
--- test/std/re/re.alg/re.alg.match/exponential.pass.cpp
+++ test/std/re/re.alg/re.alg.match/exponential.pass.cpp
@@ -9,7 +9,7 @@
 
 // 
 // UNSUPPORTED: libcpp-no-exceptions
-// UNSUPPORTED: c++03
+// UNSUPPORTED: c++98, c++03
 
 // template 
 // bool


Index: test/std/re/re.alg/re.alg.search/exponential.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/exponential.pass.cpp
+++ test/std/re/re.alg/re.alg.search/exponential.pass.cpp
@@ -9,7 +9,7 @@
 
 // 
 // UNSUPPORTED: libcpp-no-exceptions
-// UNSUPPORTED: c++03
+// UNSUPPORTED: c++98, c++03
 
 // template 
 // bool
Index: test/std/re/re.alg/re.alg.match/exponential.pass.cpp
===
--- test/std/re/re.alg/re.alg.match/exponential.pass.cpp
+++ test/std/re/re.alg/re.alg.match/exponential.pass.cpp
@@ -9,7 +9,7 @@
 
 // 
 // UNSUPPORTED: libcpp-no-exceptions
-// UNSUPPORTED: c++03
+// UNSUPPORTED: c++98, c++03
 
 // template 
 // bool
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38134: [OpenCL] Emit enqueued block as kernel

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:144
+  if (auto *I = dyn_cast(V)) {
+// If the block literal is emitted as an instruction, it is an alloca
+// and the block invoke function is stored to GEP of this alloca.

Anastasia wrote:
> Why do we need to replace original block calls with the kernels? I think in 
> case of calling a block we could use the original block function and only for 
> enqueue use the kernel that would call the block function inside. The pointer 
> to the kernel wrapper could be passed as an additional parameter to 
> `enqueue_kernel` calls. We won't need to iterate through all IR then.
`CGF.EmitScalarExpr(Block)` returns the block literal structure which contains 
the size/align/invoke_function/captures. The block invoke function is stored to 
the struct by a `StoreInst`. To create the wrapper kernel, we need to get the 
block invoke function, therefore we have to iterate through IR.

Since we need to find the store instruction any way, it is simpler to just 
replace the stored function with the kernel and pass the block literal struct, 
instead of passing the kernel separately.



Comment at: lib/CodeGen/TargetInfo.cpp:8927
+llvm::Function *
+TargetCodeGenInfo::createEnqueuedBlockKernel(CodeGenFunction &CGF,
+ llvm::Function *Invoke,

Anastasia wrote:
> Could you add some comments please?
Will do.



Comment at: lib/CodeGen/TargetInfo.cpp:8949
+  Builder.restoreIP(IP);
+  return F;
+}

Anastasia wrote:
> Wondering if we should add the kernel metadata (w/o args) since it was used 
> for long time to indicate the kernel.
Currently (before this change), clang already does not generate kernel metadata 
if there is no vec_type_hint, work_group_size_hint, reqd_work_group_size. 
Remember last time we made the change to use function metadata to represent 
these attributes. Whether a function is a kernel can be determined by its 
calling convention.



Comment at: lib/CodeGen/TargetInfo.h:35
 class Decl;
+class ASTContext;
 

Anastasia wrote:
> Do we need this?
Will remove it.



Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:9
 
-// N.B. The check here only exists to set BL_GLOBAL
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) 
addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ 
i32, i32, i8 addrspace(4)* } addrspace(1)* 
[[BL_GLOBAL:@__block_literal_global(\.[0-9]+)?]] to void (i8 addrspace(3)*) 
addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: %struct.__opencl_block_literal_generic = type { i32, i32, i8 
addrspace(4)* }
+

Anastasia wrote:
> Can we check generated kernel function too?
will do.


https://reviews.llvm.org/D38134



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

[PATCH] D38627: [clangd] Added a move-only function helpers.

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Fix to ForwardBinder.
- Add UniqueFunction(nullptr) constructor.
- Added missing STL headers to Function.h


https://reviews.llvm.org/D38627

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Function.h

Index: clangd/Function.h
===
--- /dev/null
+++ clangd/Function.h
@@ -0,0 +1,132 @@
+//===--- Function.h - Utility callable wrappers  -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// A move-only type-erasing function wrapper. Similar to `std::function`, but
+/// allows to store move-only callables.
+template  class UniqueFunction;
+
+template  class UniqueFunction {
+public:
+  UniqueFunction() = default;
+  UniqueFunction(std::nullptr_t) : UniqueFunction(){};
+
+  UniqueFunction(UniqueFunction const &) = delete;
+  UniqueFunction &operator=(UniqueFunction const &) = delete;
+
+  UniqueFunction(UniqueFunction &&) noexcept = default;
+  UniqueFunction &operator=(UniqueFunction &&) noexcept = default;
+
+  template 
+  UniqueFunction(Callable Func)
+  : CallablePtr(llvm::make_unique<
+FunctionCallImpl::type>>(
+std::forward(Func))) {}
+
+  operator bool() { return CallablePtr; }
+
+  Ret operator()(Args... As) {
+assert(CallablePtr);
+CallablePtr->Call(std::forward(As)...);
+  }
+
+private:
+  class FunctionCallBase {
+  public:
+virtual ~FunctionCallBase() = default;
+virtual Ret Call(Args... As) = 0;
+  };
+
+  template 
+  class FunctionCallImpl final : public FunctionCallBase {
+static_assert(
+std::is_same::type>::value,
+"FunctionCallImpl must be instanstiated with std::decay'ed types");
+
+  public:
+FunctionCallImpl(Callable Func) : Func(std::move(Func)) {}
+
+Ret Call(Args... As) override { return Func(std::forward(As)...); }
+
+  private:
+Callable Func;
+  };
+
+  std::unique_ptr CallablePtr;
+};
+
+/// Stores a callable object (Func) and arguments (Args) and allows to call the
+/// callable with provided arguments later using `operator ()`. The arguments
+/// are std::forward'ed into the callable in the body of `operator()`. Therefore
+/// `operator()` can only be called once, as some of the arguments could be
+/// std::move'ed into the callable on first call.
+template  struct ForwardBinder {
+  using Tuple = std::tuple::type,
+   typename std::decay::type...>;
+  Tuple FuncWithArguments;
+#ifndef NDEBUG
+  bool WasCalled = false;
+#endif
+
+public:
+  ForwardBinder(Tuple FuncWithArguments)
+  : FuncWithArguments(std::move(FuncWithArguments)) {}
+
+private:
+  template 
+  auto CallImpl(llvm::integer_sequence Seq,
+RestArgs... Rest)
+  -> decltype(std::get<0>(this->FuncWithArguments)(
+  std::forward(std::get(this->FuncWithArguments))...,
+  std::forward(Rest)...)) {
+return std::get<0>(this->FuncWithArguments)(
+std::forward(std::get(this->FuncWithArguments))...,
+std::forward(Rest)...);
+  }
+
+public:
+  template 
+  auto operator()(RestArgs... Rest)
+  -> decltype(CallImpl(llvm::index_sequence_for(),
+   std::forward(Rest)...)) {
+
+#ifndef NDEBUG
+assert(!WasCalled && "Can only call result of BindWithForward once.");
+WasCalled = true;
+#endif
+return CallImpl(llvm::index_sequence_for(),
+std::forward(Rest)...);
+  }
+};
+
+/// Creates an object that stores a callable (\p F) and first arguments to the
+/// callable (\p As) and allows to call \p F with \Args at a later point.
+/// Similar to std::bind, but also works with move-only \p F and \p As.
+///
+/// The returned object can only be called once, as \p As are std::forwarded'ed
+/// (therefore can be std::move`d) into \p F for the call.
+template 
+ForwardBinder BindWithForward(Func F, Args... As) {
+  return ForwardBinder(
+  std::make_tuple(std::forward(F), std::forward(As)...));
+}
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 
 #include "ClangdUnit.h"
+#include "Function.h"
 #include "Protocol.h"
 
 #include 
@@ -132,9 +133,8 @@
 
 {
   std::lock_guard Lock(Mutex);
-  RequestQueue.push_front(std::async(std::launch::deferred,
- std::forward(F),
-  

[PATCH] D38629: [clangd] Added a callback-based codeComplete in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

https://reviews.llvm.org/D38629

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h

Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -252,6 +252,14 @@
llvm::Optional OverridenContents = llvm::None,
IntrusiveRefCntPtr *UsedFS = nullptr);
 
+  /// A version of `codeComplete` that runs \p Callback on the processing thread
+  /// when codeComplete results become available.
+  void codeComplete(
+  UniqueFunction>)> Callback,
+  PathRef File, Position Pos,
+  llvm::Optional OverridenContents = llvm::None,
+  IntrusiveRefCntPtr *UsedFS = nullptr);
+
   /// Provide signature help for \p File at \p Pos. If \p OverridenContents is
   /// not None, they will used only for signature help, i.e. no diagnostics
   /// update will be scheduled and a draft for \p File will not be updated. If
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -48,6 +48,25 @@
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
 }
 
+template 
+std::future makeFutureAPIFromCallback(
+ClangdServer *Server,
+void (ClangdServer::*CallbackFunPtr)(UniqueFunction, Args...),
+Args... As) {
+  std::promise ResultPromise;
+  std::future ResultFuture = ResultPromise.get_future();
+
+  auto Callback = [](std::promise ResultPromise, Ret Result) -> void {
+ResultPromise.set_value(std::move(Result));
+  };
+
+  (Server->*CallbackFunPtr)(
+  BindWithForward(std::move(Callback), std::move(ResultPromise)),
+  std::forward(As)...);
+
+  return ResultFuture;
+}
+
 } // namespace
 
 size_t clangd::positionToOffset(StringRef Code, Position P) {
@@ -198,6 +217,17 @@
 ClangdServer::codeComplete(PathRef File, Position Pos,
llvm::Optional OverridenContents,
IntrusiveRefCntPtr *UsedFS) {
+  return makeFutureAPIFromCallback(this, &ClangdServer::codeComplete, File, Pos,
+   OverridenContents, UsedFS);
+}
+
+void ClangdServer::codeComplete(
+UniqueFunction>)> Callback,
+PathRef File, Position Pos, llvm::Optional OverridenContents,
+IntrusiveRefCntPtr *UsedFS) {
+  using CallbackType =
+  UniqueFunction>)>;
+
   std::string Contents;
   if (OverridenContents) {
 Contents = *OverridenContents;
@@ -216,36 +246,33 @@
   std::shared_ptr Resources = Units.getFile(File);
   assert(Resources && "Calling completion on non-added file");
 
-  using PackagedTask =
-  std::packaged_task>()>;
-
   // Remember the current Preamble and use it when async task starts executing.
   // At the point when async task starts executing, we may have a different
   // Preamble in Resources. However, we assume the Preamble that we obtain here
   // is reusable in completion more often.
   std::shared_ptr Preamble =
   Resources->getPossiblyStalePreamble();
-  // A task that will be run asynchronously.
-  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
-if (!Preamble) {
-  // Maybe we built some preamble before processing this request.
-  Preamble = Resources->getPossiblyStalePreamble();
-}
-// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
-// both the old and the new version in case only one of them matches.
-
-std::vector Result = clangd::codeComplete(
-File, Resources->getCompileCommand(),
-Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, TaggedFS.Value,
-PCHs, SnippetCompletions, Logger);
-return make_tagged(std::move(Result), std::move(TaggedFS.Tag));
-  });
 
-  auto Future = Task.get_future();
-  // FIXME(ibiryukov): to reduce overhead for wrapping the same callable
-  // multiple times, ClangdScheduler should return future<> itself.
-  WorkScheduler.addToFront([](PackagedTask Task) { Task(); }, std::move(Task));
-  return Future;
+  // A task that will be run asynchronously.
+  auto Task =
+  // 'mutable' to reassign Preamble variable.
+  [=](CallbackType Callback) mutable {
+if (!Preamble) {
+  // Maybe we built some preamble before processing this request.
+  Preamble = Resources->getPossiblyStalePreamble();
+}
+// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
+// both the old and the new version in case only one of them matches.
+
+std::vector Result = clangd::codeComplete(
+File, Resources->getCompileCommand(),
+Preamble ? &Preamble->Preamble : nullptr, Contents, Pos,
+TaggedFS.Value, PCHs, SnippetCompletions, Logger);
+
+Callback(make_tagged(std::move(Result), std::move(TaggedFS.Tag)));
+  };
+
+  WorkScheduler.addToFront(std::move(Task), std::move(Callback));
 }
 
 Tagged
_

[PATCH] D38629: [clangd] Added a callback-based codeComplete in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I hope the whole design doesn't seem overly complicated. Very much open to 
suggestions on how to simplify it :-)


https://reviews.llvm.org/D38629



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

r315074 - [OPENMP] Capture references to global variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  6 09:17:25 2017
New Revision: 315074

URL: http://llvm.org/viewvc/llvm-project?rev=315074&view=rev
Log:
[OPENMP] Capture references to global variables.

In C++11 variable to global variables are considered as constant
expressions and these variables are not captured in the outlined
regions. Patch allows capturing of such variables in the OpenMP regions.

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_private_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_private_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=315074&r1=315073&r2=315074&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Oct  6 09:17:25 2017
@@ -2297,8 +2297,12 @@ LValue CodeGenFunction::EmitDeclRefLValu
 VD->isUsableInConstantExpressions(getContext()) &&
 VD->checkInitIsICE() &&
 // Do not emit if it is private OpenMP variable.
-!(E->refersToEnclosingVariableOrCapture() && CapturedStmtInfo &&
-  LocalDeclMap.count(VD))) {
+!(E->refersToEnclosingVariableOrCapture() &&
+  ((CapturedStmtInfo &&
+(LocalDeclMap.count(VD->getCanonicalDecl()) ||
+ CapturedStmtInfo->lookup(VD->getCanonicalDecl( ||
+   LambdaCaptureFields.lookup(VD->getCanonicalDecl()) ||
+   isa(CurCodeDecl {
   llvm::Constant *Val =
 ConstantEmitter(*this).emitAbstract(E->getLocation(),
 *VD->evaluateValue(),

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=315074&r1=315073&r2=315074&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Oct  6 09:17:25 2017
@@ -14893,7 +14893,8 @@ static void DoMarkVarDeclReferenced(Sema
   IsVariableAConstantExpression(Var, SemaRef.Context)) {
 // A reference initialized by a constant expression can never be
 // odr-used, so simply ignore it.
-if (!Var->getType()->isReferenceType())
+if (!Var->getType()->isReferenceType() ||
+(SemaRef.LangOpts.OpenMP && SemaRef.IsOpenMPCapturedDecl(Var)))
   SemaRef.MaybeODRUseExprs.insert(E);
   } else if (OdrUseContext) {
 MarkVarDeclODRUsed(Var, Loc, SemaRef,

Modified: cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp?rev=315074&r1=315073&r2=315074&view=diff
==
--- cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp Fri Oct  6 09:17:25 2017
@@ -83,6 +83,7 @@ int main() {
 // LAMBDA: alloca i{{[0-9]+}},
 // LAMBDA: [[G_PRIVATE_ADDR:%.+]] = alloca i{{[0-9]+}},
 // LAMBDA: [[G1_PRIVATE_ADDR:%.+]] = alloca i{{[0-9]+}},
+// LAMBDA: [[G1_PRIVATE_REF:%.+]] = alloca i{{[0-9]+}}*,
 // LAMBDA: [[SIVAR2_PRIVATE_ADDR:%.+]] = alloca i{{[0-9]+}},
 
 // LAMBDA:  store i{{[0-9]+}}* [[SIVAR_REF]], i{{[0-9]+}}** %{{.+}},
@@ -91,20 +92,26 @@ int main() {
 
 // LAMBDA: [[G_VAL:%.+]] = load volatile i{{[0-9]+}}, i{{[0-9]+}}* [[G]]
 // LAMBDA: store i{{[0-9]+}} [[G_VAL]], i{{[0-9]+}}* [[G_PRIVATE_ADDR]]
+// LAMBDA: store i{{[0-9]+}}* [[G1_PRIVATE_ADDR]], i{{[0-9]+}}** 
[[G1_PRIVATE_REF]],
 // LAMBDA: [[SIVAR2_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* 
[[SIVAR2_PRIVATE_ADDR_REF]]
 // LAMBDA: store i{{[0-9]+}} [[SIVAR2_VAL]], i{{[0-9]+}}* 
[[SIVAR2_PRIVATE_ADDR]]
 
 // LAMBDA-NOT: call void @__kmpc_barrier(
 g = 1;
-g1 = 1;
-sivar = 2;
+g1 = 2;
+sivar = 3;
 // LAMBDA: call void @__kmpc_for_static_init_4(
 
 // LAMBDA: store i{{[0-9]+}} 1, i{{[0-9]+}}* [[G_PRIVATE_ADDR]],
-// LAMBDA: store i{{[0-9]+}} 2, i{{[0-9]+}}* [[SIVAR2_PRIVATE_ADDR]],
+// LAMBDA: [[G1_PRIVATE_ADDR:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** 
[[G1_PRIVATE_REF]],
+// LAMBDA: store volatile i{{[0-9]+}} 2, i{{[0-9]+}}* [[G1_PRIVATE_ADDR]],
+// LAMBDA: store i{{[0-9]+}} 3, i{{[0-9]+}}* [[SIVAR2_PRIVATE_ADDR]],
 // LAMBDA: [[G_PRIVATE_ADDR_REF:%.+]] = getelementptr inbounds %{{.+}}, 
%{{.+}}* [[ARG:%.+]], i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // LAMBDA: store i{{[0-9]+}}* [[G_PRIVATE_ADDR]], i{{[0-9]+}}** 
[[G_PRIVATE_ADDR_REF]]
-// LAMBDA: [[SIVAR_PRIVATE_ADDR_REF:%.+]] = getelementptr inbounds 
%{{.+}}, %{{.+}}* [[ARG:%.+]], i{{[0-9]+}} 0, i{{[0-9]+}} 1
+// LAMBDA: [[

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande added a comment.

Hi @bruno, @rsmith, does this approach look ok?

I couldn't easily figure out a better way to store inherited-default-info; 
doing it alongside the default value seemed the cleanest.

Note: I saw there are three ways to store these data inside the 
`DefaultArgStorage` struct, see here:
https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/include/clang/AST/DeclTemplate.h;315074$272

There's (1) the default arg; (2) the reference to the `Decl` having the default 
arg (inherited by this `Decl`); and (3) a pointer to a `Chain` link, which is 
the case when modules are used.

The serializer already had (1) covered, and I'm adding (2) here.  Maybe the 
most proper fix is to handle (3) as well.  Although it seems no unit tests 
broke, so this is either ok as is, or there's no test for it :)

So: right now the serialization stream generated in `ASTWriterDecl.cpp` looks 
sort of like:

   [ . . . other fields]
  bool defaultArgFollows
  [Stmt defaultArgStmtRef (if above is true)]
  bool inheritedFromDeclFollows
  [Decl inheritedFromDecl (if above is true)]

This could be changed to:

   [ . . . other fields]
  bool isPartOfChain <- add this
  bool defaultArgFollows
  [Stmt defaultArgStmtRef (if above is true)]
  bool inheritedFromDeclFollows
  [Decl inheritedFromDecl (if above is true)]

so this can store the `DefaultArgStorage` with better fidelity.  Thoughts?

Thanks!  :)


Repository:
  rL LLVM

https://reviews.llvm.org/D38320



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

r315075 - Split X86::BI__builtin_cpu_init handling into own function[NFC]

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Oct  6 09:40:45 2017
New Revision: 315075

URL: http://llvm.org/viewvc/llvm-project?rev=315075&view=rev
Log:
Split X86::BI__builtin_cpu_init handling into own function[NFC]

The Cpu Init functionality is required for the target
attribute, so this patch simply splits it out into its own
function, exactly like CpuIs and CpuSupports.

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=315075&r1=315074&r2=315075&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  6 09:40:45 2017
@@ -7708,12 +7708,21 @@ Value *CodeGenFunction::EmitX86CpuSuppor
   return Builder.CreateICmpNE(Bitset, llvm::ConstantInt::get(Int32Ty, 0));
 }
 
+Value *CodeGenFunction::EmitX86CpuInit() {
+  llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
+/*Variadic*/ false);
+  llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy, 
"__cpu_indicator_init");
+  return Builder.CreateCall(Func);
+}
+
 Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
const CallExpr *E) {
   if (BuiltinID == X86::BI__builtin_cpu_is)
 return EmitX86CpuIs(E);
   if (BuiltinID == X86::BI__builtin_cpu_supports)
 return EmitX86CpuSupports(E);
+  if (BuiltinID == X86::BI__builtin_cpu_init)
+return EmitX86CpuInit();
 
   SmallVector Ops;
 
@@ -7765,13 +7774,6 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 
   switch (BuiltinID) {
   default: return nullptr;
-  case X86::BI__builtin_cpu_init: {
-llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
-  /*Variadic*/false);
-llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy,
- "__cpu_indicator_init");
-return Builder.CreateCall(Func);
-  }
   case X86::BI_mm_prefetch: {
 Value *Address = Ops[0];
 Value *RW = ConstantInt::get(Int32Ty, 0);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=315075&r1=315074&r2=315075&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Oct  6 09:40:45 2017
@@ -3901,6 +3901,7 @@ private:
   llvm::Value *EmitX86CpuIs(StringRef CPUStr);
   llvm::Value *EmitX86CpuSupports(const CallExpr *E);
   llvm::Value *EmitX86CpuSupports(ArrayRef FeatureStrs);
+  llvm::Value *EmitX86CpuInit();
 };
 
 /// Helper class with most of the code for saving a value for a


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

[PATCH] D38628: Remove unneeded typename from test

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

FYI, there is a similar issue in  
`test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp` added 
by the 
same commit

git hash: a12318f5ae0aae44eb17f376d3598717b45f7a5f
 "Added failing tests for index out of range for tuple_element> and 
variant_alternative<>"


https://reviews.llvm.org/D38628



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

[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

Can you add a test case with shared libraries?


https://reviews.llvm.org/D38124



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

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Sema/Scope.h:129
+/// We are between inheritance colon and the real class/struct definition 
scope
+ClassInheritanceScope = 0x40,
   };

Could you please rebase this patch? There's another scope flag that uses this 
value already.


https://reviews.llvm.org/D38618



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

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8719
+  // Type limit values are handled later by CheckTautologicalComparison().
+  if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
 return;

lebedev.ri wrote:
> rsmith wrote:
> > Is this necessary? (Aren't the type limit values always within the range of 
> > the type in question?)
> > 
> > Can we avoid evaluating `Constant` a extra time here? (We already have its 
> > value in `Value`.)
> Uhm, good question :)
> If i remove this, `check-clang-sema` and `check-clang-semacxx` still pass.
> I agree that it does not make much sense. Initially it was only checking for 
> `Value == 0`.
> Git suggests that initially this branch was added by @rtrieu, maybe can help.
[[ 
https://github.com/llvm-mirror/clang/commit/526e627d2bd7e8cbf630526d315c90864898d9ff#diff-93faf32157a807b1b7953f3747db08b6R4332
 | The most original version of this code ]]
After some though i think the initial check `Value == 0` was simply to quickly 
bail out
out of `DiagnoseOutOfRangeComparison()`, and not waste any time for the obvious 
case
(`0`), which can't be out-of-range, ever. So i think the right behaviour could 
be:
1. Either simply restore the original check:
```
// 0 values are handled later by CheckTautologicalComparison().
if ((Value == 0) && (!OtherIsBooleanType))
  return;
```
And add a comment there about it
2. Or, drop it completely
3. Or, perhaps refactor `CheckTautologicalComparison()`, and more or less call 
it from
 function `AnalyzeComparison()`, before calling 
`DiagnoseOutOfRangeComparison()`,
 thus completely avoiding the need to re-evaluate `Constant` there later on,
 and simplify the logic in the process.

I personally think the `3.` *might* be best.
WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



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

r315076 - [OPENMP] Do not capture local static variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  6 10:00:28 2017
New Revision: 315076

URL: http://llvm.org/viewvc/llvm-project?rev=315076&view=rev
Log:
[OPENMP] Do not capture local static variables.

Previously we may erroneously try to capture locally declared static
variables, which will lead to crash for target-based constructs.
Patch fixes this problem.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=315076&r1=315075&r2=315076&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Oct  6 10:00:28 2017
@@ -1838,6 +1838,10 @@ public:
   if (DVar.RefExpr || !ImplicitDeclarations.insert(VD).second)
 return;
 
+  // Skip internally declared static variables.
+  if (VD->hasGlobalStorage() && !CS->capturesVariable(VD))
+return;
+
   auto ELoc = E->getExprLoc();
   auto DKind = Stack->getCurrentDirective();
   // The default(none) clause requires that each variable that is 
referenced

Modified: cfe/trunk/test/OpenMP/target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_codegen.cpp?rev=315076&r1=315075&r2=315076&view=diff
==
--- cfe/trunk/test/OpenMP/target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/target_codegen.cpp Fri Oct  6 10:00:28 2017
@@ -34,7 +34,9 @@
 // code, only 6 will have mapped arguments, and only 4 have all-constant map
 // sizes.
 
-// CHECK-DAG: [[SIZET2:@.+]] = private unnamed_addr constant [1 x i{{32|64}}] 
[i[[SZ:32|64]] 2]
+// CHECK-DAG: [[SIZET:@.+]] = private unnamed_addr constant [2 x i[[SZ]]] 
[i[[SZ]] 0, i[[SZ]] 4]
+// CHECK-DAG: [[MAPT:@.+]] = private unnamed_addr constant [2 x i32] [i32 32, 
i32 288]
+// CHECK-DAG: [[SIZET2:@.+]] = private unnamed_addr constant [1 x i{{32|64}}] 
[i[[SZ]] 2]
 // CHECK-DAG: [[MAPT2:@.+]] = private unnamed_addr constant [1 x i32] [i32 288]
 // CHECK-DAG: [[SIZET3:@.+]] = private unnamed_addr constant [2 x i[[SZ]]] 
[i[[SZ]] 4, i[[SZ]] 2]
 // CHECK-DAG: [[MAPT3:@.+]] = private unnamed_addr constant [2 x i32] [i32 
288, i32 288]
@@ -59,6 +61,7 @@
 // TCHECK: @{{.+}} = constant [[ENTTY]]
 // TCHECK: @{{.+}} = constant [[ENTTY]]
 // TCHECK: @{{.+}} = constant [[ENTTY]]
+// TCHECK: @{{.+}} = {{.*}}constant [[ENTTY]]
 // TCHECK-NOT: @{{.+}} = constant [[ENTTY]]
 
 // Check if offloading descriptor is created.
@@ -91,6 +94,7 @@ int foo(int n) {
   double c[5][10];
   double cn[5][n];
   TT d;
+  static long *plocal;
 
   // CHECK:   [[ADD:%.+]] = add nsw i32
   // CHECK:   store i32 [[ADD]], i32* [[CAPTURE:%.+]],
@@ -106,6 +110,39 @@ int foo(int n) {
   {
   }
 
+  // CHECK:   [[ADD:%.+]] = add nsw i32
+  // CHECK:   store i32 [[ADD]], i32* [[CAPTURE:%.+]],
+  // CHECK-DAG:   [[LD:%.+]] = load i32, i32* [[CAPTURE]],
+  // CHECK-DAG:   [[RET:%.+]] = call i32 @__tgt_target(i32 [[LD]], i8* 
@{{[^,]+}}, i32 2, i8** [[BPR:%[^,]+]], i8** [[PR:%[^,]+]], i[[SZ]]* 
getelementptr inbounds ([2 x i[[SZ]]], [2 x i[[SZ]]]* [[SIZET]], i32 0, i32 0), 
i32* getelementptr inbounds ([2 x i32], [2 x i32]* [[MAPT]], i32 0, i32 0)
+  // CHECK-DAG:   [[BPR]] = getelementptr inbounds [2 x i8*], [2 x i8*]* 
[[BP:%[^,]+]], i32 0, i32 0
+  // CHECK-DAG:   [[PR]] = getelementptr inbounds [2 x i8*], [2 x i8*]* 
[[P:%[^,]+]], i32 0, i32 0
+
+  // CHECK-DAG:   [[BPADDR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x 
i8*]* [[BP]], i32 0, i32 0
+  // CHECK-DAG:   [[PADDR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x 
i8*]* [[P]], i32 0, i32 0
+  // CHECK-DAG:   [[CBPADDR0:%.+]] = bitcast i8** [[BPADDR0]] to i[[SZ]]**
+  // CHECK-DAG:   [[CPADDR0:%.+]] = bitcast i8** [[PADDR0]] to i[[SZ]]**
+  // CHECK-DAG:   store i[[SZ]]* [[BP0:%[^,]+]], i[[SZ]]** [[CBPADDR0]]
+  // CHECK-DAG:   store i[[SZ]]* [[BP0]], i[[SZ]]** [[CPADDR0]]
+
+  // CHECK-DAG:   [[BPADDR1:%.+]] = getelementptr inbounds [2 x i8*], [2 x 
i8*]* [[BP]], i32 0, i32 1
+  // CHECK-DAG:   [[PADDR1:%.+]] = getelementptr inbounds [2 x i8*], [2 x 
i8*]* [[P]], i32 0, i32 1
+  // CHECK-DAG:   [[CBPADDR1:%.+]] = bitcast i8** [[BPADDR1]] to i[[SZ]]*
+  // CHECK-DAG:   [[CPADDR1:%.+]] = bitcast i8** [[PADDR1]] to i[[SZ]]*
+  // CHECK-DAG:   store i[[SZ]] [[BP1:%[^,]+]], i[[SZ]]* [[CBPADDR1]]
+  // CHECK-DAG:   store i[[SZ]] [[BP1]], i[[SZ]]* [[CPADDR1]]
+  // CHECK:   [[ERROR:%.+]] = icmp ne i32 [[RET]], 0
+  // CHECK-NEXT:  br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
+  // CHECK:   [[FAIL]]
+  // CHECK:   call void [[HVT0_:@.+]](i[[SZ]]* [[BP0]], i[[SZ]] [[BP1]])
+  // CHECK-NEXT:  br label %[[END]]
+  // CHECK:   [[END]]
+  #pragma omp target device(global + a)
+  {
+static int local1;
+*plocal = global;
+local1 = global;
+  }
+
   // CHECK:  

r315078 - For Windows, allow .exe extension in a test.

[Paul Robinson via cfe-commits]

Author: probinson
Date: Fri Oct  6 10:12:28 2017
New Revision: 315078

URL: http://llvm.org/viewvc/llvm-project?rev=315078&view=rev
Log:
For Windows, allow .exe extension in a test.

Modified:
cfe/trunk/test/Driver/baremetal.cpp

Modified: cfe/trunk/test/Driver/baremetal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/baremetal.cpp?rev=315078&r1=315077&r2=315078&view=diff
==
--- cfe/trunk/test/Driver/baremetal.cpp (original)
+++ cfe/trunk/test/Driver/baremetal.cpp Fri Oct  6 10:12:28 2017
@@ -10,7 +10,7 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" 
"[[SYSROOT]]{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
-// CHECK-V6M-C-NEXT: "{{[^"]*}}ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" 
"-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
 // CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -32,7 +32,7 @@
 // RUN: -target armv6m-none-eabi \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
-// CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -45,7 +45,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
 // CHECK-V6M-LIBCXX-NOT: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}{{[^v].*}}"
 // CHECK-V6M-LIBCXX: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECK-V6M-LIBCXX: "{{[^"]*}}ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
 // CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -58,7 +58,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
 // CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-V6M-LIBSTDCXX: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
-// CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBSTDCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -69,7 +69,7 @@
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -nodefaultlibs \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-NDL %s
-// CHECK-V6M-NDL: "{{[^"]*}}ld.lld" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-NDL: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-NDL-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 "-o" "{{.*}}.o"
 
 // RUN: %clangxx -target arm-none-eabi -v 2>&1 \


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

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: test/CodeCompletion/call.cpp:22
   // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
-  // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+  // CHECK-CC1: f(Y y, <#int ZZ#>)
   // CHECK-CC1-NEXT: f(int i, <#int j#>, int k)

Could we also test a similar class to `Y` that's not typedefed, so you'd see 
`f(N::Y)`?



Comment at: test/CodeCompletion/enum-switch-case-qualified.cpp:25
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
-// CHECK-CC1: Blue : [#M::N::C::Color#]N::C::Blue
-// CHECK-CC1-NEXT: Green : [#M::N::C::Color#]N::C::Green
-// CHECK-CC1-NEXT: Indigo : [#M::N::C::Color#]N::C::Indigo
-// CHECK-CC1-NEXT: Orange : [#M::N::C::Color#]N::C::Orange
-// CHECK-CC1-NEXT: Red : [#M::N::C::Color#]N::C::Red
-// CHECK-CC1-NEXT: Violet : [#M::N::C::Color#]N::C::Violet
-// CHECK-CC1: Yellow : [#M::N::C::Color#]N::C::Yellow
+// CHECK-CC1: Blue : [#Color#]N::C::Blue
+// CHECK-CC1-NEXT: Green : [#Color#]N::C::Green

ilya-biryukov wrote:
> This may be a somewhat unwanted part of this change.
> Enum type is now written without qualifier here. I would argue that's ok, 
> since the actual enum values are always properly qualified (they have to be, 
> as they are actually inserted by completion) and those qualifiers provide all 
> the necessary context for the user.
I'm not 100% comfortable with making this kind of change right now. I'll try to 
investigate what's best for our users.


https://reviews.llvm.org/D38538



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

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D35082#890359, @Anastasia wrote:

> In https://reviews.llvm.org/D35082#890162, @rjmccall wrote:
>
> > Okay.  I think I see your point about why it would be better to have a 
> > canonical __private address space that is different from the implicit 
> > address space 0.  I assume this means that there should basically never be 
> > pointers, references, or l-values in address space 0 in OpenCL.
>
>
> If you mean 0 is `private` address space then no. There can be private AS 
> pointers. But if you mean 0 is no address space then this doesn't exist in 
> OpenCL. I think the right design in the first place  would have been to keep 
> address spaces in AST just as they are in the source code and add explicit 
> address spaces to all types in IR instead. In this case absence of any 
> address spaces in AST would signal implicit AS to be used. This would make 
> some Sema checks a bit more complicated though, but the design would become a 
> lot cleaner. Unfortunately I think it would not be worth doing this big 
> change now.


My understanding is that that is exactly what the majority of this patch is 
trying to achieve: making __private its own address space, separate from 
address space 0.  The patch is certainly introducing a new address-space 
enumerator for __private; if that's meant to be treated as canonically equal to 
address space 0, well, (1) this patch doesn't seem to add any of the APIs that 
I'd expect would be required to make that work, like a way to canonicalize an 
address space or ask if two address spaces are canonically the same, and (2) 
that would be a major new complexity in the representation that we wouldn't 
welcome anyway.

The patch is also tracking explicitness of address spaces in the non-canonical 
type system, but the majority of the patch is spent dealing with the 
consequences of splitting __private away from address space 0.

Yaxun has convinced me that it's important to represent a __private-qualified 
type differently from an unqualified type.(*)  It seems that you agree, but 
you're concerned about how that will affect your schedule.  I can't speak to 
your schedule; it seems to me that you and Yaxun need to figure that out 
privately.  If you decide not to separate them yet, then the majority of this 
patch needs to be put on hold.  If you do decide to separate them, it seems to 
me that we should probably split the change to __private out from the 
introduction of implicit address spaces.  Just let me know what you want to do.

(*) I know that you aren't considering OpenCL C++ yet, but often these 
representation/model questions are easier to answer when thinking about C++ 
instead of C because type differences are so much more directly important in 
C++.  In OpenCL C++, I assume it will be important to distinguish between  
and <__private int> as template arguments.  That tells us straight up that 
__private needs to be represented differently from a lack of qualifier.  Note 
that the result, where certain type representations (e.g. an unqualified int*) 
become basically meaningless and should never be constructed by Sema, is 
already precedented in Clang: ObjC ARC does the same thing to unqualified ObjC 
pointer types, like id*.


https://reviews.llvm.org/D35082



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

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D36918



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

r315084 - Run pyformat on lit code.

[Zachary Turner via cfe-commits]

Author: zturner
Date: Fri Oct  6 10:54:27 2017
New Revision: 315084

URL: http://llvm.org/viewvc/llvm-project?rev=315084&view=rev
Log:
Run pyformat on lit code.

Modified:
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=315084&r1=315083&r2=315084&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Fri Oct  6 10:54:27 2017
@@ -24,7 +24,8 @@ config.name = 'Clang'
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.cppm', '.m', '.mm', '.cu', '.ll', '.cl', 
'.s', '.S', '.modulemap', '.test', '.rs']
+config.suffixes = ['.c', '.cpp', '.cppm', '.m', '.mm', '.cu',
+   '.ll', '.cl', '.s', '.S', '.modulemap', '.test', '.rs']
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
@@ -65,15 +66,19 @@ if platform.system() != 'Windows':
 llvm_config.clear_environment(possibly_dangerous_env_vars)
 
 # Tweak the PATH to include the tools dir and the scripts dir.
-llvm_config.with_environment('PATH', [config.llvm_tools_dir, 
config.clang_tools_dir], append_path=True)
+llvm_config.with_environment(
+'PATH', [config.llvm_tools_dir, config.clang_tools_dir], append_path=True)
 
-llvm_config.with_environment('LD_LIBRARY_PATH', [config.llvm_shlib_dir, 
config.llvm_libs_dir], append_path=True)
+llvm_config.with_environment('LD_LIBRARY_PATH', [
+ config.llvm_shlib_dir, config.llvm_libs_dir], 
append_path=True)
 
 # Propagate path to symbolizer for ASan/MSan.
-llvm_config.with_system_environment(['ASAN_SYMBOLIZER_PATH', 
'MSAN_SYMBOLIZER_PATH'])
+llvm_config.with_system_environment(
+['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
 
 # Discover the 'clang' and 'clangcc' to use.
 
+
 def inferClang(PATH):
 # Determine which clang to use.
 clang = os.getenv('CLANG')
@@ -88,10 +93,11 @@ def inferClang(PATH):
 
 if not clang:
 lit_config.fatal("couldn't find 'clang' program, try setting "
- "CLANG in your environment")
+ 'CLANG in your environment')
 
 return clang
 
+
 config.clang = inferClang(config.environment['PATH']).replace('\\', '/')
 if not lit_config.quiet:
 lit_config.note('using clang: %r' % config.clang)
@@ -106,73 +112,76 @@ else:
 if has_plugins and config.llvm_plugin_ext:
 config.available_features.add('plugins')
 
-config.substitutions.append( ('%llvmshlibdir', config.llvm_shlib_dir) )
-config.substitutions.append( ('%pluginext', config.llvm_plugin_ext) )
-config.substitutions.append( ('%PATH%', config.environment['PATH']) )
+config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+config.substitutions.append(('%PATH%', config.environment['PATH']))
 
 if config.clang_examples:
 config.available_features.add('examples')
 
 builtin_include_dir = llvm_config.get_clang_builtin_include_dir(config.clang)
-config.substitutions.append( ('%clang_analyze_cc1',
-  '%clang_cc1 -analyze %analyze') )
-config.substitutions.append( ('%clang_cc1',
-  '%s -cc1 -internal-isystem %s -nostdsysteminc'
-  % (config.clang, builtin_include_dir)) )
-config.substitutions.append( ('%clang_cpp', ' ' + config.clang +
-  ' --driver-mode=cpp '))
-config.substitutions.append( ('%clang_cl', ' ' + config.clang +
-  ' --driver-mode=cl '))
-config.substitutions.append( ('%clangxx', ' ' + config.clang +
-  ' --driver-mode=g++ '))
+config.substitutions.append(('%clang_analyze_cc1',
+ '%clang_cc1 -analyze %analyze'))
+config.substitutions.append(('%clang_cc1',
+ '%s -cc1 -internal-isystem %s -nostdsysteminc'
+ % (config.clang, builtin_include_dir)))
+config.substitutions.append(('%clang_cpp', ' ' + config.clang +
+ ' --driver-mode=cpp '))
+config.substitutions.append(('%clang_cl', ' ' + config.clang +
+ ' --driver-mode=cl '))
+config.substitutions.append(('%clangxx', ' ' + config.clang +
+ ' --driver-mode=g++ '))
 
-clang_func_map = lit.util.which('clang-func-mapping', 
config.environment['PATH'])
+clang_func_map = lit.util.which(
+'clang-func-mapping', config.environment['PATH'])
 if clang_func_map:
-config.substitutions.append( ('%clang_func_map', ' ' + clang_func_map + ' 
') )
+config.substitutions.append(
+('%clang_func_map', ' ' + clang_func_map + ' '))
 
-config.substitut

r315085 - [lit] Improve tool substitution in lit.

[Zachary Turner via cfe-commits]

Author: zturner
Date: Fri Oct  6 10:54:46 2017
New Revision: 315085

URL: http://llvm.org/viewvc/llvm-project?rev=315085&view=rev
Log:
[lit] Improve tool substitution in lit.

This addresses two sources of inconsistency in test configuration
files.

1. Substitution boundaries.  Previously you would specify a
   substitution, such as 'lli', and then additionally a set
   of characters that should fail to match before and after
   the tool.  This was used, for example, so that matches that
   are parts of full paths would not be replaced.  But not all
   tools did this, and those that did would often re-invent
   the set of characters themselves, leading to inconsistency.
   Now, every tool substitution defaults to using a sane set
   of reasonable defaults and you have to explicitly opt out
   of it.  This actually fixed a few latent bugs that were
   never being surfaced, but only on accident.

2. There was no standard way for the system to decide how to
   locate a tool.  Sometimes you have an explicit path, sometimes
   we would search for it and build up a path ourselves, and
   sometimes we would build up a full command line.  Furthermore,
   there was no standardized way to handle missing tools.  Do we
   warn, fail, ignore, etc?  All of this is now encapsulated in
   the ToolSubst class.  You either specify an exact command to
   run, or an instance of FindTool('') and everything
   else just works.  Furthermore, you can specify an action to
   take if the tool cannot be resolved.

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

Modified:
cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c?rev=315085&r1=315084&r2=315085&view=diff
==
--- cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c (original)
+++ cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c Fri Oct  6 10:54:46 
2017
@@ -1,4 +1,4 @@
-// RUN: not %clang-cc1 -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s
 
 // IMPORTANT: This test case intentionally DOES NOT use --disable-free.  It
 // tests that we are properly reclaiming the ASTs and we do not have a double 
free.

Modified: cfe/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=315085&r1=315084&r2=315085&view=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Fri Oct  6 10:54:46 2017
@@ -10,7 +10,8 @@ import lit.formats
 import lit.util
 
 from lit.llvm import llvm_config
-from lit.llvm import ToolFilter
+from lit.llvm.subst import ToolSubst
+from lit.llvm.subst import FindTool
 
 # Configuration file for the 'lit' test runner.
 
@@ -76,6 +77,8 @@ llvm_config.with_environment('LD_LIBRARY
 llvm_config.with_system_environment(
 ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
 
+llvm_config.use_default_substitutions()
+
 # Discover the 'clang' and 'clangcc' to use.
 
 
@@ -120,44 +123,37 @@ if config.clang_examples:
 config.available_features.add('examples')
 
 builtin_include_dir = llvm_config.get_clang_builtin_include_dir(config.clang)
-config.substitutions.append(('%clang_analyze_cc1',
- '%clang_cc1 -analyze %analyze'))
-config.substitutions.append(('%clang_cc1',
- '%s -cc1 -internal-isystem %s -nostdsysteminc'
- % (config.clang, builtin_include_dir)))
-config.substitutions.append(('%clang_cpp', ' ' + config.clang +
- ' --driver-mode=cpp '))
-config.substitutions.append(('%clang_cl', ' ' + config.clang +
- ' --driver-mode=cl '))
-config.substitutions.append(('%clangxx', ' ' + config.clang +
- ' --driver-mode=g++ '))
-
-clang_func_map = lit.util.which(
-'clang-func-mapping', config.environment['PATH'])
-if clang_func_map:
-config.substitutions.append(
-('%clang_func_map', ' ' + clang_func_map + ' '))
-
-config.substitutions.append(('%clang', ' ' + config.clang + ' '))
-config.substitutions.append(('%test_debuginfo',
- ' ' + config.llvm_src_root + 
'/utils/test_debuginfo.pl '))
-config.substitutions.append(('%itanium_abi_triple',
- 
llvm_config.make_itanium_abi_triple(config.target_triple)))
-config.substitutions.append(('%ms_abi_triple',
- 
llvm_config.make_msabi_triple(config.target_triple)))
-config.substitutions.append(('%resource_dir', builtin_include_dir))
-config.substitutions.append(('%python', config.python_executable))
 
-# The host triple might not be set, at least if we're compiling clang from
-# an already installed llvm.
-if config.ho

[PATCH] D38402: [clang-refactor] Apply source replacements

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: test/Refactor/tool-apply-replacements.cpp:6
+
+class RenameMe { // CHECK: class {
+};

ioeric wrote:
> Why is the result `class {`?
Sorry, the test was broken. Fixed!


Repository:
  rL LLVM

https://reviews.llvm.org/D38402



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

[PATCH] D38402: [clang-refactor] Apply source replacements

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 118042.
arphaman marked 5 inline comments as done.
arphaman added a comment.

Address review comments


Repository:
  rL LLVM

https://reviews.llvm.org/D38402

Files:
  include/clang/Frontend/CommandLineSourceLoc.h
  test/Refactor/tool-apply-replacements.cpp
  test/Refactor/tool-selection-option.c
  tools/clang-refactor/ClangRefactor.cpp

Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -14,6 +14,7 @@
 //===--===//
 
 #include "TestSupport.h"
+#include "clang/Frontend/CommandLineSourceLoc.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
@@ -54,7 +55,7 @@
 
   /// Prints any additional state associated with the selection argument to
   /// the given output stream.
-  virtual void print(raw_ostream &OS) = 0;
+  virtual void print(raw_ostream &OS) {}
 
   /// Returns a replacement refactoring result consumer (if any) that should
   /// consume the results of a refactoring operation.
@@ -99,6 +100,41 @@
   TestSelectionRangesInFile TestSelections;
 };
 
+/// Stores the parsed -selection=filename:line:column[-line:column] option.
+class SourceRangeSelectionArgument final : public SourceSelectionArgument {
+public:
+  SourceRangeSelectionArgument(ParsedSourceRange Range)
+  : Range(std::move(Range)) {}
+
+  bool forAllRanges(const SourceManager &SM,
+llvm::function_ref Callback) override {
+const FileEntry *FE = SM.getFileManager().getFile(Range.FileName);
+FileID FID = FE ? SM.translateFile(FE) : FileID();
+if (!FE || FID.isInvalid()) {
+  llvm::errs() << "error: -selection=" << Range.FileName
+   << ":... : given file is not in the target TU\n";
+  return true;
+}
+
+SourceLocation Start = SM.getMacroArgExpandedLocation(
+SM.translateLineCol(FID, Range.Begin.first, Range.Begin.second));
+SourceLocation End = SM.getMacroArgExpandedLocation(
+SM.translateLineCol(FID, Range.End.first, Range.End.second));
+if (Start.isInvalid() || End.isInvalid()) {
+  llvm::errs() << "error: -selection=" << Range.FileName << ':'
+   << Range.Begin.first << ':' << Range.Begin.second << '-'
+   << Range.End.first << ':' << Range.End.second
+   << " : invalid source location\n";
+  return true;
+}
+Callback(SourceRange(Start, End));
+return false;
+  }
+
+private:
+  ParsedSourceRange Range;
+};
+
 std::unique_ptr
 SourceSelectionArgument::fromString(StringRef Value) {
   if (Value.startswith("test:")) {
@@ -110,10 +146,12 @@
 return llvm::make_unique(
 std::move(*ParsedTestSelection));
   }
-  // FIXME: Support true selection ranges.
+  Optional Range = ParsedSourceRange::fromString(Value);
+  if (Range)
+return llvm::make_unique(std::move(*Range));
   llvm::errs() << "error: '-selection' option must be specified using "
   ":: or "
-  "::-: format";
+  "::-: format\n";
   return nullptr;
 }
 
@@ -272,7 +310,14 @@
 llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
 
-  // FIXME: Consume atomic changes and apply them to files.
+  void handle(AtomicChanges Changes) {
+SourceChanges.insert(SourceChanges.begin(), Changes.begin(), Changes.end());
+  }
+
+  const AtomicChanges &getSourceChanges() const { return SourceChanges; }
+
+private:
+  AtomicChanges SourceChanges;
 };
 
 class ClangRefactorTool {
@@ -352,6 +397,39 @@
 }
   }
 
+  bool applySourceChanges(const AtomicChanges &Replacements) {
+std::set Files;
+for (const auto &Change : Replacements)
+  Files.insert(Change.getFilePath());
+// FIXME: Add automatic formatting support as well.
+tooling::ApplyChangesSpec Spec;
+// FIXME: We should probably cleanup the result by default as well.
+Spec.Cleanup = false;
+for (const auto &File : Files) {
+  llvm::ErrorOr> BufferErr =
+  llvm::MemoryBuffer::getFile(File);
+  if (!BufferErr) {
+llvm::errs() << "error: failed to open" << File << " for rewriting\n";
+return true;
+  }
+  auto Result = tooling::applyAtomicChanges(File, (*BufferErr)->getBuffer(),
+Replacements, Spec);
+  if (!Result) {
+llvm::errs() << toString(Result.takeError());
+return true;
+  }
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
+  if (EC) {
+llvm::errs() << EC.message() << "\n";
+return true;
+  }
+  OS << *Result;
+}
+return false;
+  }
+
   bool invokeAction(RefactoringActionSubcommand &Subcommand,
 const CompilationDatabase &DB,

[PATCH] D38639: [clangd] #include statements support for Open definition

[William Enright via Phabricator via cfe-commits]

Nebiroth created this revision.

ctrl-clicking on #include statements now opens the file being pointed by that 
statement.


https://reviews.llvm.org/D38639

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -7,7 +7,6 @@
 //
 //===--===//
 
-#include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "Logger.h"
 #include "clang/Basic/VirtualFileSystem.h"
@@ -900,82 +899,55 @@
   }
 }
 
-TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) {
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
-  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  ClangdServer Server(CDB, DiagConsumer, FS, 0,
   /*SnippetCompletions=*/false, EmptyLogger::getInstance());
 
-  auto SourceContents = R"cpp(
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  const auto SourceContents = R"cpp(
   #include "foo.h"
+  #include "invalid.h"
   int b = a;
   )cpp";
-
-  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  FS.Files[FooCpp] = SourceContents;
   auto FooH = getVirtualTestFilePath("foo.h");
-  auto Invalid = getVirtualTestFilePath("main.cpp");
+  const auto HeaderContents = "int a;";
 
   FS.Files[FooCpp] = SourceContents;
-  FS.Files[FooH] = "int a;";
-  FS.Files[Invalid] = "int main() { \n return 0; \n }";
+  FS.Files[FooH] = HeaderContents;
 
-  llvm::Optional PathResult = Server.switchSourceHeader(FooCpp);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooH);
-
-  PathResult = Server.switchSourceHeader(FooH);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooCpp);
+  Server.addDocument(FooH, HeaderContents);
+  Server.addDocument(FooCpp, SourceContents);
 
-  SourceContents = R"c(
-  #include "foo.HH"
-  int b = a;
-  )c";
+  Position P = Position{1, 11};
 
-  // Test with header file in capital letters and different extension, source
-  // file with different extension
-  auto FooC = getVirtualTestFilePath("bar.c");
-  auto FooHH = getVirtualTestFilePath("bar.HH");
-
-  FS.Files[FooC] = SourceContents;
-  FS.Files[FooHH] = "int a;";
-
-  PathResult = Server.switchSourceHeader(FooC);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooHH);
-
-  // Test with both capital letters
-  auto Foo2C = getVirtualTestFilePath("foo2.C");
-  auto Foo2HH = getVirtualTestFilePath("foo2.HH");
-  FS.Files[Foo2C] = SourceContents;
-  FS.Files[Foo2HH] = "int a;";
-
-  PathResult = Server.switchSourceHeader(Foo2C);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), Foo2HH);
-
-  // Test with source file as capital letter and .hxx header file
-  auto Foo3C = getVirtualTestFilePath("foo3.C");
-  auto Foo3HXX = getVirtualTestFilePath("foo3.hxx");
+  std::vector Locations = (Server.findDefinitions(FooCpp, P)).Value;
+  EXPECT_TRUE(!Locations.empty());
+  std::string s("file:///");
+  std::string check = URI::unparse(Locations[0].uri);
+  check = check.erase(0, s.size());
+  check = check.substr(0, check.size() - 1);
+  ASSERT_EQ(check, FooH);
+  ASSERT_EQ(Locations[0].range.start.line, 0);
+  ASSERT_EQ(Locations[0].range.start.character, 0);
+  ASSERT_EQ(Locations[0].range.end.line, 0);
+  ASSERT_EQ(Locations[0].range.end.character, 0);
+
+  // Test ctrl-clicking on the #include part on the statement
+  Position P3 = Position{1, 3};
 
-  SourceContents = R"c(
-  #include "foo3.hxx"
-  int b = a;
-  )c";
+  Locations = (Server.findDefinitions(FooCpp, P3)).Value;
+  EXPECT_TRUE(!Locations.empty());
 
-  FS.Files[Foo3C] = SourceContents;
-  FS.Files[Foo3HXX] = "int a;";
+  // Test invalid include
+  Position P2 = Position{2, 11};
 
-  PathResult = Server.switchSourceHeader(Foo3C);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), Foo3HXX);
-
-  // Test if asking for a corresponding file that doesn't exist returns an empty
-  // string.
-  PathResult = Server.switchSourceHeader(Invalid);
-  EXPECT_FALSE(PathResult.hasValue());
+  Locations = (Server.findDefinitions(FooCpp, P2)).Value;
+  EXPECT_TRUE(Locations.empty());
 }
 
 TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -127,11 +127,13 @@
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
std::vector TopLevelDeclIDs,
-   std::vector Diags);
+   std::vector Diags,
+   std::map IncludeMap);
 
   PrecompiledPreamble Preamble;
   std::vector TopLevelDeclIDs;
   std::

r315087 - [refactor] add support for refactoring options

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Oct  6 11:12:29 2017
New Revision: 315087

URL: http://llvm.org/viewvc/llvm-project?rev=315087&view=rev
Log:
[refactor] add support for refactoring options

This commit adds initial support for refactoring options. One can now use
optional and required std::string options.

This commit also adds a NewNameOption for the local-rename refactoring action to
allow rename to work with custom names.

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

Added:
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOption.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptionVisitor.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptions.h
Modified:
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
cfe/trunk/test/Refactor/LocalRename/Field.cpp
cfe/trunk/test/Refactor/tool-test-support.c
cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h?rev=315087&r1=315086&r2=315087&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h 
(original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h Fri Oct 
 6 11:12:29 2017
@@ -16,6 +16,7 @@
 namespace clang {
 namespace tooling {
 
+class RefactoringOptionVisitor;
 class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
@@ -43,6 +44,14 @@ public:
   /// Returns true when the rule has a source selection requirement that has
   /// to be fullfilled before refactoring can be performed.
   virtual bool hasSelectionRequirement() = 0;
+
+  /// Traverses each refactoring option used by the rule and invokes the
+  /// \c visit callback in the consumer for each option.
+  ///
+  /// Options are visited in the order of use, e.g. if a rule has two
+  /// requirements that use options, the options from the first requirement
+  /// are visited before the options in the second requirement.
+  virtual void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) = 0;
 };
 
 } // end namespace tooling

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h?rev=315087&r1=315086&r2=315087&view=diff
==
--- 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h 
(original)
+++ 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h 
Fri Oct  6 11:12:29 2017
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 
 #include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringOption.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include 
@@ -53,6 +54,45 @@ public:
   }
 };
 
+/// A base class for any requirement that requires some refactoring options.
+class RefactoringOptionsRequirement : public RefactoringActionRuleRequirement {
+public:
+  virtual ~RefactoringOptionsRequirement() {}
+
+  /// Returns the set of refactoring options that are used when evaluating this
+  /// requirement.
+  virtual ArrayRef>
+  getRefactoringOptions() const = 0;
+};
+
+/// A requirement that evaluates to the value of the given \c OptionType when
+/// the \c OptionType is a required option. When the \c OptionType is an
+/// optional option, the requirement will evaluate to \c None if the option is
+/// not specified or to an appropriate value otherwise.
+template 
+class OptionRequirement : public RefactoringOptionsRequirement {
+public:
+  OptionRequirement() : Opt(createRefactoringOption()) {}
+
+  ArrayRef>
+  getRefactoringOptions() const final override {
+return static_cast &>(Opt);
+  }
+
+  Expected
+  evaluate(RefactoringRuleContext &) const {
+return Opt->getValue();
+  }
+
+private:
+  /// The partially-owned option.
+  ///
+  /// The ownership of the option is shared among the different requirements
+  /// because the same option can be used by multiple rules in one refactoring
+  /// action.
+  std::shared_ptr Opt;
+};
+
 } // end namespace tooling
 } // end namespace clang
 

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h?rev=315087&r1=315086&r2=3

[PATCH] D37856: [refactor] add support for refactoring options

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arphaman marked 3 inline comments as done.
Closed by commit rL315087: [refactor] add support for refactoring options 
(authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D37856?vs=117124&id=118044#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37856

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
  
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOption.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptionVisitor.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptions.h
  cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  cfe/trunk/test/Refactor/LocalRename/Field.cpp
  cfe/trunk/test/Refactor/tool-test-support.c
  cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Index: cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
===
--- cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
+++ cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
@@ -18,6 +18,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
+#include "clang/Tooling/Refactoring/RefactoringOptions.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
@@ -32,10 +33,10 @@
 
 namespace opts {
 
-static cl::OptionCategory CommonRefactorOptions("Common refactoring options");
+static cl::OptionCategory CommonRefactorOptions("Refactoring options");
 
 static cl::opt Verbose("v", cl::desc("Use verbose output"),
- cl::cat(CommonRefactorOptions),
+ cl::cat(cl::GeneralCategory),
  cl::sub(*cl::AllSubCommands));
 } // end namespace opts
 
@@ -116,6 +117,92 @@
   return nullptr;
 }
 
+/// A container that stores the command-line options used by a single
+/// refactoring option.
+class RefactoringActionCommandLineOptions {
+public:
+  void addStringOption(const RefactoringOption &Option,
+   std::unique_ptr> CLOption) {
+StringOptions[&Option] = std::move(CLOption);
+  }
+
+  const cl::opt &
+  getStringOption(const RefactoringOption &Opt) const {
+auto It = StringOptions.find(&Opt);
+return *It->second;
+  }
+
+private:
+  llvm::DenseMap>>
+  StringOptions;
+};
+
+/// Passes the command-line option values to the options used by a single
+/// refactoring action rule.
+class CommandLineRefactoringOptionVisitor final
+: public RefactoringOptionVisitor {
+public:
+  CommandLineRefactoringOptionVisitor(
+  const RefactoringActionCommandLineOptions &Options)
+  : Options(Options) {}
+
+  void visit(const RefactoringOption &Opt,
+ Optional &Value) override {
+const cl::opt &CLOpt = Options.getStringOption(Opt);
+if (!CLOpt.getValue().empty()) {
+  Value = CLOpt.getValue();
+  return;
+}
+Value = None;
+if (Opt.isRequired())
+  MissingRequiredOptions.push_back(&Opt);
+  }
+
+  ArrayRef getMissingRequiredOptions() const {
+return MissingRequiredOptions;
+  }
+
+private:
+  llvm::SmallVector MissingRequiredOptions;
+  const RefactoringActionCommandLineOptions &Options;
+};
+
+/// Creates the refactoring options used by all the rules in a single
+/// refactoring action.
+class CommandLineRefactoringOptionCreator final
+: public RefactoringOptionVisitor {
+public:
+  CommandLineRefactoringOptionCreator(
+  cl::OptionCategory &Category, cl::SubCommand &Subcommand,
+  RefactoringActionCommandLineOptions &Options)
+  : Category(Category), Subcommand(Subcommand), Options(Options) {}
+
+  void visit(const RefactoringOption &Opt, Optional &) override {
+if (Visited.insert(&Opt).second)
+  Options.addStringOption(Opt, create(Opt));
+  }
+
+private:
+  template 
+  std::unique_ptr> create(const RefactoringOption &Opt) {
+if (!OptionNames.insert(Opt.getName()).second)
+  llvm::report_fatal_error("Multiple identical refactoring options "
+   "specified for one refactoring action");
+// FIXME: cl::Required can be specified when this option is present
+// in all rules in an action.
+return llvm::make_unique>(
+Opt.getName(), cl::desc(Opt.getDescription()), cl::Optional,
+cl::cat(Category), cl::sub(Subcommand));
+  }
+
+  llvm::SmallPtrSet Visited;
+  llvm::StringSet<> OptionNames;
+  cl::OptionCategory &Category;
+  cl::SubCommand &Subcommand;
+  RefactoringActionCommandLineOptions &Options;
+};
+
 /// A subcommand that corresponds to individual refactoring action.
 class RefactoringActio

[PATCH] D37856: [refactor] add support for refactoring options

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:73
+template 
+class OptionRequirement : public RefactoringOptionsRequirement {
+public:

ioeric wrote:
> nit: `OptionRequirement` sounds more general than the base class 
> `RefactoringOptionsRequirement`. 
I couldn't really think of a better name, sorry.


Repository:
  rL LLVM

https://reviews.llvm.org/D37856



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

[PATCH] D38639: [clangd] #include statements support for Open definition

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 118046.
Nebiroth added a comment.

Fixed accidental removal of CheckSourceHeaderSwitch test


https://reviews.llvm.org/D38639

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -7,7 +7,6 @@
 //
 //===--===//
 
-#include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "Logger.h"
 #include "clang/Basic/VirtualFileSystem.h"
@@ -978,6 +977,57 @@
   EXPECT_FALSE(PathResult.hasValue());
 }
 
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
+
+  ClangdServer Server(CDB, DiagConsumer, FS, 0,
+  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  const auto SourceContents = R"cpp(
+  #include "foo.h"
+  #include "invalid.h"
+  int b = a;
+  )cpp";
+  FS.Files[FooCpp] = SourceContents;
+  auto FooH = getVirtualTestFilePath("foo.h");
+  const auto HeaderContents = "int a;";
+
+  FS.Files[FooCpp] = SourceContents;
+  FS.Files[FooH] = HeaderContents;
+
+  Server.addDocument(FooH, HeaderContents);
+  Server.addDocument(FooCpp, SourceContents);
+
+  Position P = Position{1, 11};
+
+  std::vector Locations = (Server.findDefinitions(FooCpp, P)).Value;
+  EXPECT_TRUE(!Locations.empty());
+  std::string s("file:///");
+  std::string check = URI::unparse(Locations[0].uri);
+  check = check.erase(0, s.size());
+  check = check.substr(0, check.size() - 1);
+  ASSERT_EQ(check, FooH);
+  ASSERT_EQ(Locations[0].range.start.line, 0);
+  ASSERT_EQ(Locations[0].range.start.character, 0);
+  ASSERT_EQ(Locations[0].range.end.line, 0);
+  ASSERT_EQ(Locations[0].range.end.character, 0);
+
+  // Test ctrl-clicking on the #include part on the statement
+  Position P3 = Position{1, 3};
+
+  Locations = (Server.findDefinitions(FooCpp, P3)).Value;
+  EXPECT_TRUE(!Locations.empty());
+
+  // Test invalid include
+  Position P2 = Position{2, 11};
+
+  Locations = (Server.findDefinitions(FooCpp, P2)).Value;
+  EXPECT_TRUE(Locations.empty());
+}
+
 TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
   class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
   public:
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -127,11 +127,13 @@
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
std::vector TopLevelDeclIDs,
-   std::vector Diags);
+   std::vector Diags,
+   std::map IncludeMap);
 
   PrecompiledPreamble Preamble;
   std::vector TopLevelDeclIDs;
   std::vector Diags;
+  std::map IncludeMap;
 };
 
 /// Manages resources, required by clangd. Allows to rebuild file with new
@@ -261,7 +263,8 @@
 
 /// Get definition of symbol at a specified \p Pos.
 std::vector findDefinitions(ParsedAST &AST, Position Pos,
-  clangd::Logger &Logger);
+  clangd::Logger &Logger,
+  std::map);
 
 /// For testing/debugging purposes. Note that this method deserializes all
 /// unserialized Decls, so use with care.
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -78,6 +78,10 @@
 return std::move(TopLevelDeclIDs);
   }
 
+  std::map takeIncludeMap() {
+return std::move(IncludeMap);
+  }
+
   void AfterPCHEmitted(ASTWriter &Writer) override {
 TopLevelDeclIDs.reserve(TopLevelDecls.size());
 for (Decl *D : TopLevelDecls) {
@@ -96,9 +100,55 @@
 }
   }
 
+  void AfterExecute(CompilerInstance &CI) override {
+const SourceManager &SM = CI.getSourceManager();
+unsigned n = SM.local_sloc_entry_size();
+SmallVector InclusionStack;
+std::map::iterator it = IncludeMap.begin();
+
+for (unsigned i = 0; i < n; ++i) {
+  bool Invalid = false;
+  const SrcMgr::SLocEntry &SL = SM.getLocalSLocEntry(i, &Invalid);
+  if (!SL.isFile() || Invalid)
+continue;
+  const SrcMgr::FileInfo &FI = SL.getFile();
+  SourceLocation L = FI.getIncludeLoc();
+  InclusionStack.clear();
+
+  SourceLocation LocationToInsert;
+
+  while (L.isValid()) {
+PresumedLoc PLoc = SM.getPresumedLoc(L);
+InclusionStack.push_back(L);
+L = PLoc.isValid() ? PLoc.getIncludeLoc() : SourceLocation();
+  }
+  if (InclusionStack.size() == 0) {
+// Skip main file
+continue;
+  }
+
+  if (InclusionStack.size() > 1) {
+// Don't care a

[PATCH] D38639: [clangd] #include statements support for Open definition

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.



Comment at: clangd/ClangdUnit.cpp:81
 
+  std::map takeIncludeMap() {
+return std::move(IncludeMap);

takeIncludeLocationMap?



Comment at: clangd/ClangdUnit.cpp:105
+const SourceManager &SM = CI.getSourceManager();
+unsigned n = SM.local_sloc_entry_size();
+SmallVector InclusionStack;

n -> NumSlocs?



Comment at: clangd/ClangdUnit.cpp:107
+SmallVector InclusionStack;
+std::map::iterator it = IncludeMap.begin();
+

do you need that iterator?



Comment at: clangd/ClangdUnit.cpp:109
+
+for (unsigned i = 0; i < n; ++i) {
+  bool Invalid = false;

i -> I



Comment at: clangd/ClangdUnit.cpp:137
+  FI.getContentCache()->OrigEntry->tryGetRealPathName();
+  if (FilePath.empty()) {
+// FIXME: Does tryGetRealPathName return empty if and only if the path

I think you can just skip it if empty (continue)



Comment at: clangd/ClangdUnit.cpp:143
+  }
+  IncludeMap.insert(std::pair(
+  InclusionStack.front(), FilePath.str()));

I think you can do instead 
IncludeMap.insert({InclusionStack.front(), FilePath.str()});



Comment at: clangd/ClangdUnit.cpp:151
   std::vector TopLevelDeclIDs;
+  std::map IncludeMap;
 };

IncludeMap -> IncludeLocationMap ?



Comment at: clangd/ClangdUnit.cpp:800
 
-  void addDeclarationLocation(const SourceRange &ValSourceRange) {
+  bool isSameLine(unsigned line) const {
+const SourceManager &SourceMgr = AST.getSourceManager();

line -> Line



Comment at: clangd/ClangdUnit.cpp:806
+  void addDeclarationLocation(const SourceRange &ValSourceRange,
+  bool test = false) {
 const SourceManager &SourceMgr = AST.getSourceManager();

remove bool test?



Comment at: clangd/ClangdUnit.cpp:824
+
+  void addLocation(URI uri, Range R) {
+

uri -> Uri



Comment at: clangd/ClangdUnit.cpp:834
+
+for (auto it = IncludeMap.begin(); it != IncludeMap.end(); ++it) {
+  SourceLocation L = it->first;

it -> It



Comment at: clangd/ClangdUnit.cpp:837
+  std::string &Path = it->second;
+  Range r = Range();
+  unsigned line = AST.getSourceManager().getSpellingLineNumber(L);

r -> R



Comment at: clangd/ClangdUnit.cpp:839
+  unsigned line = AST.getSourceManager().getSpellingLineNumber(L);
+  if (isSameLine(line))
+addLocation(URI::fromFile(Path), Range());

line -> Line



Comment at: clangd/ClangdUnit.h:136
   std::vector Diags;
+  std::map IncludeMap;
 };

IncludeLocationMap?



Comment at: clangd/ClangdUnit.h:267
+  clangd::Logger &Logger,
+  std::map);
 

do you want to add a name to the parameter here?



Comment at: unittests/clangd/ClangdTests.cpp:902
 
-TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) {
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
   MockFSProvider FS;

the test "CheckSourceHeaderSwitch" was removed


https://reviews.llvm.org/D38639



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

[PATCH] D38126: Make TBAA information to be part of LValueBaseInfo

[Ivan A. Kosarev via Phabricator via cfe-commits]

kosarev updated this revision to Diff 118045.
kosarev edited the summary of this revision.
kosarev added a comment.

Re-based on top of the previous refinements: https://reviews.llvm.org/D38404, 
https://reviews.llvm.org/D38408, https://reviews.llvm.org/D38456, 
https://reviews.llvm.org/D38460, https://reviews.llvm.org/D38503 and 
https://reviews.llvm.org/D37826. Still large, but most of the changes are local 
and trivial. Please let me know if it is too complex to be reviewed. Thanks.


https://reviews.llvm.org/D38126

Files:
  lib/CodeGen/CGAtomic.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CGValue.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTBAA.cpp
  lib/CodeGen/CodeGenTBAA.h

Index: lib/CodeGen/CodeGenTBAA.h
===
--- lib/CodeGen/CodeGenTBAA.h
+++ lib/CodeGen/CodeGenTBAA.h
@@ -47,6 +47,20 @@
 : TBAAAccessInfo(/* AccessType= */ nullptr)
   {}
 
+  bool operator==(const TBAAAccessInfo &Other) const {
+return BaseType == Other.BaseType &&
+   AccessType == Other.AccessType &&
+   Offset == Other.Offset;
+  }
+
+  bool operator!=(const TBAAAccessInfo &Other) const {
+return !(*this == Other);
+  }
+
+  explicit operator bool() const {
+return *this != TBAAAccessInfo();
+  }
+
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
@@ -136,6 +150,19 @@
   /// getMayAliasAccessInfo - Get TBAA information that represents may-alias
   /// accesses.
   TBAAAccessInfo getMayAliasAccessInfo();
+
+  /// isMayAliasAccessInfo - Test for the may-alias TBAA access descriptor.
+  bool isMayAliasAccessInfo(TBAAAccessInfo Info);
+
+  /// mergeTBAAInfoForCast - Get merged TBAA information for the purpose of
+  /// type casts.
+  TBAAAccessInfo mergeTBAAInfoForCast(TBAAAccessInfo SourceInfo,
+  TBAAAccessInfo TargetInfo);
+
+  /// mergeTBAAInfoForConditionalOperator - Get merged TBAA information for the
+  /// purpose of conditional operator.
+  TBAAAccessInfo mergeTBAAInfoForConditionalOperator(TBAAAccessInfo InfoA,
+ TBAAAccessInfo InfoB);
 };
 
 }  // end namespace CodeGen
@@ -166,9 +193,7 @@
 
   static bool isEqual(const clang::CodeGen::TBAAAccessInfo &LHS,
   const clang::CodeGen::TBAAAccessInfo &RHS) {
-return LHS.BaseType == RHS.BaseType &&
-   LHS.AccessType == RHS.AccessType &&
-   LHS.Offset == RHS.Offset;
+return LHS == RHS;
   }
 };
 
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -88,18 +88,45 @@
   return false;
 }
 
+/// Check if the given type is a valid base type to be used in access tags.
+static bool isValidBaseType(QualType QTy) {
+  if (QTy->isReferenceType())
+return false;
+  if (const RecordType *TTy = QTy->getAs()) {
+const RecordDecl *RD = TTy->getDecl()->getDefinition();
+// Incomplete types are not valid base access types.
+if (!RD)
+  return false;
+if (RD->hasFlexibleArrayMember())
+  return false;
+// RD can be struct, union, class, interface or enum.
+// For now, we only handle struct and class.
+if (RD->isStruct() || RD->isClass())
+  return true;
+  }
+  return false;
+}
+
 llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) {
   // At -O0 or relaxed aliasing, TBAA is not emitted for regular types.
   if (CodeGenOpts.OptimizationLevel == 0 || CodeGenOpts.RelaxedAliasing)
 return nullptr;
 
+  // In some cases, such as dereferencing a structure member, the final access
+  // type may well itself be an aggregate. Since it is possible to dereference
+  // a member of that aggregate, this function shall be able to generate
+  // descriptors for any object types, including aggregate ones, without
+  // falling back to returning the "omnipotent char" type node.
+  // TODO: Combine getTypeInfo() and getBaseTypeInfo() into a single function.
+  if (isValidBaseType(QTy))
+return getBaseTypeInfo(QTy);
+
   // If the type has the may_alias attribute (even on a typedef), it is
   // effectively in the general char alias class.
   if (TypeHasMayAlias(QTy))
 return getChar();
 
   const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
-
   if (llvm::MDNode *N = MetadataCache[Ty])
 return N;
 
@@ -232,20 +259,6 @@
   return StructMetadataCache[Ty] = nullptr;
 }
 
-/// Check if the given type is a valid base type to be used in access tags.
-static bool isValidBaseType(QualType QTy) {
-  if (const RecordType *TT

[PATCH] D38126: Make TBAA information to be part of LValueBaseInfo

[Ivan A. Kosarev via Phabricator via cfe-commits]

kosarev added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:55
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Verifier.h"  // TODO
 #include "llvm/ProfileData/InstrProfReader.h"

Oops. Will be removed.


https://reviews.llvm.org/D38126



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

[libunwind] r315090 - [docs] Mention that SjLj works on any OS on the archs where supported by the compiler

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Fri Oct  6 12:14:07 2017
New Revision: 315090

URL: http://llvm.org/viewvc/llvm-project?rev=315090&view=rev
Log:
[docs] Mention that SjLj works on any OS on the archs where supported by the 
compiler

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

Modified:
libunwind/trunk/docs/index.rst

Modified: libunwind/trunk/docs/index.rst
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/docs/index.rst?rev=315090&r1=315089&r2=315090&view=diff
==
--- libunwind/trunk/docs/index.rst (original)
+++ libunwind/trunk/docs/index.rst Fri Oct  6 12:14:07 2017
@@ -50,6 +50,7 @@ Linuxi386, x86_64 Clang,
 LinuxARM  Clang, GCC   EHABI
 Bare Metal   ARM  Clang, GCC   EHABI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.


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

[PATCH] D38576: |libunwind] [docs] Mention that SjLj works on any OS on the archs where supported by the compiler

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315090: [docs] Mention that SjLj works on any OS on the 
archs where supported by the… (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D38576?vs=117778&id=118054#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38576

Files:
  libunwind/trunk/docs/index.rst


Index: libunwind/trunk/docs/index.rst
===
--- libunwind/trunk/docs/index.rst
+++ libunwind/trunk/docs/index.rst
@@ -50,6 +50,7 @@
 LinuxARM  Clang, GCC   EHABI
 Bare Metal   ARM  Clang, GCC   EHABI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.


Index: libunwind/trunk/docs/index.rst
===
--- libunwind/trunk/docs/index.rst
+++ libunwind/trunk/docs/index.rst
@@ -50,6 +50,7 @@
 LinuxARM  Clang, GCC   EHABI
 Bare Metal   ARM  Clang, GCC   EHABI
 NetBSD   x86_64   Clang, GCC   DWARF CFI
+Any  i386, x86_64, ARMClangSjLj
    
 
 The following minimum compiler versions are strongly recommended.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r315093 - [ObjC] Don't warn on readwrite properties with custom setters that

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Oct  6 12:24:26 2017
New Revision: 315093

URL: http://llvm.org/viewvc/llvm-project?rev=315093&view=rev
Log:
[ObjC] Don't warn on readwrite properties with custom setters that
override readonly properties from protocols

rdar://34192541

Added:
cfe/trunk/test/SemaObjC/property-implement-readonly-with-custom-setter.m
Modified:
cfe/trunk/lib/Sema/SemaObjCProperty.cpp

Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=315093&r1=315092&r2=315093&view=diff
==
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Fri Oct  6 12:24:26 2017
@@ -1599,7 +1599,11 @@ Sema::DiagnosePropertyMismatch(ObjCPrope
   // meaningless for readonly properties, so don't diagnose if the
   // atomic property is 'readonly'.
   checkAtomicPropertyMismatch(*this, SuperProperty, Property, false);
-  if (Property->getSetterName() != SuperProperty->getSetterName()) {
+  // Readonly properties from protocols can be implemented as "readwrite"
+  // with a custom setter name.
+  if (Property->getSetterName() != SuperProperty->getSetterName() &&
+  !(SuperProperty->isReadOnly() &&
+isa(SuperProperty->getDeclContext( {
 Diag(Property->getLocation(), diag::warn_property_attribute)
   << Property->getDeclName() << "setter" << inheritedName;
 Diag(SuperProperty->getLocation(), diag::note_property_declare);

Added: cfe/trunk/test/SemaObjC/property-implement-readonly-with-custom-setter.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-implement-readonly-with-custom-setter.m?rev=315093&view=auto
==
--- cfe/trunk/test/SemaObjC/property-implement-readonly-with-custom-setter.m 
(added)
+++ cfe/trunk/test/SemaObjC/property-implement-readonly-with-custom-setter.m 
Fri Oct  6 12:24:26 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1  -fsyntax-only -verify -Wno-objc-root-class %s
+// rdar://34192541
+
+@class NSString;
+
+@protocol MyProtocol
+@property (nonatomic, strong, readonly) NSString *myString;
+@end
+
+@interface MyClass 
+// Don't warn about this setter:
+@property (nonatomic, strong, setter=setMYString:) NSString *myString;
+
+
+@property (nonatomic, strong, readonly) NSString *overridenInClass; // 
expected-note {{property declared here}}
+@end
+
+@interface MySubClass: MyClass
+@property (nonatomic, strong, setter=setMYOverride:) NSString 
*overridenInClass;
+// expected-warning@-1 {{'setter' attribute on property 'overridenInClass' 
does not match the property inherited from 'MyClass'}}
+@end


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

r315094 - OpenCL: Assume functions are convergent

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Fri Oct  6 12:34:40 2017
New Revision: 315094

URL: http://llvm.org/viewvc/llvm-project?rev=315094&view=rev
Log:
OpenCL: Assume functions are convergent

This was done for CUDA functions in r261779, and for the same
reason this also needs to be done for OpenCL. An arbitrary
function could have a barrier() call in it, which in turn
requires the calling function to be convergent.

Modified:
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl
cfe/trunk/test/CodeGenOpenCL/convergent.cl

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=315094&r1=315093&r2=315094&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Fri Oct  6 12:34:40 2017
@@ -197,6 +197,10 @@ public:
   bool allowsNonTrivialObjCLifetimeQualifiers() const {
 return ObjCAutoRefCount || ObjCWeak;
   }
+
+  bool assumeFunctionsAreConvergent() const {
+return (CUDA && CUDAIsDevice) || OpenCL;
+  }
 };
 
 /// \brief Floating point control options

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=315094&r1=315093&r2=315094&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Oct  6 12:34:40 2017
@@ -1750,13 +1750,16 @@ void CodeGenModule::ConstructDefaultFnAt
   FuncAttrs.addAttribute("backchain");
   }
 
-  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
-// Conservatively, mark all functions and calls in CUDA as convergent
-// (meaning, they may call an intrinsically convergent op, such as
-// __syncthreads(), and so can't have certain optimizations applied around
-// them).  LLVM will remove this attribute where it safely can.
+  if (getLangOpts().assumeFunctionsAreConvergent()) {
+// Conservatively, mark all functions and calls in CUDA and OpenCL as
+// convergent (meaning, they may call an intrinsically convergent op, such
+// as __syncthreads() / barrier(), and so can't have certain optimizations
+// applied around them).  LLVM will remove this attribute where it safely
+// can.
 FuncAttrs.addAttribute(llvm::Attribute::Convergent);
+  }
 
+  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
 // Exceptions aren't supported in CUDA device code.
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
 

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl?rev=315094&r1=315093&r2=315094&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl Fri Oct  6 12:34:40 2017
@@ -151,28 +151,28 @@ kernel void reqd_work_group_size_32_2_1_
 // CHECK-NOT: "amdgpu-num-sgpr"="0"
 // CHECK-NOT: "amdgpu-num-vgpr"="0"
 
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { noinline nounwind 
optnone "amdgpu-flat-work-group-size"="32,64"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_64_64]] = { noinline nounwind 
optnone "amdgpu-flat-work-group-size"="64,64"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_16_128]] = { noinline nounwind 
optnone "amdgpu-flat-work-group-size"="16,128"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { noinline nounwind optnone 
"amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { noinline nounwind optnone 
"amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32]] = { noinline nounwind optnone 
"amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[NUM_VGPR_64]] = { noinline nounwind optnone 
"amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64"
+// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_64_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="64,64"
+// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_16_128]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="16,128"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind 
optnone "amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind 
optnone "amdgpu-waves-per-eu"="2,4"
+// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind 
optnone "amdgpu-num-sgpr"="32"
+// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind 
optnone "amdgpu-num-vgpr"="64"
 
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { 
noinline nounwind optnone "amdgpu-fla

[PATCH] D38113: OpenCL: Assume functions are convergent

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r315094




Comment at: test/CodeGenOpenCL/convergent.cl:130
+// CHECK: attributes #0 = { noinline norecurse nounwind "
+// CHECK: attributes #1 = { {{[^}]*}}convergent{{[^}]*}} }
+// CHECK: attributes #2 = { {{[^}]*}}convergent{{[^}]*}} }

Anastasia wrote:
> We won't have noduplicate any more?
noduplicate is problematic for the same reason that an unknown call could have 
noduplicate. We should probably just remove noduplicate entirely.


https://reviews.llvm.org/D38113



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

r315095 - Revert r315087

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Oct  6 12:49:29 2017
New Revision: 315095

URL: http://llvm.org/viewvc/llvm-project?rev=315095&view=rev
Log:
Revert r315087

clang-refactor crashes on some bots after this commit

Removed:
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOption.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptionVisitor.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringOptions.h
Modified:
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
cfe/trunk/test/Refactor/LocalRename/Field.cpp
cfe/trunk/test/Refactor/tool-test-support.c
cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h?rev=315095&r1=315094&r2=315095&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h 
(original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h Fri Oct 
 6 12:49:29 2017
@@ -16,7 +16,6 @@
 namespace clang {
 namespace tooling {
 
-class RefactoringOptionVisitor;
 class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
@@ -44,14 +43,6 @@ public:
   /// Returns true when the rule has a source selection requirement that has
   /// to be fullfilled before refactoring can be performed.
   virtual bool hasSelectionRequirement() = 0;
-
-  /// Traverses each refactoring option used by the rule and invokes the
-  /// \c visit callback in the consumer for each option.
-  ///
-  /// Options are visited in the order of use, e.g. if a rule has two
-  /// requirements that use options, the options from the first requirement
-  /// are visited before the options in the second requirement.
-  virtual void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) = 0;
 };
 
 } // end namespace tooling

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h?rev=315095&r1=315094&r2=315095&view=diff
==
--- 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h 
(original)
+++ 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h 
Fri Oct  6 12:49:29 2017
@@ -11,7 +11,6 @@
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 
 #include "clang/Basic/LLVM.h"
-#include "clang/Tooling/Refactoring/RefactoringOption.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include 
@@ -54,45 +53,6 @@ public:
   }
 };
 
-/// A base class for any requirement that requires some refactoring options.
-class RefactoringOptionsRequirement : public RefactoringActionRuleRequirement {
-public:
-  virtual ~RefactoringOptionsRequirement() {}
-
-  /// Returns the set of refactoring options that are used when evaluating this
-  /// requirement.
-  virtual ArrayRef>
-  getRefactoringOptions() const = 0;
-};
-
-/// A requirement that evaluates to the value of the given \c OptionType when
-/// the \c OptionType is a required option. When the \c OptionType is an
-/// optional option, the requirement will evaluate to \c None if the option is
-/// not specified or to an appropriate value otherwise.
-template 
-class OptionRequirement : public RefactoringOptionsRequirement {
-public:
-  OptionRequirement() : Opt(createRefactoringOption()) {}
-
-  ArrayRef>
-  getRefactoringOptions() const final override {
-return static_cast &>(Opt);
-  }
-
-  Expected
-  evaluate(RefactoringRuleContext &) const {
-return Opt->getValue();
-  }
-
-private:
-  /// The partially-owned option.
-  ///
-  /// The ownership of the option is shared among the different requirements
-  /// because the same option can be used by multiple rules in one refactoring
-  /// action.
-  std::shared_ptr Opt;
-};
-
 } // end namespace tooling
 } // end namespace clang
 

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h?rev=315095&r1=315094&r2=315095&view=diff
==
--- 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h 
(original)
+++ 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h 

[PATCH] D38596: Implement attribute target multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 31 inline comments as done.
erichkeane added a subscriber: rnk.
erichkeane added a comment.

Weew... I think I got everything.  Thanks for the review you three!  I've got 
another patch coming momentarily with what I believe is all your suggestions.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9301
+def err_target_causes_illegal_multiversioning
+: Error<"function redeclaration causes a multiversioned function, but a "
+"previous declaration lacks a 'target' attribute">;

aaron.ballman wrote:
> 'causes' seems a bit strange; how about `function redeclaration declares a 
> multiversioned function,`? (Or something else, I'm not tied to my wording.)
Thats as good as anything I can think of. I'll change to yours for now, but if 
someone else comes up with something that sounds better, I'll consider us open 
to it.



Comment at: lib/Basic/Targets/X86.cpp:1295
   .Case("amdfam10h", true)
+  .Case("amdfam10", true)
   .Case("amdfam15h", true)

hfinkel wrote:
> Can you please separate out these changes adding the amdfam10 target strings 
> into a separate patch?
Talked to Craig on this... apparently the rules for 
amdfam10/amdfam15/amdfam10h/amdfam15h are a bit more complicated (-march 
supports ONLY the amdfam10, validateCpuIs only supports the "h" versions (of 
both), and only the 'march' version is supported in this feature.  I may have 
to toss another function to mess with these strings to make it work.



Comment at: lib/CodeGen/CodeGenFunction.cpp:2279
+llvm::Value *
+CodeGenFunction::FormResolverCondition(const ResolverOption &RO) {
+  llvm::Value *TrueCondition = nullptr;

There IS a "CodeGen/TargetInfo" here, but it isn't as comprehensive as the 
Basic/TargetInfo types.  I wonder if there would be value in creating a 
'TargetCodeGenInfo::EmitCpuIs' and TargetCodeGenInfo::EmitCpuSupports to 
replace the X86 version of these, and suppressing this?

It might be valuable to simply replace the 'EmitTargetArchBuiltinExpr' with 
something to do this as well.  Those handful of target-arch-builtins are a 
little messy, and are pretty messy in CGFunction.

Probably will have to ask @rnk if he's got a good thought here.  I mentioned 
trying to break up this target info at one point, and he thought it wasn't a 
great idea.



Comment at: lib/CodeGen/CodeGenFunction.h:3707
+llvm::Function *Function;
+ResolverOption(TargetAttr::ParsedTargetAttr &&AttrInfo,// can be moved?
+   llvm::Function *Function)

aaron.ballman wrote:
> The comment doesn't inspire confidence with the trailing question mark. ;-)
Woops!  Personal review comment was left in :)



Comment at: lib/Sema/SemaDecl.cpp:9297-9300
+if (NewFD->hasAttr())
+  NewFD->setMultiVersionKind(FunctionDecl::MultiVersionKind::All);
+else
+  NewFD->setMultiVersionKind(FunctionDecl::MultiVersionKind::None);

aaron.ballman wrote:
> Might be more succinctly written with `?:` (unsure which looks better, try it 
> and see).
It does actually... Clang format makes this look really nice.



Comment at: lib/Sema/SemaDecl.cpp:9306
+  switch (OldFD->getMultiVersionKind()) {
+  default:
+llvm_unreachable("Invalid State for Multiversioning.");

aaron.ballman wrote:
> I suspect this will give warnings about having a `default` label in a 
> fully-covered switch.
I didn't see any, but I have no problem removing it anyway.



Comment at: test/CodeGenCXX/attr-target-multiversion.cpp:20
+}
+
+// CHECK: define i{{[0-9]+}} (%struct.S*)* @_ZN1S2mvEv.resolver

hfinkel wrote:
> Tests for interactions with function pointers and virtual functions?
Looking into it, I'm not sure how virtual functions should LOOK in this case 
(and GCC doesn't actually implement it either!).  I might punt for now while 
the rest of the patch is in shape, and add it in with function templates/etc.


https://reviews.llvm.org/D38596



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

[PATCH] D38596: Implement attribute target multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 118058.
erichkeane marked 5 inline comments as done.
erichkeane added a comment.

All requested changes AFAIK.  Also, discovered that I wasn't calling cpu-init 
at the beginning of my resolver, so added that.

Note: Virtual functions seem to have a bunch of complexity involved, so I'm 
going to punt on that for now.  GCC also doesn't support it, so I feel ok doing 
that later while dealing with function templates.


https://reviews.llvm.org/D38596

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Sema/Sema.h
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Sema/SemaDecl.cpp
  test/CodeGen/attr-target-multiversion.c
  test/CodeGenCXX/attr-target-multiversion.cpp
  test/Sema/attr-target-multiversion.c
  test/SemaCXX/attr-target-multiversion.cpp

Index: test/SemaCXX/attr-target-multiversion.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-target-multiversion.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
+
+struct S {
+  __attribute__((target("arch=sandybridge")))
+  void mv(){}
+  __attribute__((target("arch=ivybridge")))
+  void mv(){}
+  __attribute__((target("default")))
+  void mv(){}
+
+  // NOTE: Virtual functions aren't implement for multiversioning in GCC either,
+  // so this is a 'TBD' feature.
+  // expected-error@+2 {{function multiversioning with 'target' doesn't support virtual functions yet}}
+  __attribute__((target("arch=sandybridge")))
+  virtual void mv_2(){}
+  // expected-error@+4 {{function multiversioning with 'target' doesn't support virtual functions yet}}
+  // expected-error@+3 {{class member cannot be redeclared}}
+  // expected-note@-3 {{previous definition is here}}
+  __attribute__((target("arch=ivybridge")))
+  virtual void mv_2(){}
+  // expected-error@+3 {{class member cannot be redeclared}}
+  // expected-note@-7 {{previous definition is here}}
+  __attribute__((target("default")))
+  virtual void mv_2(){}
+};
+
+// Note: Template attribute 'target' isn't implemented in GCC either, and would
+// end up causing some nasty issues attempting it, so ensure that it still gives
+// the same errors as without the attribute.
+
+template
+__attribute__((target("arch=sandybridge")))
+void mv_temp(){}
+
+template
+__attribute__((target("arch=ivybridge")))
+//expected-error@+2 {{redefinition of}}
+//expected-note@-5{{previous definition is here}}
+void mv_temp(){}
+
+template
+__attribute__((target("default")))
+void mv_temp(){}
+
+void foo() {
+  //expected-error@+2{{no matching function for call to}}
+  //expected-note@-8{{candidate template ignored}}
+  mv_temp();
+}
Index: test/Sema/attr-target-multiversion.c
===
--- /dev/null
+++ test/Sema/attr-target-multiversion.c
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
+// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only -DCHECK_DEFAULT %s
+
+#if defined(CHECK_DEFAULT)
+__attribute__((target("arch=sandybridge")))
+//expected-error@+1 {{function multiversioning with 'target' requires a 'default' implementation}}
+void no_default(){}
+__attribute__((target("arch=ivybridge")))
+void no_default(){}
+#else
+// Only multiversioning causes issues with redeclaration changing 'target'.
+__attribute__((target("arch=sandybridge")))
+void fine_since_no_mv();
+void fine_since_no_mv();
+
+void also_fine_since_no_mv();
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv();
+
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv2();
+__attribute__((target("arch=sandybridge")))
+void also_fine_since_no_mv2();
+void also_fine_since_no_mv2();
+
+__attribute__((target("arch=sandybridge")))
+void mv(){}
+__attribute__((target("arch=ivybridge")))
+void mv(){}
+__attribute__((target("default")))
+void mv(){}
+
+void redecl_causes_mv();
+__attribute__((target("arch=sandybridge")))
+void redecl_causes_mv();
+// expected-error@+3 {{function redeclaration declares a multiversioned function, but a previous declaration lacks a 'target' attribute}}
+// expected-note@-4 {{previous declaration is here}}
+__attribute__((target("arch=ivybridge")))
+void redecl_causes_mv();
+
+__attribute__((target("arch=sandybridge")))
+void redecl_causes_mv2();
+void redecl_causes_mv2();
+// expected-error@+3 {{function redeclaration declares a multiversioned function, but a previous declaration lacks a 'target' attribute}}
+// expected-note@-2 {{previous declaration is here}}
+__attribute__((target("arch=ivybridge")))
+void redecl_causes_mv2();
+
+__attribute__((target("arch=sandy

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: mgorny.

Build with DummyClangFuzzer.cpp as entry point when coverage
instrumentation isn't present.


https://reviews.llvm.org/D38642

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-fuzzer/DummyClangFuzzer.cpp

Index: clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
===
--- /dev/null
+++ clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
@@ -0,0 +1,21 @@
+//===-- DummyClangFuzzer.cpp - Entry point to sanity check fuzzers ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Provides a main() to build without linking libFuzzer.
+//
+//===--===//
+#include "llvm/FuzzMutate/FuzzerCLI.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv);
+
+int main(int argc, char *argv[]) {
+  return llvm::runFuzzerOnInputs(argc, argv, LLVMFuzzerTestOneInput,
+ LLVMFuzzerInitialize);
+}
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -17,6 +17,8 @@
 
 using namespace clang_fuzzer;
 
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
+
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, {"-O2"});
Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,60 +1,65 @@
-if( LLVM_USE_SANITIZE_COVERAGE )
-  set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
-  set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
+set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(DUMMY_MAIN DummyClangFuzzer.cpp)
+if(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+  unset(DUMMY_MAIN)
+endif()
+
+# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
+# executables from this directory.
+set(LLVM_OPTIONAL_SOURCES
+  ClangFuzzer.cpp
+  DummyClangFuzzer.cpp
+  ExampleClangProtoFuzzer.cpp
+  )
+
+if(CLANG_ENABLE_PROTO_FUZZER)
+  # Create protobuf .h and .cc files, and put them in a library for use by
+  # clang-proto-fuzzer components.
+  find_package(Protobuf REQUIRED)
+  add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI)
+  include_directories(${PROTOBUF_INCLUDE_DIRS})
+  include_directories(${CMAKE_CURRENT_BINARY_DIR})
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})
+  add_clang_library(clangCXXProto
+${PROTO_SRCS}
+${PROTO_HDRS}
+
+LINK_LIBS
+${PROTOBUF_LIBRARIES}
+)
 
-  if(CLANG_ENABLE_PROTO_FUZZER)
-# Create protobuf .h and .cc files, and put them in a library for use by
-# clang-proto-fuzzer components.
-find_package(Protobuf REQUIRED)
-add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI)
-include_directories(${PROTOBUF_INCLUDE_DIRS})
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES
-  ClangFuzzer.cpp
-  ExampleClangProtoFuzzer.cpp
-  ${PROTO_SRCS}
-  )
-add_clang_library(clangCXXProto
-  ${PROTO_SRCS}
-  ${PROTO_HDRS}
-
-  LINK_LIBS
-  ${PROTOBUF_LIBRARIES}
-  )
-
-# Build and include libprotobuf-mutator
-include(ProtobufMutator)
-include_directories(${ProtobufMutator_INCLUDE_DIRS})
-
-# Build the protobuf->C++ translation library and driver.
-add_clang_subdirectory(proto-to-cxx)
-
-# Build the protobuf fuzzer
-add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
-target_link_libraries(clang-proto-fuzzer
-  ${ProtobufMutator_LIBRARIES}
-  clangCXXProto
-  clangHandleCXX
-  clangProtoToCXX
-  )
-  else()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.cpp)
-  endif()
-
-  add_clang_subdirectory(handle-cxx)
-
-  add_clang_executable(clang-fuzzer
-EXCLUDE_FROM_ALL
-ClangFuzzer.cpp
+  # Build and include libprotobuf-mutator
+  include(ProtobufMutator)
+  includ

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple created this revision.

Added array type mangling to USR generation. Included test from bug report.


Repository:
  rL LLVM

https://reviews.llvm.org/D38643

Files:
  lib/Index/USRGeneration.cpp
  test/Index/USR/array-type.cpp


Index: test/Index/USR/array-type.cpp
===
--- /dev/null
+++ test/Index/USR/array-type.cpp
@@ -0,0 +1,8 @@
+// RUN: c-index-test core -print-source-symbols -- %s | grep 
"function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+
+// Function template specializations differing in array type parameter should 
have unique USRs.
+
+template void foo(buffer);
+template<> void foo(char[16]);
+template<> void foo(char[32]);
+template<> void foo(char[64]);
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -816,6 +816,22 @@
   T = VT->getElementType();
   continue;
 }
+if (const ArrayType *const AT = dyn_cast(T)) {
+  VisitType(AT->getElementType());
+  Out << "[";
+
+  switch( AT->getSizeModifier() ) {
+case ArrayType::Static  : Out << "s"; break;
+case ArrayType::Star: Out << "*"; break;
+default : ;
+  }
+  if (const ConstantArrayType* const CAT = dyn_cast(T)) 
{
+Out << CAT->getSize();
+  }
+
+  Out << "]";
+  return;
+}
 
 // Unhandled type.
 Out << ' ';


Index: test/Index/USR/array-type.cpp
===
--- /dev/null
+++ test/Index/USR/array-type.cpp
@@ -0,0 +1,8 @@
+// RUN: c-index-test core -print-source-symbols -- %s | grep "function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+
+// Function template specializations differing in array type parameter should have unique USRs.
+
+template void foo(buffer);
+template<> void foo(char[16]);
+template<> void foo(char[32]);
+template<> void foo(char[64]);
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -816,6 +816,22 @@
   T = VT->getElementType();
   continue;
 }
+if (const ArrayType *const AT = dyn_cast(T)) {
+  VisitType(AT->getElementType());
+  Out << "[";
+
+  switch( AT->getSizeModifier() ) {
+case ArrayType::Static  : Out << "s"; break;
+case ArrayType::Star: Out << "*"; break;
+default : ;
+  }
+  if (const ConstantArrayType* const CAT = dyn_cast(T)) {
+Out << CAT->getSize();
+  }
+
+  Out << "]";
+  return;
+}
 
 // Unhandled type.
 Out << ' ';
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

It's not about coverage instrumentation (not) being present, but about 
libFuzzer's main() being present, right? 
Will we be able to reuse some of Justin's code instead of creating one more 
main() function? 
Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) even if we 
don't us einstrumentation at compile time?


https://reviews.llvm.org/D38642



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

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D38642#890963, @kcc wrote:

> It's not about coverage instrumentation (not) being present, but about 
> libFuzzer's main() being present, right?


Yes.

> Will we be able to reuse some of Justin's code instead of creating one more 
> main() function?

This reuses the code that Justin moved to FuzzMutate/FuzzerCLI.  That's why the 
main is so short.  But perhaps we could move the main itself into FuzzerCLI?

> Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) even if we 
> don't us einstrumentation at compile time?

When I tried this, I got undefined references to all kinds of 
`__sanitizer_cov_*` symbols.


https://reviews.llvm.org/D38642



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

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

>> Will we be able to reuse some of Justin's code instead of creating one more 
>> main() function?
> 
> This reuses the code that Justin moved to FuzzMutate/FuzzerCLI.  That's why 
> the main is so short.  But perhaps we could move the main itself into 
> FuzzerCLI?

Yes, having one common main makes sense, but see below.

>> Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) even if we 
>> don't us einstrumentation at compile time?
> 
> When I tried this, I got undefined references to all kinds of 
> `__sanitizer_cov_*` symbols.

I'd like to know more. 
At least simple cases work fine:

  clang++ ~/llvm/projects/compiler-rt/test/fuzzer/SimpleTest.cpp -std=c++11  -c 
&& clang++ SimpleTest.o -fsanitize=fuzzer 


https://reviews.llvm.org/D38642



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

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks for working on this!
Could you please post the patch with full context (`git diff -U99`)?




Comment at: test/Index/USR/array-type.cpp:1
+// RUN: c-index-test core -print-source-symbols -- %s | grep 
"function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+

Please use `FileCheck` and verify the exact USR strings. This way you'll know 
that they're unique without actually trying to verify if they're unique in the 
test.


Repository:
  rL LLVM

https://reviews.llvm.org/D38643



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

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: lib/Sema/SemaAccess.cpp:1798
+  EffectiveContext EC(CurScope->getEntity());
+  if (IsAccessible(*this, EC, Entity) == ::AR_accessible)
+return AR_accessible;

You don't need that scope resolution operator there. Also, I guess you don't 
have to create `EC`, you can just pass 
`EffectiveContext(CurScope->getEntity())` directly to `IsAccessible`.


https://reviews.llvm.org/D36918



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

[PATCH] D38134: [OpenCL] Emit enqueued block as kernel

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 118064.
yaxunl marked 5 inline comments as done.
yaxunl added a comment.

Revise by Anastasia's comments.


https://reviews.llvm.org/D38134

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/TargetInfo.cpp
  lib/CodeGen/TargetInfo.h
  test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -6,10 +6,33 @@
 typedef void (^bl_t)(local void *);
 typedef struct {int a;} ndrange_t;
 
-// N.B. The check here only exists to set BL_GLOBAL
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL:@__block_literal_global(\.[0-9]+)?]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: %struct.__opencl_block_literal_generic = type { i32, i32, i8 addrspace(4)* }
+
+// For a block global variable, first emit the block literal as a global variable, then emit the block variable itself.
+// COMMON: [[BL_GLOBAL:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INV_G:@[^ ]+]] to i8*) to i8 addrspace(4)*) }
+// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+
+// For anonymous blocks without captures, emit block literals as global variable.
+// COMMON: [[BLG1:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG1:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG2:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG2:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG3:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG3:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG4:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG4:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG5:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG5:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG6:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*, i8 addrspace(3)*, i8 addrspace(3)*)* [[INVG6:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG7:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG7:@[^ ]+_kernel]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG8:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* [[INVG8:@[^ ]+]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG9:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INVG9:@[^ ]+]] to i8*) to i8 addrspace(4)*) }
+// COMMON: [[BLG8K:@__opencl_enqueued_block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* 

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple updated this revision to Diff 118066.
jkorous-apple added a comment.

Uploaded full diff.


https://reviews.llvm.org/D38643

Files:
  lib/Index/USRGeneration.cpp
  test/Index/USR/array-type.cpp


Index: test/Index/USR/array-type.cpp
===
--- /dev/null
+++ test/Index/USR/array-type.cpp
@@ -0,0 +1,8 @@
+// RUN: c-index-test core -print-source-symbols -- %s | grep 
"function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+
+// Function template specializations differing in array type parameter should 
have unique USRs.
+
+template void foo(buffer);
+template<> void foo(char[16]);
+template<> void foo(char[32]);
+template<> void foo(char[64]);
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -816,6 +816,22 @@
   T = VT->getElementType();
   continue;
 }
+if (const ArrayType *const AT = dyn_cast(T)) {
+  VisitType(AT->getElementType());
+  Out << "[";
+
+  switch( AT->getSizeModifier() ) {
+case ArrayType::Static  : Out << "s"; break;
+case ArrayType::Star: Out << "*"; break;
+default : ;
+  }
+  if (const ConstantArrayType* const CAT = dyn_cast(T)) 
{
+Out << CAT->getSize();
+  }
+
+  Out << "]";
+  return;
+}
 
 // Unhandled type.
 Out << ' ';


Index: test/Index/USR/array-type.cpp
===
--- /dev/null
+++ test/Index/USR/array-type.cpp
@@ -0,0 +1,8 @@
+// RUN: c-index-test core -print-source-symbols -- %s | grep "function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+
+// Function template specializations differing in array type parameter should have unique USRs.
+
+template void foo(buffer);
+template<> void foo(char[16]);
+template<> void foo(char[32]);
+template<> void foo(char[64]);
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -816,6 +816,22 @@
   T = VT->getElementType();
   continue;
 }
+if (const ArrayType *const AT = dyn_cast(T)) {
+  VisitType(AT->getElementType());
+  Out << "[";
+
+  switch( AT->getSizeModifier() ) {
+case ArrayType::Static  : Out << "s"; break;
+case ArrayType::Star: Out << "*"; break;
+default : ;
+  }
+  if (const ConstantArrayType* const CAT = dyn_cast(T)) {
+Out << CAT->getSize();
+  }
+
+  Out << "]";
+  return;
+}
 
 // Unhandled type.
 Out << ' ';
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D38642#890969, @kcc wrote:

> I'd like to know more. 
>  At least simple cases work fine:


You're right.  I was trying to add `-fsanitize=fuzzer` to `CMAKE_CXX_FLAGS` 
right before the link command, which was causing a later compilation to give 
the error.  Setting `CMAKE_EXE_LINKER_FLAGS` seems to work though.

This seems simpler and cleaner than the approach @bogner took.


https://reviews.llvm.org/D38642



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

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Index/USRGeneration.cpp:820
+if (const ArrayType *const AT = dyn_cast(T)) {
+  VisitType(AT->getElementType());
+  Out << "[";

We should probably follow the other types and just set `T = 
AT->getElementType()` instead of using `VisitType` and `continue` instead of 
`return`ing at the end of this if.



Comment at: lib/Index/USRGeneration.cpp:821
+  VisitType(AT->getElementType());
+  Out << "[";
+

The "[" "]" syntax could collide with the vector-type USRs. What about using 
another character? Maybe '{'?



Comment at: lib/Index/USRGeneration.cpp:826
+case ArrayType::Star: Out << "*"; break;
+default : ;
+  }

I think it's better to check 'ArrayType::Normal' instead of `default` to ensure 
we will be able to distinguish between added size modifiers that could be added 
in the future. We should also probably give it some representation, like 'n'.


https://reviews.llvm.org/D38643



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

[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.

This raises our default past 1900, which controls whether char16_t is a
builtin type or not.

Implements PR34243


https://reviews.llvm.org/D38646

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


Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
   if (MSVT.empty() &&
   Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.10 is default, aka 2017
+MSVT = VersionTuple(19, 10);
   }
   return MSVT;
 }


Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
   if (MSVT.empty() &&
   Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.10 is default, aka 2017
+MSVT = VersionTuple(19, 10);
   }
   return MSVT;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r315103 - -Wdocumentation should allow '...' params in variadic function type aliases

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Oct  6 13:51:04 2017
New Revision: 315103

URL: http://llvm.org/viewvc/llvm-project?rev=315103&view=rev
Log:
-Wdocumentation should allow '...' params in variadic function type aliases

rdar://34811344

Modified:
cfe/trunk/lib/AST/CommentSema.cpp
cfe/trunk/test/Sema/warn-documentation.cpp
cfe/trunk/test/Sema/warn-documentation.m

Modified: cfe/trunk/lib/AST/CommentSema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentSema.cpp?rev=315103&r1=315102&r2=315103&view=diff
==
--- cfe/trunk/lib/AST/CommentSema.cpp (original)
+++ cfe/trunk/lib/AST/CommentSema.cpp Fri Oct  6 13:51:04 2017
@@ -813,7 +813,7 @@ bool Sema::isAnyFunctionDecl() {
 }
 
 bool Sema::isFunctionOrMethodVariadic() {
-  if (!isAnyFunctionDecl() && !isObjCMethodDecl() && !isFunctionTemplateDecl())
+  if (!isFunctionDecl() || !ThisDeclInfo->CurrentDecl)
 return false;
   if (const FunctionDecl *FD =
 dyn_cast(ThisDeclInfo->CurrentDecl))
@@ -824,6 +824,14 @@ bool Sema::isFunctionOrMethodVariadic()
   if (const ObjCMethodDecl *MD =
 dyn_cast(ThisDeclInfo->CurrentDecl))
 return MD->isVariadic();
+  if (const TypedefNameDecl *TD =
+  dyn_cast(ThisDeclInfo->CurrentDecl)) {
+QualType Type = TD->getUnderlyingType();
+if (Type->isFunctionPointerType() || Type->isBlockPointerType())
+  Type = Type->getPointeeType();
+if (const auto *FT = Type->getAs())
+  return FT->isVariadic();
+  }
   return false;
 }
 

Modified: cfe/trunk/test/Sema/warn-documentation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation.cpp?rev=315103&r1=315102&r2=315103&view=diff
==
--- cfe/trunk/test/Sema/warn-documentation.cpp (original)
+++ cfe/trunk/test/Sema/warn-documentation.cpp Fri Oct  6 13:51:04 2017
@@ -1282,3 +1282,25 @@ struct HasMoreFields {
 };
 
 }
+
+/*!
+ * Function pointer typedef with variadic params.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+typedef void (*VariadicFnType)(int a, ...);
+
+/*!
+ * Function pointer type alias with variadic params.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+using VariadicFnType2 = void (*)(int a, ...);

Modified: cfe/trunk/test/Sema/warn-documentation.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation.m?rev=315103&r1=315102&r2=315103&view=diff
==
--- cfe/trunk/test/Sema/warn-documentation.m (original)
+++ cfe/trunk/test/Sema/warn-documentation.m Fri Oct  6 13:51:04 2017
@@ -299,3 +299,14 @@ void (^_Nullable blockPointerVariableTha
 @property void (^blockReturnsNothing)();
 
 @end
+
+/*!
+ * Block typedef with variadic params.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+typedef void (^VariadicBlockType)(int a, ...);


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