r359962 - Use DiagRuntimeBehavior for -Wunsequenced to weed out false positives

2019-05-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri May  3 22:20:14 2019
New Revision: 359962

URL: http://llvm.org/viewvc/llvm-project?rev=359962=rev
Log:
Use DiagRuntimeBehavior for -Wunsequenced to weed out false positives
where either the modification or the other access is unreachable.

Modified:
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/warn-unsequenced.c
cfe/trunk/test/SemaCXX/warn-unsequenced.cpp

Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=359962=359961=359962=diff
==
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Fri May  3 22:20:14 2019
@@ -84,11 +84,11 @@ class PossiblyUnreachableDiag {
 public:
   PartialDiagnostic PD;
   SourceLocation Loc;
-  const Stmt *stmt;
+  llvm::TinyPtrVector Stmts;
 
   PossiblyUnreachableDiag(const PartialDiagnostic , SourceLocation Loc,
-  const Stmt *stmt)
-  : PD(PD), Loc(Loc), stmt(stmt) {}
+  ArrayRef Stmts)
+  : PD(PD), Loc(Loc), Stmts(Stmts) {}
 };
 
 /// Retains information about a function, method, or block that is

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=359962=359961=359962=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri May  3 22:20:14 2019
@@ -4252,6 +4252,10 @@ public:
   /// If it is unreachable, the diagnostic will not be emitted.
   bool DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
const PartialDiagnostic );
+  /// Similar, but diagnostic is only produced if all the specified statements
+  /// are reachable.
+  bool DiagRuntimeBehavior(SourceLocation Loc, ArrayRef Stmts,
+   const PartialDiagnostic );
 
   // Primary Expressions.
   SourceRange getExprRange(Expr *E) const;

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=359962=359961=359962=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri May  3 22:20:14 2019
@@ -2089,16 +2089,16 @@ AnalysisBasedWarnings::IssueWarnings(sem
 
 // Register the expressions with the CFGBuilder.
 for (const auto  : fscope->PossiblyUnreachableDiags) {
-  if (D.stmt)
-AC.registerForcedBlockExpression(D.stmt);
+  for (const Stmt *S : D.Stmts)
+AC.registerForcedBlockExpression(S);
 }
 
 if (AC.getCFG()) {
   analyzed = true;
   for (const auto  : fscope->PossiblyUnreachableDiags) {
-bool processed = false;
-if (D.stmt) {
-  const CFGBlock *block = AC.getBlockForRegisteredExpression(D.stmt);
+bool AllReachable = true;
+for (const Stmt *S : D.Stmts) {
+  const CFGBlock *block = AC.getBlockForRegisteredExpression(S);
   CFGReverseBlockReachabilityAnalysis *cra =
   AC.getCFGReachablityAnalysis();
   // FIXME: We should be able to assert that block is non-null, but
@@ -2106,15 +2106,17 @@ AnalysisBasedWarnings::IssueWarnings(sem
   // edge cases; see test/Sema/vla-2.c.
   if (block && cra) {
 // Can this block be reached from the entrance?
-if (cra->isReachable(()->getEntry(), block))
-  S.Diag(D.Loc, D.PD);
-processed = true;
+if (!cra->isReachable(()->getEntry(), block)) {
+  AllReachable = false;
+  break;
+}
   }
+  // If we cannot map to a basic block, assume the statement is
+  // reachable.
 }
-if (!processed) {
-  // Emit the warning anyway if we cannot map to a basic block.
+
+if (AllReachable)
   S.Diag(D.Loc, D.PD);
-}
   }
 }
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=359962=359961=359962=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri May  3 22:20:14 2019
@@ -12155,10 +12155,11 @@ class SequenceChecker : public Evaluated
 if (OtherKind == UK_Use)
   std::swap(Mod, ModOrUse);
 
-SemaRef.Diag(Mod->getExprLoc(),
- IsModMod ? diag::warn_unsequenced_mod_mod
-  : 

r359960 - Reduce amount of work ODR hashing does.

2019-05-03 Thread Richard Trieu via cfe-commits
Author: rtrieu
Date: Fri May  3 21:22:33 2019
New Revision: 359960

URL: http://llvm.org/viewvc/llvm-project?rev=359960=rev
Log:
Reduce amount of work ODR hashing does.

When a FunctionProtoType is in the original type in a DecayedType, the decayed
type is a PointerType which points back the original FunctionProtoType.  The
visitor for ODRHashing will attempt to process both Type's, doing double work.
By chaining together multiple DecayedType's and FunctionProtoType's, this would
result in 2^N Type's visited only N DecayedType's and N FunctionProtoType's
exsit.  Another bug where VisitDecayedType and VisitAdjustedType did
redundant work doubled the work at each level, giving 4^N Type's visited.  This
patch removed the double work and detects when a FunctionProtoType decays to
itself to only check the Type once.  This lowers the exponential runtime to
linear runtime.  Fixes https://bugs.llvm.org/show_bug.cgi?id=41625

Modified:
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/test/Modules/odr_hash.cpp

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=359960=359959=359960=diff
==
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Fri May  3 21:22:33 2019
@@ -703,14 +703,36 @@ public:
   void VisitType(const Type *T) {}
 
   void VisitAdjustedType(const AdjustedType *T) {
-AddQualType(T->getOriginalType());
-AddQualType(T->getAdjustedType());
+QualType Original = T->getOriginalType();
+QualType Adjusted = T->getAdjustedType();
+
+// The original type and pointee type can be the same, as in the case of
+// function pointers decaying to themselves.  Set a bool and only process
+// the type once, to prevent doubling the work.
+SplitQualType split = Adjusted.split();
+if (auto Pointer = dyn_cast(split.Ty)) {
+  if (Pointer->getPointeeType() == Original) {
+Hash.AddBoolean(true);
+ID.AddInteger(split.Quals.getAsOpaqueValue());
+AddQualType(Original);
+VisitType(T);
+return;
+  }
+}
+
+// The original type and pointee type are different, such as in the case
+// of a array decaying to an element pointer.  Set a bool to false and
+// process both types.
+Hash.AddBoolean(false);
+AddQualType(Original);
+AddQualType(Adjusted);
+
 VisitType(T);
   }
 
   void VisitDecayedType(const DecayedType *T) {
-AddQualType(T->getDecayedType());
-AddQualType(T->getPointeeType());
+// getDecayedType and getPointeeType are derived from getAdjustedType
+// and don't need to be separately processed.
 VisitAdjustedType(T);
   }
 

Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=359960=359959=359960=diff
==
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Fri May  3 21:22:33 2019
@@ -4587,6 +4587,43 @@ int num = bar();
 #endif
 }
 
+namespace FunctionProtoTypeDecay {
+#if defined(FIRST)
+struct S1 {
+  struct X {};
+  using Y = X(X());
+};
+#elif defined(SECOND)
+struct S1 {
+  struct X {};
+  using Y = X(X(X()));
+};
+#else
+S1 s1;
+// expected-error@first.h:* {{'FunctionProtoTypeDecay::S1::Y' from module 
'FirstModule' is not present in definition of 'FunctionProtoTypeDecay::S1' in 
module 'SecondModule'}}
+// expected-note@second.h:* {{declaration of 'Y' does not match}}
+#endif
+
+#if defined(FIRST)
+struct S2 {
+  struct X {};
+  using Y =
+  X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
+  X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
+  X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
+  X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(X(
+  
+  
+  
+  ;
+};
+#elif defined(SECOND)
+#else
+S2 s2;
+#endif
+
+}
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST


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


r359958 - Disallow the operand of __builtin_constant_p from modifying enclosing

2019-05-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri May  3 21:00:45 2019
New Revision: 359958

URL: http://llvm.org/viewvc/llvm-project?rev=359958=rev
Log:
Disallow the operand of __builtin_constant_p from modifying enclosing
state when it's encountered while evaluating a constexpr function.

We attempt to follow GCC trunk's behavior here, but it is somewhat
inscrutible, so our behavior is only approximately the same for now.
Specifically, we only permit modification of objects whose lifetime
began within the operand of the __builtin_constant_p. GCC appears to
have effectively the same restriction, but also some unknown restriction
based on where and how the local state of the constexpr function is
mentioned within the operand (see added testcases).

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/builtin-constant-p.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359958=359957=359958=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri May  3 21:00:45 2019
@@ -716,6 +716,10 @@ namespace {
   EvaluatingObject(Decl, {CallIndex, Version}));
 }
 
+/// If we're currently speculatively evaluating, the outermost call stack
+/// depth at which we can mutate state, otherwise 0.
+unsigned SpeculativeEvaluationDepth = 0;
+
 /// The current array initialization index, if we're performing array
 /// initialization.
 uint64_t ArrayInitIndex = -1;
@@ -728,9 +732,6 @@ namespace {
 /// fold (not just why it's not strictly a constant expression)?
 bool HasFoldFailureDiagnostic;
 
-/// Whether or not we're currently speculatively evaluating.
-bool IsSpeculativelyEvaluating;
-
 /// Whether or not we're in a context where the front end requires a
 /// constant value.
 bool InConstantContext;
@@ -795,7 +796,7 @@ namespace {
 BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
 EvaluatingDecl((const ValueDecl *)nullptr),
 EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+HasFoldFailureDiagnostic(false),
 InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
@@ -823,14 +824,20 @@ namespace {
   return false;
 }
 
-CallStackFrame *getCallFrame(unsigned CallIndex) {
-  assert(CallIndex && "no call index in getCallFrame");
+std::pair
+getCallFrameAndDepth(unsigned CallIndex) {
+  assert(CallIndex && "no call index in getCallFrameAndDepth");
   // We will eventually hit BottomFrame, which has Index 1, so Frame can't
   // be null in this loop.
+  unsigned Depth = CallStackDepth;
   CallStackFrame *Frame = CurrentCall;
-  while (Frame->Index > CallIndex)
+  while (Frame->Index > CallIndex) {
 Frame = Frame->Caller;
-  return (Frame->Index == CallIndex) ? Frame : nullptr;
+--Depth;
+  }
+  if (Frame->Index == CallIndex)
+return {Frame, Depth};
+  return {nullptr, 0};
 }
 
 bool nextStep(const Stmt *S) {
@@ -,12 +1118,12 @@ namespace {
   class SpeculativeEvaluationRAII {
 EvalInfo *Info = nullptr;
 Expr::EvalStatus OldStatus;
-bool OldIsSpeculativelyEvaluating;
+unsigned OldSpeculativeEvaluationDepth;
 
 void moveFromAndCancel(SpeculativeEvaluationRAII &) {
   Info = Other.Info;
   OldStatus = Other.OldStatus;
-  OldIsSpeculativelyEvaluating = Other.OldIsSpeculativelyEvaluating;
+  OldSpeculativeEvaluationDepth = Other.OldSpeculativeEvaluationDepth;
   Other.Info = nullptr;
 }
 
@@ -1125,7 +1132,7 @@ namespace {
 return;
 
   Info->EvalStatus = OldStatus;
-  Info->IsSpeculativelyEvaluating = OldIsSpeculativelyEvaluating;
+  Info->SpeculativeEvaluationDepth = OldSpeculativeEvaluationDepth;
 }
 
   public:
@@ -1134,9 +1141,9 @@ namespace {
 SpeculativeEvaluationRAII(
 EvalInfo , SmallVectorImpl *NewDiag = 
nullptr)
 : Info(), OldStatus(Info.EvalStatus),
-  OldIsSpeculativelyEvaluating(Info.IsSpeculativelyEvaluating) {
+  OldSpeculativeEvaluationDepth(Info.SpeculativeEvaluationDepth) {
   Info.EvalStatus.Diag = NewDiag;
-  Info.IsSpeculativelyEvaluating = true;
+  Info.SpeculativeEvaluationDepth = Info.CallStackDepth + 1;
 }
 
 SpeculativeEvaluationRAII(const SpeculativeEvaluationRAII ) = delete;
@@ -3138,8 +3145,10 @@ static CompleteObject findCompleteObject
   }
 
   CallStackFrame *Frame = nullptr;
+  unsigned Depth = 0;
   if (LVal.getLValueCallIndex()) {
-Frame = Info.getCallFrame(LVal.getLValueCallIndex());
+std::tie(Frame, Depth) =
+Info.getCallFrameAndDepth(LVal.getLValueCallIndex());
 if (!Frame) {
   Info.FFDiag(E, 

[PATCH] D61545: [analyzer] Fix a crash in RVO from within blocks.

2019-05-03 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

While blocks are an Apple extension to C, i'm adding everybody because my 
mistake that i'm fixing here may hurt us more when we add more kinds of 
location contexts, so we shouldn't be making it anywhere.

In the attached test case, `getA()` returns an object by value, and therefore 
requires a construction context. Its construction context is that of a value 
that's immediately* returned (by value, without any sort of conversion), so 
copy elision (namely, RVO) applies. This means that we unwind the stack of 
`LocationContext`s in order to see where is the current callee called from. 
This way we obtain the construction context for the call site, and from that we 
can figure out what object is constructed. In this case it's going to be the 
first-and-only argument of `use()`. This is all good.

In this case RVO is applied to a return value of an anonymous block that's 
declared only to be immediately called and discarded. The Static Analyzer 
models block calls by putting two location contexts on the stack: a 
`BlockInvocationContext` followed by the actual `StackFrameContext`. I don't 
really know why it does that :) but that's not important, because if we 
introduce more kinds of location contexts, it'll look similarly anyway.

Therefore the correct idiom for obtaining the parent stack frame is to first 
obtain the parent, and then don't forget to obtain its stack frame, which is 
not necessarily the parent itself. This is the mistake that i made in my RVO 
code that i'm fixing here.

The code was crashing because it was looking up the call site in a `CFGStmtMap` 
for the wrong CFG (obtained from a wrong location context). This was happening 
in `CallEvent::getCalleeStackFrame()`.


Repository:
  rC Clang

https://reviews.llvm.org/D61545

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/copy-elision.mm


Index: clang/test/Analysis/copy-elision.mm
===
--- /dev/null
+++ clang/test/Analysis/copy-elision.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify %s
+
+// expected-no-diagnostics
+
+namespace block_rvo_crash {
+struct A {};
+struct B {};
+
+A getA();
+void use(A a) {}
+
+void foo() {
+  // This used to crash when finding construction context for getA()
+  // (which is use()'s argument due to RVO).
+  use(^{
+return getA();  // no-crash
+  }());
+}
+} // namespace block_rvo_crash
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -188,6 +188,8 @@
   // top frame of the analysis.
   const StackFrameContext *SFC = LCtx->getStackFrame();
   if (const LocationContext *CallerLCtx = SFC->getParent()) {
+// Skip non-stack-frame contexts.
+const StackFrameContext *CallerSFC = CallerLCtx->getStackFrame();
 auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
.getAs();
 if (!RTC) {
@@ -197,7 +199,7 @@
   break;
 }
 return prepareForObjectConstruction(
-cast(SFC->getCallSite()), State, CallerLCtx,
+cast(SFC->getCallSite()), State, CallerSFC,
 RTC->getConstructionContext(), CallOpts);
   } else {
 // We are on the top frame of the analysis. We do not know where is the


Index: clang/test/Analysis/copy-elision.mm
===
--- /dev/null
+++ clang/test/Analysis/copy-elision.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify %s
+
+// expected-no-diagnostics
+
+namespace block_rvo_crash {
+struct A {};
+struct B {};
+
+A getA();
+void use(A a) {}
+
+void foo() {
+  // This used to crash when finding construction context for getA()
+  // (which is use()'s argument due to RVO).
+  use(^{
+return getA();  // no-crash
+  }());
+}
+} // namespace block_rvo_crash
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -188,6 +188,8 @@
   // top frame of the analysis.
   const StackFrameContext *SFC = LCtx->getStackFrame();
   if (const LocationContext *CallerLCtx = SFC->getParent()) {
+// Skip non-stack-frame contexts.
+const StackFrameContext *CallerSFC = CallerLCtx->getStackFrame();
 auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
.getAs();
 if (!RTC) {
@@ -197,7 +199,7 @@
   

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

2019-05-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 198118.
ahatanak added a comment.

Make sure private linkage is replaced with internal linkage only when the 
object file format is MachO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61454

Files:
  lib/CodeGen/CGObjCMac.cpp
  test/CodeGenObjC/arc.m
  test/CodeGenObjC/boxing.m
  test/CodeGenObjC/exceptions-asm-attribute.m
  test/CodeGenObjC/externally-initialized-selectors.m
  test/CodeGenObjC/forward-protocol-metadata-symbols.m
  test/CodeGenObjC/instance-method-metadata.m
  test/CodeGenObjC/interface-layout-64.m
  test/CodeGenObjC/metadata-class-properties.m
  test/CodeGenObjC/metadata-symbols-32.m
  test/CodeGenObjC/metadata-symbols-64.m
  test/CodeGenObjC/metadata_symbols.m
  test/CodeGenObjC/mrc-weak.m
  test/CodeGenObjC/non-lazy-classes.m
  test/CodeGenObjC/private-extern-selector-reference.m
  test/CodeGenObjC/property-category-impl.m
  test/CodeGenObjC/property-list-in-class.m
  test/CodeGenObjC/property-list-in-extension.m
  test/CodeGenObjC/protocol-comdat.m
  test/CodeGenObjC/protocols.m
  test/CodeGenObjC/section-name.m
  test/CodeGenObjC/sections.m
  test/CodeGenObjCXX/externally-initialized-selectors.mm
  test/CodeGenObjCXX/mrc-weak.mm

Index: test/CodeGenObjCXX/mrc-weak.mm
===
--- test/CodeGenObjCXX/mrc-weak.mm
+++ test/CodeGenObjCXX/mrc-weak.mm
@@ -7,7 +7,7 @@
 @end
 
 // CHECK-MODERN: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\01\00"
-// CHECK-MODERN: @"\01l_OBJC_CLASS_RO_$_Foo" = {{.*}} { i32 772
+// CHECK-MODERN: @"_OBJC_CLASS_RO_$_Foo" = {{.*}} { i32 772
 //   772 == 0x304
 //^ HasMRCWeakIvars
 //^ HasCXXDestructorOnly
Index: test/CodeGenObjCXX/externally-initialized-selectors.mm
===
--- test/CodeGenObjCXX/externally-initialized-selectors.mm
+++ test/CodeGenObjCXX/externally-initialized-selectors.mm
@@ -1,7 +1,8 @@
-// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.5 -o - -emit-llvm %s | FileCheck %s
-// RUN: %clang_cc1 -o - -emit-llvm %s | FileCheck %s
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.5 -o - -emit-llvm %s | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -o - -emit-llvm %s | FileCheck -check-prefix=NONFRAGILE %s
 
-// CHECK: @OBJC_SELECTOR_REFERENCES_ = private externally_initialized global
+// NONFRAGILE: @OBJC_SELECTOR_REFERENCES_ = internal externally_initialized global
+// FRAGILE: @OBJC_SELECTOR_REFERENCES_ = private externally_initialized global
 
 void test(id x) {
   [x doSomething];
Index: test/CodeGenObjC/sections.m
===
--- test/CodeGenObjC/sections.m
+++ test/CodeGenObjC/sections.m
@@ -34,39 +34,42 @@
   return [I class] == @protocol(P);
 }
 
-// CHECK-COFF: @"OBJC_CLASSLIST_SUP_REFS_$_" = {{.*}}, section ".objc_superrefs$B"
-// CHECK-COFF: @OBJC_SELECTOR_REFERENCES_ = {{.*}}, section ".objc_selrefs$B"
-// CHECK-COFF: @"OBJC_CLASSLIST_REFERENCES_$_" = {{.*}}, section ".objc_classrefs$B"
-// CHECK-COFF: @"\01l_objc_msgSend_fixup_class" = {{.*}}, section ".objc_msgrefs$B"
+// CHECK-COFF: @"_OBJC_$_CLASS_METHODS_I" = private
+// CHECK-COFF: @"OBJC_CLASSLIST_SUP_REFS_$_" = private {{.*}}, section ".objc_superrefs$B"
+// CHECK-COFF: @OBJC_SELECTOR_REFERENCES_ = private {{.*}}, section ".objc_selrefs$B"
+// CHECK-COFF: @"OBJC_CLASSLIST_REFERENCES_$_" = private {{.*}}, section ".objc_classrefs$B"
+// CHECK-COFF: @_objc_msgSend_fixup_class = {{.*}}, section ".objc_msgrefs$B"
 // CHECK-COFF: @"_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section ".objc_protolist$B"
-// CHECK-COFF: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P" = {{.*}}, section ".objc_protorefs$B"
-// CHECK-COFF: @"OBJC_LABEL_CLASS_$" = {{.*}}, section ".objc_classlist$B"
-// CHECK-COFF: @"OBJC_LABEL_NONLAZY_CLASS_$" = {{.*}}, section ".objc_nlclslist$B"
-// CHECK-COFF: @"OBJC_LABEL_CATEGORY_$" = {{.*}}, section ".objc_catlist$B"
-// CHECK-COFF: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = {{.*}}, section ".objc_nlcatlist$B"
+// CHECK-COFF: @"_OBJC_PROTOCOL_REFERENCE_$_P" = {{.*}}, section ".objc_protorefs$B"
+// CHECK-COFF: @"OBJC_LABEL_CLASS_$" = private {{.*}}, section ".objc_classlist$B"
+// CHECK-COFF: @"OBJC_LABEL_NONLAZY_CLASS_$" = private {{.*}}, section ".objc_nlclslist$B"
+// CHECK-COFF: @"OBJC_LABEL_CATEGORY_$" = private {{.*}}, section ".objc_catlist$B"
+// CHECK-COFF: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private {{.*}}, section ".objc_nlcatlist$B"
 // CHECK-COFF: !{{[0-9]+}} = !{i32 1, !"Objective-C Image Info Section", !".objc_imageinfo$B"}
 
-// CHECK-ELF: @"OBJC_CLASSLIST_SUP_REFS_$_" = {{.*}}, section "objc_superrefs"
-// CHECK-ELF: @OBJC_SELECTOR_REFERENCES_ = {{.*}}, section "objc_selrefs"
-// CHECK-ELF: @"OBJC_CLASSLIST_REFERENCES_$_" = {{.*}}, section "objc_classrefs"
-// CHECK-ELF: @"\01l_objc_msgSend_fixup_class" = {{.*}}, section "objc_msgrefs"
+// CHECK-ELF: 

r359954 - [Driver] Create non-existent directory for -fcrash-diagnostics-dir

2019-05-03 Thread Petr Hosek via cfe-commits
Author: phosek
Date: Fri May  3 17:55:14 2019
New Revision: 359954

URL: http://llvm.org/viewvc/llvm-project?rev=359954=rev
Log:
[Driver] Create non-existent directory for -fcrash-diagnostics-dir

When user specifies non-existent directory to -fcrash-diagnostics-dir,
create it rather than failing with an error as would be the case before.

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/crash-diagnostics-dir.c

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=359954=359953=359954=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri May  3 17:55:14 2019
@@ -4263,6 +4263,8 @@ const char *Driver::GetNamedOutputPath(C
 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
 if (CCGenDiagnostics && A) {
   SmallString<128> CrashDirectory(A->getValue());
+  if (!getVFS().exists(CrashDirectory))
+llvm::sys::fs::create_directories(CrashDirectory);
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
   std::error_code EC = llvm::sys::fs::createUniqueFile(

Modified: cfe/trunk/test/Driver/crash-diagnostics-dir.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-diagnostics-dir.c?rev=359954=359953=359954=diff
==
--- cfe/trunk/test/Driver/crash-diagnostics-dir.c (original)
+++ cfe/trunk/test/Driver/crash-diagnostics-dir.c Fri May  3 17:55:14 2019
@@ -1,5 +1,4 @@
 // RUN: rm -rf %t
-// RUN: mkdir -p %t
 // RUN: not %clang -fcrash-diagnostics-dir=%t -c %s -o - 2>&1 | FileCheck %s
 #pragma clang __debug parser_crash
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:


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


[PATCH] D61542: [Driver] Create non-existent directory for -fcrash-diagnostics-dir

2019-05-03 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359954: [Driver] Create non-existent directory for 
-fcrash-diagnostics-dir (authored by phosek, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61542?vs=198109=198117#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61542

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/crash-diagnostics-dir.c


Index: cfe/trunk/test/Driver/crash-diagnostics-dir.c
===
--- cfe/trunk/test/Driver/crash-diagnostics-dir.c
+++ cfe/trunk/test/Driver/crash-diagnostics-dir.c
@@ -1,5 +1,4 @@
 // RUN: rm -rf %t
-// RUN: mkdir -p %t
 // RUN: not %clang -fcrash-diagnostics-dir=%t -c %s -o - 2>&1 | FileCheck %s
 #pragma clang __debug parser_crash
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -4263,6 +4263,8 @@
 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
 if (CCGenDiagnostics && A) {
   SmallString<128> CrashDirectory(A->getValue());
+  if (!getVFS().exists(CrashDirectory))
+llvm::sys::fs::create_directories(CrashDirectory);
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
   std::error_code EC = llvm::sys::fs::createUniqueFile(


Index: cfe/trunk/test/Driver/crash-diagnostics-dir.c
===
--- cfe/trunk/test/Driver/crash-diagnostics-dir.c
+++ cfe/trunk/test/Driver/crash-diagnostics-dir.c
@@ -1,5 +1,4 @@
 // RUN: rm -rf %t
-// RUN: mkdir -p %t
 // RUN: not %clang -fcrash-diagnostics-dir=%t -c %s -o - 2>&1 | FileCheck %s
 #pragma clang __debug parser_crash
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -4263,6 +4263,8 @@
 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
 if (CCGenDiagnostics && A) {
   SmallString<128> CrashDirectory(A->getValue());
+  if (!getVFS().exists(CrashDirectory))
+llvm::sys::fs::create_directories(CrashDirectory);
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
   std::error_code EC = llvm::sys::fs::createUniqueFile(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359953 - [cxx_status] Don't list -fmodules / -fmodules-ts against C++ modules

2019-05-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri May  3 17:27:21 2019
New Revision: 359953

URL: http://llvm.org/viewvc/llvm-project?rev=359953=rev
Log:
[cxx_status] Don't list -fmodules / -fmodules-ts against C++ modules
support; those turn on different modules modes. The real C++ modules
support is behind -std=c++2a like the rest of C++20.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=359953=359952=359953=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Fri May  3 17:27:21 2019
@@ -1055,7 +1055,7 @@ as the draft C++2a standard evolves.
 
   Modules
   http://wg21.link/p1103r3;>P1103R3
-  Partial (-fmodules, 
-fmodules-ts)
+  Partial
 
 
   Coroutines


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


r359951 - [cxx_status] Replace "SVN" entries with Clang 8 as appropriate.

2019-05-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri May  3 17:23:18 2019
New Revision: 359951

URL: http://llvm.org/viewvc/llvm-project?rev=359951=rev
Log:
[cxx_status] Replace "SVN" entries with Clang 8 as appropriate.

Also: use the "svn" color for "explicit(bool)" rather than the "full" color.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=359951=359950=359951=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Fri May  3 17:23:18 2019
@@ -211,7 +211,7 @@ with http://libcxx.llvm.org/;>l
   
 
 http://wg21.link/p0859r0;>P0859R0 (DR)
-SVN
+Clang 8
   
 
   Alignment support
@@ -326,7 +326,7 @@ with http://libcxx.llvm.org/;>l
   
 
 http://wg21.link/p0962r1;>P0962R1 (DR)
-SVN
+Clang 8
   
 
   Explicit virtual overrides
@@ -763,12 +763,12 @@ version 3.7.
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-SVN
+Clang 8
   
   
 
 http://wg21.link/p0969r0;>P0969R0 (DR)
-SVN
+Clang 8
   
 
   Separate variable and condition for if and 
switch
@@ -888,7 +888,7 @@ as the draft C++2a standard evolves.
 
   Range-based for statements with initializer
   http://wg21.link/p0614r1;>P0614R1
-  SVN
+  Clang 8
 
 
   ADL and function templates that are not visible
@@ -898,7 +898,7 @@ as the draft C++2a standard evolves.
 
   const mismatch with defaulted copy constructor
   http://wg21.link/p0641r2;>P0641R2
-  SVN
+  Clang 8
 
 
   Consistent comparison (operator=)
@@ -923,7 +923,7 @@ as the draft C++2a standard evolves.
 
   Default constructible and assignable stateless lambdas
   http://wg21.link/p0624r2;>P0624R2
-  SVN
+  Clang 8
 
 
   Lambdas in unevaluated contexts
@@ -969,7 +969,7 @@ as the draft C++2a standard evolves.
 

 http://wg21.link/p1002r1;>P1002R1
-SVN
+Clang 8
   
   
 http://wg21.link/p1327r1;>P1327R1
@@ -981,7 +981,7 @@ as the draft C++2a standard evolves.
 
   Prohibit aggregates with user-declared constructors
   http://wg21.link/p1008r1;>P1008R1
-  SVN
+  Clang 8
 
 
   Contracts
@@ -1002,7 +1002,7 @@ as the draft C++2a standard evolves.
 
   explicit(bool)
   http://wg21.link/p0892r2;>P0892R2
-  SVN
+  SVN
 
 
 
@@ -1028,7 +1028,7 @@ as the draft C++2a standard evolves.
 
   Nested inline namespaces
   http://wg21.link/p1094r2;>P1094R2
-  SVN
+  Clang 8
 
 
 


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


Buildbot numbers for the week of 04/21/2019 - 04/27/2019

2019-05-03 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the last week of 04/21/2019 -
04/27/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 clang-cmake-aarch64-full| 87:54:29
 clang-cmake-aarch64-lld | 86:28:43
 lld-x86_64-win7 | 52:15:20
 sanitizer-x86_64-linux-bootstrap| 19:29:12
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03  | 19:26:34
 sanitizer-x86_64-linux-fast | 19:16:40
 sanitizer-x86_64-linux-bootstrap-msan   | 19:13:33
 sanitizer-x86_64-linux-bootstrap-ubsan  | 18:58:41
 lldb-x64-windows-ninja  | 18:00:18
 clang-x64-windows-msvc  | 17:17:03
 llvm-clang-x86_64-expensive-checks-win  | 17:13:09
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 16:40:23
 sanitizer-x86_64-linux  | 15:55:58
 clang-cmake-armv7-selfhost  | 15:55:23
 clang-cmake-armv7-quick | 13:06:10
 clang-cmake-thumbv7-full-sh | 11:19:45
 clang-ppc64le-linux | 11:18:34
 clang-ppc64le-linux-lnt | 10:00:40
 clang-cmake-armv8-global-isel   | 09:29:03
 clang-s390x-linux   | 09:18:55
 clang-cmake-armv7-selfhost-neon | 09:18:05
 clang-cmake-armv7-global-isel   | 09:12:43
 clang-cmake-armv8-full  | 09:03:25
 ppc64le-lld-multistage-test | 08:59:54
 clang-cmake-armv8-quick | 08:44:35
 clang-ppc64le-linux-multistage  | 08:23:29
 clang-s390x-linux-lnt   | 08:21:38
 clang-ppc64be-linux-lnt | 08:14:14
 clang-ppc64be-linux | 08:13:27
 clang-cmake-armv8-selfhost-neon | 08:05:00
 clang-lld-x86_64-2stage | 07:57:07
 clang-hexagon-elf   | 07:49:34
 clang-ppc64be-linux-multistage  | 07:44:44
 clang-cmake-armv7-full  | 07:15:25
 clang-s390x-linux-multistage| 07:13:28
 clang-cmake-aarch64-quick   | 07:04:03
 clang-cmake-aarch64-global-isel | 07:00:47
 clang-cmake-thumbv8-full-sh | 06:51:37
 clang-cmake-armv8-lld   | 05:33:24
 clang-with-lto-ubuntu   | 04:53:36
 sanitizer-x86_64-linux-android  | 04:10:38
 sanitizer-ppc64le-linux | 03:20:03
 llvm-hexagon-elf| 02:51:15
 clang-cmake-armv8-lnt   | 02:48:53
 clang-with-thin-lto-ubuntu  | 02:45:26
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu  | 02:35:09
 sanitizer-ppc64be-linux | 02:24:37
 clang-atom-d525-fedora-rel  | 02:18:13
 clang-cmake-x86_64-avx2-linux   | 02:09:12
 reverse-iteration   | 02:03:34
 clang-cmake-x86_64-avx2-linux-perf  | 02:00:50
 sanitizer-x86_64-linux-fuzzer   | 02:00:45
 lld-x86_64-ubuntu-fast  | 02:00:05
 clang-cuda-build| 01:50:01
 sanitizer-x86_64-linux-autoconf | 01:47:07
 clang-cmake-x86_64-sde-avx512-linux | 01:44:58
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 01:44:35
 sanitizer-windows   | 01:42:57
 clang-x86_64-linux-abi-test | 01:41:11
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11 | 00:29:19
 llvm-sphinx-docs| 00:28:14
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 00:23:46
 

Buildbot numbers for the week of 04/07/2019 - 04/13/2019

2019-05-03 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 04/07/2019 - 04/13/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 lld-sphinx-docs | 95:10:46
 llvm-sphinx-docs| 90:32:22
 clang-cmake-aarch64-quick   | 73:24:35
 clang-cmake-aarch64-global-isel | 73:24:27
 sanitizer-x86_64-linux-autoconf | 24:06:01
 clang-ppc64le-linux-multistage  | 15:32:24
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a  | 14:44:24
 clang-cmake-armv8-lld   | 14:20:27
 llvm-clang-x86_64-expensive-checks-win  | 13:49:17
 clang-x64-windows-msvc  | 13:23:49
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 13:10:59
 lldb-x64-windows-ninja  | 12:40:51
 clang-cmake-thumbv7-full-sh | 12:35:55
 lld-x86_64-win7 | 12:34:09
 clang-s390x-linux-multistage| 12:01:03
 clang-cmake-thumbv8-full-sh | 11:49:14
 sanitizer-ppc64le-linux | 11:31:52
 clang-cmake-armv7-full  | 11:13:39
 clang-ppc64le-linux-lnt | 11:06:33
 clang-with-lto-ubuntu   | 10:47:40
 clang-cmake-armv8-full  | 10:37:08
 clang-ppc64be-linux-multistage  | 10:34:52
 clang-ppc64le-linux | 10:25:28
 clang-s390x-linux-lnt   | 10:23:48
 clang-ppc64be-linux-lnt | 10:14:07
 clang-ppc64be-linux | 10:09:34
 clang-with-thin-lto-ubuntu  | 10:09:04
 sanitizer-x86_64-linux  | 10:01:54
 clang-s390x-linux   | 10:01:21
 sanitizer-ppc64be-linux | 09:51:27
 clang-atom-d525-fedora-rel  | 09:20:09
 clang-cmake-armv8-global-isel   | 09:14:31
 ppc64le-lld-multistage-test | 09:09:33
 clang-cuda-build| 09:02:49
 clang-cmake-armv8-quick | 08:51:29
 sanitizer-x86_64-linux-bootstrap-msan   | 08:50:32
 clang-cmake-armv7-quick | 08:50:08
 clang-cmake-armv7-global-isel   | 08:48:05
 reverse-iteration   | 08:45:59
 sanitizer-x86_64-linux-fast | 08:44:55
 clang-cmake-armv7-selfhost  | 08:19:50
 clang-cmake-armv8-selfhost-neon | 08:10:53
 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan   | 06:36:40
 clang-cmake-armv7-selfhost-neon | 06:31:02
 clang-lld-x86_64-2stage | 06:11:58
 sanitizer-x86_64-linux-bootstrap-ubsan  | 05:18:38
 sanitizer-windows   | 04:51:24
 sanitizer-x86_64-linux-bootstrap| 04:19:44
 clang-cmake-armv7-lnt   | 02:56:58
 clang-x86_64-linux-abi-test | 02:54:47
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 02:50:41
 llvm-hexagon-elf| 02:23:32
 clang-cmake-armv8-lnt   | 02:15:36
 clang-hexagon-elf   | 02:07:35
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 02:05:26
 clang-cmake-x86_64-sde-avx512-linux | 01:52:03
 clang-cmake-x86_64-avx2-linux   | 01:40:33
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17  | 00:55:17
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14  | 00:53:19
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11  | 00:53:15
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan   | 00:52:51
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan   | 00:50:52
 

Buildbot numbers for the week of 04/14/2019 - 04/20/2019

2019-05-03 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 04/14/2019 - 04/20/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername|  was_red
--+--
 aosp-O3-polly-before-vectorizer-unprofitable | 120:00:26
 clang-cmake-aarch64-global-isel  | 44:18:18
 clang-cmake-aarch64-quick| 41:19:16
 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan| 26:46:40
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu   | 26:43:14
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14   | 26:42:46
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17   | 23:51:13
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a   | 23:50:55
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit   | 23:48:21
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan   | 23:48:21
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11   | 19:58:12
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11  | 19:56:25
 clang-with-lto-ubuntu| 17:46:55
 clang-with-thin-lto-ubuntu   | 15:30:41
 ppc64le-lld-multistage-test  | 15:16:37
 clang-lld-x86_64-2stage  | 15:07:36
 clang-cmake-armv7-selfhost   | 08:32:50
 clang-x64-windows-msvc   | 07:26:27
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 07:09:15
 clang-cmake-armv7-selfhost-neon  | 06:58:49
 clang-cmake-thumbv7-full-sh  | 06:32:58
 llvm-clang-x86_64-expensive-checks-win   | 06:02:23
 lld-x86_64-win7  | 05:55:43
 sanitizer-x86_64-linux-bootstrap-ubsan   | 05:16:12
 clang-cmake-thumbv8-full-sh  | 05:09:56
 clang-cmake-armv8-lld| 05:08:40
 sanitizer-x86_64-linux-android   | 05:03:46
 sanitizer-x86_64-linux-fast  | 04:41:56
 sanitizer-x86_64-linux-bootstrap | 04:36:05
 clang-cuda-build | 04:31:00
 clang-hexagon-elf| 04:27:31
 llvm-hexagon-elf | 04:26:04
 reverse-iteration| 04:24:51
 clang-ppc64be-linux-multistage   | 04:22:46
 lld-x86_64-darwin13  | 04:22:03
 clang-ppc64be-linux  | 04:20:38
 lld-x86_64-freebsd   | 04:20:12
 clang-atom-d525-fedora-rel   | 04:20:08
 clang-cmake-x86_64-sde-avx512-linux  | 04:20:02
 clang-s390x-linux| 04:19:50
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 04:19:48
 clang-ppc64be-linux-lnt  | 04:19:13
 clang-cmake-x86_64-avx2-linux| 04:16:25
 sanitizer-ppc64le-linux  | 04:11:11
 clang-cmake-armv8-quick  | 04:10:29
 clang-cmake-armv8-full   | 04:05:55
 sanitizer-x86_64-linux-bootstrap-msan| 04:05:07
 clang-ppc64le-linux-multistage   | 04:01:50
 clang-s390x-linux-lnt| 03:58:44
 sanitizer-x86_64-linux   | 03:52:03
 clang-s390x-linux-multistage | 03:48:20
 clang-ppc64le-linux-lnt  | 03:45:24
 clang-cmake-armv8-global-isel| 03:40:57
 clang-cmake-armv7-global-isel| 03:11:37
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan| 03:03:13
 clang-cmake-armv8-selfhost-neon  | 03:02:42
 clang-ppc64le-linux  | 02:52:29
 sanitizer-ppc64be-linux  | 02:13:18
 clang-cmake-armv7-quick  | 02:06:58
 polly-amd64-linux| 01:57:06
 clang-cmake-armv7-full   | 01:24:19
 sanitizer-windows| 01:16:48
 clang-cmake-armv7-lnt| 01:15:27
 sanitizer-x86_64-linux-autoconf  | 01:06:15
 clang-cmake-armv8-lnt| 01:04:34
 sanitizer-x86_64-linux-fuzzer| 01:00:37
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03   | 00:55:51
 lldb-x64-windows-ninja   | 00:53:58
 lld-perf-testsuite   | 00:50:03
 clang-aarch64-linux-build-cache  

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In D61165#1490608 , @rjmccall wrote:

> The flip side of that argument is that (1) there aren't very many users right 
> now and (2) it's much easier to start conservative and then weaken the rule 
> than it will be to strengthen it later.  It really isn't acceptable to just 
> turn off access/use-checking for the destructor, so if we get trapped by the 
> choice we make here, we'll end up having to either leak or call 
> `std::terminate`.


I agree that'd we'd be much better off if we had to change our minds and relax 
the requirement here. Though we haven't been pushing on this, I would disagree 
with the point that there aren't many users, this was included in an open 
source release, an Xcode release, and there was a wg21 paper about it. That 
paper is currently the first result on google for the search "disabling static 
destructors". Hedging our bets here is an option, but I'd really like to avoid 
it if we can.


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

https://reviews.llvm.org/D61165



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


[PATCH] D61542: [Driver] Create non-existent directory for -fcrash-diagnostics-dir

2019-05-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D61542



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


[PATCH] D61345: Allow 'CodeGenObjC/illegal-UTF8.m' test for 32-bit targets.

2019-05-03 Thread Galina via Phabricator via cfe-commits
gkistanova accepted this revision.
gkistanova added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this, Vlad!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61345



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


r359947 - CWG issue 727: Fix numerous bugs in support for class-scope explicit

2019-05-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri May  3 16:51:38 2019
New Revision: 359947

URL: http://llvm.org/viewvc/llvm-project?rev=359947=rev
Log:
CWG issue 727: Fix numerous bugs in support for class-scope explicit
specializations for variable templates.

Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Sema/Template.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CXX/drs/dr7xx.cpp
cfe/trunk/test/PCH/cxx-templates.cpp
cfe/trunk/test/PCH/cxx-templates.h
cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=359947=359946=359947=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri May  3 16:51:38 2019
@@ -1434,6 +1434,12 @@ public:
   /// template specialization or instantiation this is.
   TemplateSpecializationKind getTemplateSpecializationKind() const;
 
+  /// Get the template specialization kind of this variable for the purposes of
+  /// template instantiation. This differs from getTemplateSpecializationKind()
+  /// for an instantiation of a class-scope explicit specialization.
+  TemplateSpecializationKind
+  getTemplateSpecializationKindForInstantiation() const;
+
   /// If this variable is an instantiation of a variable template or a
   /// static data member of a class template, determine its point of
   /// instantiation.

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=359947=359946=359947=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri May  3 16:51:38 2019
@@ -8043,7 +8043,8 @@ public:
  LateInstantiatedAttrVec *LateAttrs,
  DeclContext *Owner,
  LocalInstantiationScope *StartingScope,
- bool InstantiatingVarTemplate = false);
+ bool InstantiatingVarTemplate = false,
+ VarTemplateSpecializationDecl *PrevVTSD = 
nullptr);
   void InstantiateVariableInitializer(
   VarDecl *Var, VarDecl *OldVar,
   const MultiLevelTemplateArgumentList );

Modified: cfe/trunk/include/clang/Sema/Template.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Template.h?rev=359947=359946=359947=diff
==
--- cfe/trunk/include/clang/Sema/Template.h (original)
+++ cfe/trunk/include/clang/Sema/Template.h Fri May  3 16:51:38 2019
@@ -545,7 +545,8 @@ class VarDecl;
 Decl *VisitVarTemplateSpecializationDecl(
 VarTemplateDecl *VarTemplate, VarDecl *FromVar, void *InsertPos,
 const TemplateArgumentListInfo ,
-ArrayRef Converted);
+ArrayRef Converted,
+VarTemplateSpecializationDecl *PrevDecl = nullptr);
 
 Decl *InstantiateTypedefNameDecl(TypedefNameDecl *D, bool IsTypeAlias);
 ClassTemplatePartialSpecializationDecl *

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=359947=359946=359947=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri May  3 16:51:38 2019
@@ -2414,48 +2414,61 @@ bool VarDecl::isNonEscapingByref() const
 }
 
 VarDecl *VarDecl::getTemplateInstantiationPattern() const {
-  // If it's a variable template specialization, find the template or partial
-  // specialization from which it was instantiated.
-  if (auto *VDTemplSpec = dyn_cast(this)) {
-auto From = VDTemplSpec->getInstantiatedFrom();
-if (auto *VTD = From.dyn_cast()) {
-  while (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate()) {
-if (NewVTD->isMemberSpecialization())
-  break;
-VTD = NewVTD;
-  }
-  return getDefinitionOrSelf(VTD->getTemplatedDecl());
-}
-if (auto *VTPSD =
-From.dyn_cast()) {
-  while (auto *NewVTPSD = VTPSD->getInstantiatedFromMember()) {
-if (NewVTPSD->isMemberSpecialization())
-  break;
-VTPSD = NewVTPSD;
-  }
-  return getDefinitionOrSelf(VTPSD);
-}
-  }
+  const VarDecl *VD = this;
 
-  if (MemberSpecializationInfo *MSInfo = getMemberSpecializationInfo()) {
+  // If this is an instantiated member, walk back to the template from which
+  // it was instantiated.
+  if (MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo()) {
 if 

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

The flip side of that argument is that (1) there aren't very many users right 
now and (2) it's much easier to start conservative and then weaken the rule 
than it will be to strengthen it later.  It really isn't acceptable to just 
turn off access/use-checking for the destructor, so if we get trapped by the 
choice we make here, we'll end up having to either leak or call 
`std::terminate`.


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

https://reviews.llvm.org/D61165



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


[PATCH] D61542: [Driver] Create non-existent directory for -fcrash-diagnostics-dir

2019-05-03 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: hans, inglorion, rnk, thakis.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When user specifies non-existent directory to -fcrash-diagnostics-dir,
create it rather than failing with an error.


Repository:
  rC Clang

https://reviews.llvm.org/D61542

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/crash-diagnostics-dir.c


Index: clang/test/Driver/crash-diagnostics-dir.c
===
--- clang/test/Driver/crash-diagnostics-dir.c
+++ clang/test/Driver/crash-diagnostics-dir.c
@@ -1,5 +1,4 @@
 // RUN: rm -rf %t
-// RUN: mkdir -p %t
 // RUN: not %clang -fcrash-diagnostics-dir=%t -c %s -o - 2>&1 | FileCheck %s
 #pragma clang __debug parser_crash
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4263,6 +4263,8 @@
 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
 if (CCGenDiagnostics && A) {
   SmallString<128> CrashDirectory(A->getValue());
+  if (!getVFS().exists(CrashDirectory))
+llvm::sys::fs::create_directories(CrashDirectory);
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
   std::error_code EC = llvm::sys::fs::createUniqueFile(


Index: clang/test/Driver/crash-diagnostics-dir.c
===
--- clang/test/Driver/crash-diagnostics-dir.c
+++ clang/test/Driver/crash-diagnostics-dir.c
@@ -1,5 +1,4 @@
 // RUN: rm -rf %t
-// RUN: mkdir -p %t
 // RUN: not %clang -fcrash-diagnostics-dir=%t -c %s -o - 2>&1 | FileCheck %s
 #pragma clang __debug parser_crash
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4263,6 +4263,8 @@
 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
 if (CCGenDiagnostics && A) {
   SmallString<128> CrashDirectory(A->getValue());
+  if (!getVFS().exists(CrashDirectory))
+llvm::sys::fs::create_directories(CrashDirectory);
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
   std::error_code EC = llvm::sys::fs::createUniqueFile(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 198108.
gtbercea added a comment.

- Add new tests. Add stub headers.
- Remove old tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_cuda_cmath.h
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_libdevice_declares.h
  lib/Headers/__clang_cuda_math_forward_declares.h
  lib/Headers/openmp_wrappers/__clang_openmp_math.h
  lib/Headers/openmp_wrappers/cmath
  lib/Headers/openmp_wrappers/math.h
  test/Driver/openmp-offload-gpu.c
  test/Headers/Inputs/include/cmath
  test/Headers/Inputs/include/limits
  test/Headers/Inputs/include/math.h
  test/Headers/nvptx_device_cmath_functions.c
  test/Headers/nvptx_device_cmath_functions.cpp
  test/Headers/nvptx_device_math_functions.c
  test/Headers/nvptx_device_math_functions.cpp

Index: test/Headers/nvptx_device_math_functions.cpp
===
--- /dev/null
+++ test/Headers/nvptx_device_math_functions.cpp
@@ -0,0 +1,21 @@
+// Test calling of device math functions.
+///==///
+
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include math.h -x c++ -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -internal-isystem %S/Inputs/include -include stdlib.h -include limits -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+
+#include 
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @__nv_sqrt(double
+double l1 = sqrt(a1);
+// CHECK-YES: call double @__nv_pow(double
+double l2 = pow(a1, a1);
+// CHECK-YES: call double @__nv_modf(double
+double l3 = modf(a1 + 3.5, );
+  }
+}
Index: test/Headers/nvptx_device_math_functions.c
===
--- /dev/null
+++ test/Headers/nvptx_device_math_functions.c
@@ -0,0 +1,21 @@
+// Test calling of device math functions.
+///==///
+
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include math.h -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include math.h -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+
+#include 
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @__nv_sqrt(double
+double l1 = sqrt(a1);
+// CHECK-YES: call double @__nv_pow(double
+double l2 = pow(a1, a1);
+// CHECK-YES: call double @__nv_modf(double
+double l3 = modf(a1 + 3.5, );
+  }
+}
Index: test/Headers/nvptx_device_cmath_functions.cpp
===
--- /dev/null
+++ test/Headers/nvptx_device_cmath_functions.cpp
@@ -0,0 +1,21 @@
+// Test calling of device math functions.
+///==///
+
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include -include cmath -x c++ -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include cmath -internal-isystem %S/Inputs/include -include stdlib.h -x c++ -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck -check-prefix CHECK-YES %s
+
+#include 
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @__nv_sqrt(double
+double l1 = sqrt(a1);
+// CHECK-YES: call double @__nv_pow(double
+double l2 = pow(a1, a1);
+// CHECK-YES: call double @__nv_modf(double
+double l3 = modf(a1 + 3.5, );
+  }
+}
Index: test/Headers/nvptx_device_cmath_functions.c
===
--- /dev/null
+++ test/Headers/nvptx_device_cmath_functions.c
@@ -0,0 +1,21 @@
+// Test calling of device math functions.

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

2019-05-03 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359943: [clang-format] Fix bug in block comment reflow that 
joins * and / (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61276?vs=198057=198107#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61276

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/BreakableToken.h
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle , bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex ) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -65,7 +65,8 @@
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle ) {
+encoding::Encoding Encoding, const FormatStyle ,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex ) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11150,6 +11150,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"


Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle , bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex ) const override;

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 198105.
gtbercea added a comment.

- Add driver test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_cuda_cmath.h
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_libdevice_declares.h
  lib/Headers/__clang_cuda_math_forward_declares.h
  lib/Headers/openmp_wrappers/__clang_openmp_math.h
  lib/Headers/openmp_wrappers/cmath
  lib/Headers/openmp_wrappers/math.h
  test/CodeGen/nvptx_device_cmath_functions.c
  test/CodeGen/nvptx_device_math_functions.c
  test/Driver/openmp-offload-gpu.c

Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -278,3 +278,8 @@
 // RUN:   | FileCheck -check-prefix=CUDA_RED_RECS %s
 // CUDA_RED_RECS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // CUDA_RED_RECS-SAME: "-fopenmp-cuda-teams-reduction-recs-num=2048"
+
+// RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OPENMP_NVPTX_WRAPPERS %s
+// OPENMP_NVPTX_WRAPPERS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
+// OPENMP_NVPTX_WRAPPERS-SAME: "-internal-isystem" "{{.*}}openmp_wrappers"
Index: test/CodeGen/nvptx_device_math_functions.c
===
--- /dev/null
+++ test/CodeGen/nvptx_device_math_functions.c
@@ -0,0 +1,24 @@
+// Test calling of device math functions.
+///==///
+
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -fmath-errno -S -emit-llvm -o - %s -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda | FileCheck -check-prefix CHECK-YES %s
+#include 
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @llvm.nvvm.sqrt.rn.d(double
+double l1 = sqrt(a1);
+// CHECK-YES: call double @__internal_accurate_pow(double
+double l2 = pow(a1, a1);
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call double @llvm.nvvm.trunc.d(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.lo(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call double @llvm.nvvm.lohi.i2d(i32
+double l3 = modf(a1 + 3.5, );
+  }
+}
Index: test/CodeGen/nvptx_device_cmath_functions.c
===
--- /dev/null
+++ test/CodeGen/nvptx_device_cmath_functions.c
@@ -0,0 +1,24 @@
+// Test calling of device math functions.
+///==///
+
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang++ -fmath-errno -S -emit-llvm -o - %s -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda | FileCheck -check-prefix CHECK-YES %s
+#include 
+
+void test_math_funcs(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @llvm.nvvm.sqrt.rn.d(double
+double l1 = sqrt(a1);
+// CHECK-YES: call double @__internal_accurate_pow(double
+double l2 = pow(a1, a1);
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call double @llvm.nvvm.trunc.d(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.lo(double
+// CHECK-YES: call i32 @llvm.nvvm.d2i.hi(double
+// CHECK-YES: call double @llvm.nvvm.lohi.i2d(i32
+double l3 = modf(a1 + 3.5, );
+  }
+}
Index: lib/Headers/openmp_wrappers/math.h
===
--- /dev/null
+++ lib/Headers/openmp_wrappers/math.h
@@ -0,0 +1,17 @@
+/*===- math.h - Alternative math.h header --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#include <__clang_openmp_math.h>
+
+#ifndef __CLANG_NO_HOST_MATH__
+#include_next 
+#else
+#undef __CLANG_NO_HOST_MATH__
+#endif
+
Index: lib/Headers/openmp_wrappers/cmath
===
--- /dev/null
+++ lib/Headers/openmp_wrappers/cmath
@@ -0,0 +1,16 @@
+/*===-- cmath - Alternative cmath header ---===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#include <__clang_openmp_math.h>
+
+#ifndef 

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

2019-05-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan updated this revision to Diff 198057.
owenpan added a comment.

Updated the test cases to make them precise and more varied.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle , bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex ) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -65,7 +65,8 @@
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle ) {
+encoding::Encoding Encoding, const FormatStyle ,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex ) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   

r359943 - [clang-format] Fix bug in block comment reflow that joins * and /

2019-05-03 Thread Owen Pan via cfe-commits
Author: owenpan
Date: Fri May  3 16:15:40 2019
New Revision: 359943

URL: http://llvm.org/viewvc/llvm-project?rev=359943=rev
Log:
[clang-format] Fix bug in block comment reflow that joins * and /

Fixes PR41213

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/BreakableToken.h
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=359943=359942=359943=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Fri May  3 16:15:40 2019
@@ -65,7 +65,8 @@ static StringRef getLineCommentIndentPre
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle ) {
+encoding::Encoding Encoding, const FormatStyle ,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@ getCommentSplit(StringRef Text, unsigned
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@ BreakableBlockComment::BreakableBlockCom
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex ) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment

Modified: cfe/trunk/lib/Format/BreakableToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=359943=359942=359943=diff
==
--- cfe/trunk/lib/Format/BreakableToken.h (original)
+++ cfe/trunk/lib/Format/BreakableToken.h Fri May  3 16:15:40 2019
@@ -361,6 +361,9 @@ public:
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle , bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex ) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=359943=359942=359943=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri May  3 16:15:40 2019
@@ -11150,6 +11150,24 @@ TEST_F(FormatTest, OptimizeBreakPenaltyV
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"


___
cfe-commits mailing list

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

I've only been lurking but FWIW (1) above makes the most sense to me, unless 
the Standard clearly draws a distinction between *constructed* and 
*initialized* in the way that was described, in which case (3) is the right 
approach. However, I would wait for at least a CWG issue to be filed to clarify 
the intent of the standard before adopting (3), otherwise it seems like we're 
adopting a slightly surprising behavior (and also one that's different from 
GCC) on a presumption of intent.

So for now I'd personally go with (1) and consider it a bugfix if the Standard 
decides to clarify intent in a way that (3) is the right thing to do -- we'll 
already have to change stuff anyway if that happens.

Also, I would personally be keen on potentially breaking source compatibility 
by doing access checking, as it's not clear to me at all that this is going to 
cause any actual breakage in the real world given the age and narrowness of the 
attribute.

Just my .02


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

https://reviews.llvm.org/D61165



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


[PATCH] D61506: [OpenCL] Switch to C++17

2019-05-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Per the OpenCL C++ 1.0 specification, section 2:

> The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 N 
> 3690 language specification (a.k.a. C++14 specification).

I think it would be reasonable to permit changing the base C++ standard in 
OpenCL C++ mode, but we'd need a good reason to deviate from the behavior 
specified in the OpenCL C++ specification by default.


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

https://reviews.llvm.org/D61506



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


[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

2019-05-03 Thread Casey Carter via Phabricator via cfe-commits
CaseyCarter added inline comments.



Comment at: 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:159
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);

This will blow up non-MSVC-ish when running the test suite with `-Wall -W 
-Werror` (which is typical). I suggest wrapping in `#ifdef _MSC_VER`.


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

https://reviews.llvm.org/D61365



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


[PATCH] D54784: Use --push/pop-state with XRay link deps

2019-05-03 Thread Petr Hosek via Phabricator via cfe-commits
phosek abandoned this revision.
phosek added a comment.
Herald added a project: clang.

Turned out this is not the right approach because not every linker supports 
`--push-state`/`--pop-state` but I'm hoping to instead rely on ELF autolinking 
once D60274  lands.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54784



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


[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D61165#1490417 , @erik.pilkington 
wrote:

> In D61165#1490168 , @rjmccall wrote:
>
> > I think the intuitive rule is that initialization is complete when the 
> > full-expression performing the initialization is complete because that's 
> > the normal unit of sequencing.  Note that the standard does use both 
> > "constructed" and "initialized" in different contexts, although this might 
> > just be an editorial choice.
>
>
> I think it would be quite subtle if the standard was deliberately splitting 
> this particular hair by implicitly creating a "constructed, but not 
> initialized" state of an object, without calling that out anywhere :)


Well, but you're assuming that the standard is just using two different words 
for the same concept, often in close proximity.  That's probably against some 
canon of interpretation.

> As far as I see it, we have three options here:
> 
> 1. Claim that the object is formally initialized when the constructor returns
> 
>   This appears to be what GCC implements.

Outside of aggregate initialization, yes.  For aggregate initialization, GCC 
appears to treat the object as fully initialized as soon as its last member is 
constructed; until that point, no destructors are run for fully-constructed 
previous members, which is obviously a bug at some level.

I guess my constructed-vs-initialized rule would naturally suggest that 
exceptions thrown by full-expression destructors for the last member's 
initializer cause all the members to be individually destroyed without invoking 
the aggregate's destructor.  (Somewhat oddly, aggregates can have user-defined 
destructors, so this is a detectable difference.)  This is a little weird.

> 2. Claim that the object is formally initialized when the full-expression 
> ends, and if a temporary throws don't call the destructor because the object 
> isn't initialized.
> 
>   This is what Clang implements today, but seems wrong.

Yes, I acknowledged way upthread that this is a bug.

> 3. Claim that the object is formally initialized when the full-expression 
> ends, but if a temporary throws call the destructor because the constructor 
> ran.
> 
>   This seems weird to me. If destroying temporaries is an indivisible part of 
> the initialization of an object, then we shouldn't be able to call the 
> destructor, because the initialization of the object didn't succeed. (I mean, 
> assuming there isn't a distinction between constructed and initialized)

I think drawing that distinction is a necessary precursor to applying my rule.

>> there really can't be *that* many uses of this feature yet, especially in 
>> exceptions-enabled code. We can fix semantic mistakes.
> 
> Yeah, if we assert rule 3 then we I guess we should just do this, rather than 
> try to determine whether we need the dtor for the static local in Sema.
> 
> Anyways, I think I've layed out my thinking here as clearly as I can. If you 
> still think that 3 is right here, then I'm happy to defer to you (although it 
> would be quite nice if @rsmith chimed in too). I'm happy to implement 
> whatever the right thing for CodeGen to do here is too.

I would also be interested in getting Richard's opinion on this.


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

https://reviews.llvm.org/D61165



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


[PATCH] D60974: Clang IFSO driver action.

2019-05-03 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

@rupprecht - actually, I *really* like the idea of putting the experimental in 
the name.  Also agree with you on the `ifo` vs `ifso`.  Although, if we go with 
`ifo`, I would prefer to bikeshed the option to `-emit-interface` (well with 
the experimental stuck in as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D61506: [OpenCL] Switch to C++17

2019-05-03 Thread Ronan Keryell via Phabricator via cfe-commits
keryell added inline comments.



Comment at: include/clang/Frontend/LangStandards.def:162
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 
| Digraphs | OpenCL)
 

kpet wrote:
> Suggest you add `HexFloat` as well. It is part of c++17 and all OpenCL 
> versions.
Why only C++17?
I would love to have `CPlusPlus2a` here too...


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

https://reviews.llvm.org/D61506



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


[PATCH] D61537: [clangd] Boost code completion results that were named in the last few lines.

2019-05-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

The hope is this will catch a few patterns with repetition:

  SomeClass* S = ^SomeClass::Create()
  
  int getFrobnicator() { return ^frobnicator_; }
  
  // discard the factory, it's no longer valid.
  ^MyFactory.reset();

Without triggering antipatterns too often:

  return Point(x.first, x.^second);

I'm going to gather some data on whether this turns out to be a win overall.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61537

Files:
  clangd/CodeComplete.cpp
  clangd/FindSymbols.cpp
  clangd/Quality.cpp
  clangd/Quality.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/test/completion-auto-trigger.test
  clangd/unittests/CodeCompleteTests.cpp
  clangd/unittests/QualityTests.cpp
  clangd/unittests/SourceCodeTests.cpp

Index: clangd/unittests/SourceCodeTests.cpp
===
--- clangd/unittests/SourceCodeTests.cpp
+++ clangd/unittests/SourceCodeTests.cpp
@@ -22,6 +22,7 @@
 
 using llvm::Failed;
 using llvm::HasValue;
+using testing::UnorderedElementsAreArray;
 
 MATCHER_P2(Pos, Line, Col, "") {
   return arg.line == int(Line) && arg.character == int(Col);
@@ -322,6 +323,19 @@
   EXPECT_EQ(IDs["foo"], 2u);
 }
 
+TEST(SourceCodeTests, CollectWords) {
+  auto Words = collectWords(R"cpp(
+  #define FOO_BAR
+  // this is a comment
+  std::string getSomeText() { return "magic word"; }
+  )cpp");
+  std::set ActualWords(Words.keys().begin(), Words.keys().end());
+  std::set ExpectedWords = {"define",  "foo","bar",  "this",
+ "comment", "string", "some", "text",
+ "return",  "magic",  "word"};
+  EXPECT_EQ(ActualWords, ExpectedWords);
+}
+
 TEST(SourceCodeTests, VisibleNamespaces) {
   std::vector>> Cases = {
   {
Index: clangd/unittests/QualityTests.cpp
===
--- clangd/unittests/QualityTests.cpp
+++ clangd/unittests/QualityTests.cpp
@@ -292,6 +292,16 @@
   SymbolRelevanceSignals InBaseClass;
   InBaseClass.InBaseClass = true;
   EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
+
+  llvm::StringSet<> Words = {"one", "two", "three"};
+  SymbolRelevanceSignals WithoutMatchingWord;
+  WithoutMatchingWord.ContextWords = 
+  WithoutMatchingWord.Name = "four";
+  EXPECT_EQ(WithoutMatchingWord.evaluate(), Default.evaluate());
+  SymbolRelevanceSignals WithMatchingWord;
+  WithMatchingWord.ContextWords = 
+  WithMatchingWord.Name = "TheTwoTowers";
+  EXPECT_GT(WithMatchingWord.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/unittests/CodeCompleteTests.cpp
===
--- clangd/unittests/CodeCompleteTests.cpp
+++ clangd/unittests/CodeCompleteTests.cpp
@@ -22,9 +22,11 @@
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock-generated-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -174,6 +176,7 @@
   int BBB();
   int CCC();
 };
+
 int main() { ClassWithMembers().^ }
   )cpp",
  /*IndexSymbols=*/{}, Opts);
@@ -324,7 +327,7 @@
   }
 }
 
-TEST(CompletionTest, Priorities) {
+TEST(CompletionTest, Accessible) {
   auto Internal = completions(R"cpp(
   class Foo {
 public: void pub();
@@ -334,7 +337,7 @@
   void Foo::pub() { this->^ }
   )cpp");
   EXPECT_THAT(Internal.Completions,
-  HasSubsequence(Named("priv"), Named("prot"), Named("pub")));
+  AllOf(Has("priv"), Has("prot"), Has("pub")));
 
   auto External = completions(R"cpp(
   class Foo {
@@ -502,6 +505,21 @@
   HasSubsequence(Named("absl"), Named("absb")));
 }
 
+TEST(CompletionTest, ContextWords) {
+  auto Results = completions(R"cpp(
+  enum class Color { RED, YELLOW, BLUE };
+
+  // (blank lines so the definition above isn't "context")
+
+  // "It was a yellow car," he said. "Big yellow car, new."
+  auto Finish = Color::^
+  )cpp");
+  // Yellow would normally sort last (alphabetic).
+  // But the recent mention shuold bump it up.
+  ASSERT_THAT(Results.Completions,
+  HasSubsequence(Named("YELLOW"), Named("BLUE")));
+}
+
 TEST(CompletionTest, GlobalQualified) {
   auto Results = completions(
   R"cpp(
Index: clangd/test/completion-auto-trigger.test
===
--- clangd/test/completion-auto-trigger.test
+++ clangd/test/completion-auto-trigger.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:"insertTextFormat": 1,
 # CHECK-NEXT:"kind": 5,
 # 

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

2019-05-03 Thread Mandeep Singh Grang via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359932: [COFF, ARM64] Fix ABI implementation of struct 
returns (authored by mgrang, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D60349

Files:
  include/clang/AST/DeclCXX.h
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeGen/arm64-microsoft-arguments.cpp
  test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -69,6 +69,11 @@
   int bb;
 };
 
+struct SmallWithPrivate {
+private:
+ int i;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:   (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:   i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* inalloca)
@@ -165,7 +170,7 @@
 // WIN64:   call void @"??1SmallWithDtor@@QEAA@XZ"
 // WIN64: }
 // WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i64 %s.coerce) {{.*}} {
-// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"
+// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA64: }
 
 // FIXME: MSVC incompatible!
@@ -173,6 +178,12 @@
 // WOA:   call arm_aapcs_vfpcc void @"??1SmallWithDtor@@QAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA: }
 
+
+// Test that the eligible non-aggregate is passed directly, but returned
+// indirectly on ARM64 Windows.
+// WOA64: define dso_local void @"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate* inreg noalias sret %agg.result, i64 %s.coerce) {{.*}} {
+SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }
Index: test/CodeGen/arm64-microsoft-arguments.cpp
===
--- test/CodeGen/arm64-microsoft-arguments.cpp
+++ test/CodeGen/arm64-microsoft-arguments.cpp
@@ -1,25 +1,203 @@
 // RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
 // RUN: -x c++ -o - %s | FileCheck %s
 
-struct pod { int a, b, c, d, e; };
+// Pass and return for type size <= 8 bytes.
+// CHECK: define {{.*}} i64 @{{.*}}f1{{.*}}()
+// CHECK: call i64 {{.*}}func1{{.*}}(i64 %3)
+struct S1 {
+  int a[2];
+};
+
+S1 func1(S1 x);
+S1 f1() {
+  S1 x;
+  return func1(x);
+}
+
+// Pass and return type size <= 16 bytes.
+// CHECK: define {{.*}} [2 x i64] @{{.*}}f2{{.*}}()
+// CHECK: call [2 x i64] {{.*}}func2{{.*}}([2 x i64] %3)
+struct S2 {
+  int a[4];
+};
+
+S2 func2(S2 x);
+S2 f2() {
+  S2 x;
+  return func2(x);
+}
+
+// Pass and return for type size > 16 bytes.
+// CHECK: define {{.*}} void @{{.*}}f3{{.*}}(%struct.S3* noalias sret %agg.result)
+// CHECK: call void {{.*}}func3{{.*}}(%struct.S3* sret %agg.result, %struct.S3* %agg.tmp)
+struct S3 {
+  int a[5];
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x;
+  return func3(x);
+}
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Passed directly but returned indirectly.
+// CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret %agg.result, [2 x i64] %4)
+struct S4 {
+  int a[3];
+  ~S4();
+};
+
+S4 func4(S4 x);
+S4 f4() {
+  S4 x;
+  return func4(x);
+}
+
+// Pass and return from instance method called from instance method.
+// CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* %this, %class.P1* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* %ref.tmp, %class.P1* inreg sret %agg.result, i8 %0)
+
+class P1 {
+public:
+  P1 foo(P1 x);
+};
+
+class Q1 {
+public:
+  P1 bar();
+};
+
+P1 Q1::bar() {
+  P1 p1;
+  return P1().foo(p1);
+}
+
+// Pass and return from instance method called from free function.
+// CHECK: define {{.*}} void {{.*}}bar{{.*}}()
+// CHECK: call void {{.*}}foo@P2{{.*}}(%class.P2* %ref.tmp, %class.P2* inreg sret %retval, i8 %0)
+class P2 {
+public:
+  P2 foo(P2 x);
+};
+
+P2 bar() {
+  P2 p2;
+  return P2().foo(p2);
+}
+
+// Pass and return an object with a user-provided constructor (passed directly,
+// returned indirectly)
+// CHECK: define {{.*}} void @{{.*}}f5{{.*}}(%struct.S5* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func5{{.*}}(%struct.S5* inreg sret %agg.result, i64 {{.*}})
+struct S5 {
+  S5();
+  int x;
+};
+
+S5 func5(S5 x);
+S5 f5() {
+  S5 x;
+  return func5(x);
+}
+
+// Pass and return an object with a non-trivial explicitly defaulted constructor
+// (passed directly, returned directly)
+// CHECK: define {{.*}} i64 

r359932 - [COFF, ARM64] Fix ABI implementation of struct returns

2019-05-03 Thread Mandeep Singh Grang via cfe-commits
Author: mgrang
Date: Fri May  3 14:12:24 2019
New Revision: 359932

URL: http://llvm.org/viewvc/llvm-project?rev=359932=rev
Log:
[COFF, ARM64] Fix ABI implementation of struct returns

Summary:
Related llvm patch: D60348.
Patch co-authored by Sanjin Sijaric.

Reviewers: rnk, efriedma, TomTan, ssijaric, ostannard

Reviewed By: efriedma

Subscribers: dmajor, richard.townsend.arm, ostannard, javed.absar, 
kristof.beyls, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeGen/arm64-microsoft-arguments.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=359932=359931=359932=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Fri May  3 14:12:24 2019
@@ -1325,6 +1325,14 @@ public:
   /// \note This does NOT include a check for union-ness.
   bool isEmpty() const { return data().Empty; }
 
+  bool hasPrivateFields() const {
+return data().HasPrivateFields;
+  }
+
+  bool hasProtectedFields() const {
+return data().HasProtectedFields;
+  }
+
   /// Determine whether this class has direct non-static data members.
   bool hasDirectFields() const {
 auto  = data();

Modified: cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h?rev=359932=359931=359932=diff
==
--- cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h (original)
+++ cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h Fri May  3 14:12:24 2019
@@ -95,7 +95,6 @@ private:
   bool InReg : 1;   // isDirect() || isExtend() || isIndirect()
   bool CanBeFlattened: 1;   // isDirect()
   bool SignExt : 1; // isExtend()
-  bool SuppressSRet : 1;// isIndirect()
 
   bool canHavePaddingType() const {
 return isDirect() || isExtend() || isIndirect() || isExpand();
@@ -111,14 +110,13 @@ private:
   }
 
   ABIArgInfo(Kind K)
-  : TheKind(K), PaddingInReg(false), InReg(false), SuppressSRet(false) {
+  : TheKind(K), PaddingInReg(false), InReg(false) {
   }
 
 public:
   ABIArgInfo()
   : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-TheKind(Direct), PaddingInReg(false), InReg(false),
-SuppressSRet(false) {}
+TheKind(Direct), PaddingInReg(false), InReg(false) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
   llvm::Type *Padding = nullptr,
@@ -407,16 +405,6 @@ public:
 CanBeFlattened = Flatten;
   }
 
-  bool getSuppressSRet() const {
-assert(isIndirect() && "Invalid kind!");
-return SuppressSRet;
-  }
-
-  void setSuppressSRet(bool Suppress) {
-assert(isIndirect() && "Invalid kind!");
-SuppressSRet = Suppress;
-  }
-
   void dump() const;
 };
 

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=359932=359931=359932=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri May  3 14:12:24 2019
@@ -1999,8 +1999,7 @@ void CodeGenModule::ConstructAttributeLi
   // Attach attributes to sret.
   if (IRFunctionArgs.hasSRetArg()) {
 llvm::AttrBuilder SRETAttrs;
-if (!RetAI.getSuppressSRet())
-  SRETAttrs.addAttribute(llvm::Attribute::StructRet);
+SRETAttrs.addAttribute(llvm::Attribute::StructRet);
 hasUsedSRet = true;
 if (RetAI.getInReg())
   SRETAttrs.addAttribute(llvm::Attribute::InReg);

Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=359932=359931=359932=diff
==
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Fri May  3 14:12:24 2019
@@ -1051,33 +1051,55 @@ bool MicrosoftCXXABI::hasMostDerivedRetu
   return isDeletingDtor(GD);
 }
 
+static bool IsSizeGreaterThan128(const CXXRecordDecl *RD) {
+  return RD->getASTContext().getTypeSize(RD->getTypeForDecl()) > 128;
+}
+
+static bool hasMicrosoftABIRestrictions(const CXXRecordDecl *RD) {
+  // For AArch64, we use the C++14 definition of an aggregate, so we also
+  // check for:
+  //   No private or protected non static data members.
+  //   No base classes
+  //   No virtual functions
+  // Additionally, we need to ensure that there is 

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In D61165#1490168 , @rjmccall wrote:

> I think the intuitive rule is that initialization is complete when the 
> full-expression performing the initialization is complete because that's the 
> normal unit of sequencing.  Note that the standard does use both 
> "constructed" and "initialized" in different contexts, although this might 
> just be an editorial choice.


I think it would be quite subtle if the standard was deliberately splitting 
this particular hair by implicitly creating a "constructed, but not 
initialized" state of an object, without calling that out anywhere :)

As far as I see it, we have three options here:

1. Claim that the object is formally initialized when the constructor returns

This appears to be what GCC implements.

2. Claim that the object is formally initialized when the full-expression ends, 
and if a temporary throws don't call the destructor because the object isn't 
initialized.

This is what Clang implements today, but seems wrong.

3. Claim that the object is formally initialized when the full-expression ends, 
but if a temporary throws call the destructor because the constructor ran.

This seems weird to me. If destroying temporaries is an indivisible part of the 
initialization of an object, then we shouldn't be able to call the destructor, 
because the initialization of the object didn't succeed. (I mean, assuming 
there isn't a distinction between constructed and initialized)

> I see it as inconsistent that a destructor for a temporary can abort the 
> initialization of an automatic object but not a static one. I have seen 
> programs that build idioms reliant on this kind of destructor ordering, e.g. 
> as a way to commit / abort a "transaction".

If we assert rule 1, then there isn't any inconsistency. The destructor of the 
temporary would never abort the initialization of an object when the object was 
initialized by a constructor, regardless of whether the object was static or 
automatic. The object would be considered initialized and destructed whenever 
appropriate for it's storage duration. I guess it would theoretically break 
that "transaction" pattern, buts its not like we or any other compiler actually 
supported that on static locals to begin with. We don't even do the "right 
thing" for automatics there, since we don't `invoke` the temporary destructor 
(see the IR I posted above).

> there really can't be *that* many uses of this feature yet, especially in 
> exceptions-enabled code. We can fix semantic mistakes.

Yeah, if we assert rule 3 then we I guess we should just do this, rather than 
try to determine whether we need the dtor for the static local in Sema.

Anyways, I think I've layed out my thinking here as clearly as I can. If you 
still think that 3 is right here, then I'm happy to defer to you (although it 
would be quite nice if @rsmith chimed in too). I'm happy to implement whatever 
the right thing for CodeGen to do here is too.


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

https://reviews.llvm.org/D61165



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


[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Sure, this looks better.




Comment at: clang/trunk/test/Preprocessor/indent_macro.c:1-2
-// RUN: %clang_cc1 -E %s | grep '^   zzap$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK:   zzap
 

Separate with newline



Comment at: clang/trunk/test/Preprocessor/macro_fn_varargs_named.c:1-6
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 1
+// 1:a: x
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 2
+// 2:b: x y, z,h
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 3
+// 3:c: foo(x)

```
// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix CHECK-1
// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix CHECK-2
// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix CHECK-3

// CHECK-1:a: x
// CHECK-2:b: x y, z,h
// CHECK-3:c: foo(x)
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D61496



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


[PATCH] D60974: Clang IFSO driver action.

2019-05-03 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment.

I didn't follow the technical details, but I don't see anything wrong with 
moving forward on this patch. I think this seems like an interesting idea worth 
experimenting with.

In D60974#1488563 , @jakehehrlich 
wrote:

> > Jake, I am still not sure what you would prefer for moving forward. The 
> > current patch can output the yaml format I originally proposed or the one 
> > that looks similar to the one you proposed. What I need to know is:
> > 
> > Do you want the merging if the "ifo" files to happen inside of llvm-elfabi?
> >  Do you care if we upstream the yaml we proposed as a first step (which is 
> > really a distilled version of that yaml2obj can consume anyways. this right 
> > now functions actually) ???
> >  Or, would you rather the ifo files be merged by a different separate tool 
> > (something maybe called llvm-ifsogen)???
>
> This is my proposal:
>
> 1. We should plan on adding the code ofr merging these files inside of 
> `llvm/tools/llvm-elfabi` but will it be a separate tool from `llvm-elfabi`. 
> You can create "separate" tools in the same directory using the symlink 
> trick. See llvm-ar and friends or llvm-objcopy and llvm-strip. The tool name 
> can be discussed in review.


The symlink trick is usually used when the new frontend tool is just a thin 
layer over the underlying tool. Is that going to be the case here?

For example,

- `llvm-ranlib` is equivalent to `llvm-ar s`
- `llvm-readelf` is equivalent to `llvm-readobj --elf-output-style=GNU`
- `llvm-addr2line` is equivalent to `llvm-symbolizer --output-style=GNU`
- `llvm-strip` is equivalent to `llvm-objcopy --strip-all` (maybe not exactly)

In other words, is `llvm-mergeifo` (placeholder name) going to be roughly 
equivalent to `llvm-elfabi --merge-ifo` (again, placeholder flag name)? And if 
so, it doesn't seem like a symlink is strictly necessary -- users can just call 
`llvm-elfabi --merge-ifo`. Am I missing something?

> 2. The yaml proposed thus far is not acceptable IMO for reasons already 
> outlined but this can be discussed in code review. Before adding sections a 
> clear reason for needing them is required which hasn't been given yet. Things 
> can evolve and change later as needed but we should start with the minimum 
> and add as needed later; not start with extra and remove later. Support for 
> the new format should be added into TextAPI and in the review for that 
> process we should discuss the format. After we add support for this new 
> format into TextAPI we can add support for emitting this format into clang 
> (well actually you can write the code whenever but I'm using "after" in a 
> different sense here).

What if we named it experimental for now? i.e. `--experimental-emit-ifo` for 
the clang flag name and `!experimental-ifo-elf-v1` for the yaml id? That would 
allow those working on this patch to play around with more features, but still 
give sufficient warning to anyone that fields they depend on may be removed.

In fact, if we label it experimental (or maybe even if we don't), I don't see 
any reason this couldn't land now, even without a consumer of it. So what if a 
tool produces a yaml file that we don't haven't finished the yaml parser for? 
Does anything break?




Comment at: clang/lib/Driver/Driver.cpp:3449-3450
   return C.MakeAction(Input, types::TY_Nothing);
+if (Args.hasArg(options::OPT_emit_ifso))
+  return C.MakeAction(Input, types::TY_IFO);
 return C.MakeAction(Input, types::TY_LLVM_BC);

bikeshed: I don't think we should have both ifo and ifso. Either name sounds 
fine to me, but if running "-emit-ifso" gives me an ifo and not an ifso, that's 
very surprising, and is going to be a major headache trying to remember when to 
call it ifo and when to call it ifso.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


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

2019-05-03 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

Richard Smith on cfe-dev pointed out some cases where this patch is incorrect, 
stemming from trying to calculate the linkage too early; the warning will 
either have to work without the use of `isExternallyVisible` or will have to be 
emitted later. This was reverted in `r359858` for now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61097



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


[PATCH] D61530: Add AIX Version Macros

2019-05-03 Thread Sean Fertile via Phabricator via cfe-commits
sfertile accepted this revision.
sfertile added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D61530



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


[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

Anastasia wrote:
> rjmccall wrote:
> > Sorry I didn't catch this before, but I don't see why this test is expected 
> > to work.  We can't actually pass a pointer in address space 10 to that 
> > constructor.
> Ok, I have created a bug to follow up on this issues:
> https://bugs.llvm.org/show_bug.cgi?id=41730
> 
> It seems that the check is missing here for allowing the address space 
> conversions implicitly, but I am afraid if I add it now addr spaces will 
> become less usable because implicit conversions can't be setup by the target 
> yet. And everyone is using no address space as some sort of `__generic` but 
> unofficially. :(
> 
> I have some thoughts about adding something like `__generic` address space to 
> C++ that can either be resolved by the compiler or supported by HW. I think 
> it might help to implement those cases correctly without modifying too much 
> of code base. I just struggled to find enough bandwidth to send an RFC but I 
> will try to progress on this asap.
> Ok, I have created a bug to follow up on this issues:

Thanks.

> It seems that the check is missing here for allowing the address space 
> conversions implicitly, but I am afraid if I add it now addr spaces will 
> become less usable because implicit conversions can't be setup by the target 
> yet. And everyone is using no address space as some sort of __generic but 
> unofficially. :(

As far as I'm concerned, address-space support in the C++ feature set is all 
still extremely experimental and there are no real users that we have to worry 
about making things less useful for.  The right thing for the basic model is 
for constructors to only work when the object is being constructed in an 
address space that's convertible to the address space of the constructor.  
Languages with a `__generic` superspace (whether implemented with dynamic 
selection or cloning or anything else) can consider making it the default 
address space of constructors, but that's not something we should be pushing in 
the basic model.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



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


[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment.

> Can you add tests for the bugs you fixed? thanks

The bugs were detected by existing tests (those tests triggered the newly added 
assertions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61522



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


[PATCH] D61530: Add AIX Version Macros

2019-05-03 Thread Andus Yu via Phabricator via cfe-commits
andusy created this revision.
andusy added reviewers: hubert.reinterpretcast, jasonliu, sfertile, xingxue.
andusy added a project: clang.
Herald added a subscriber: jsji.

- This patch checks the AIX version and defines the appropriate macros.
- Follow up to a comment 
 on D59048 
.


Repository:
  rC Clang

https://reviews.llvm.org/D61530

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init.c

Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -7243,6 +7243,129 @@
 // PPC-AIX:#define __powerpc__ 1
 // PPC-AIX:#define __ppc__ 1
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.2.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX72 %s
+//
+// PPC-AIX72:#define _AIX32 1
+// PPC-AIX72:#define _AIX41 1
+// PPC-AIX72:#define _AIX43 1
+// PPC-AIX72:#define _AIX50 1
+// PPC-AIX72:#define _AIX51 1
+// PPC-AIX72:#define _AIX52 1
+// PPC-AIX72:#define _AIX53 1
+// PPC-AIX72:#define _AIX61 1
+// PPC-AIX72:#define _AIX71 1
+// PPC-AIX72:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX71 %s
+//
+// PPC-AIX71:#define _AIX32 1
+// PPC-AIX71:#define _AIX41 1
+// PPC-AIX71:#define _AIX43 1
+// PPC-AIX71:#define _AIX50 1
+// PPC-AIX71:#define _AIX51 1
+// PPC-AIX71:#define _AIX52 1
+// PPC-AIX71:#define _AIX53 1
+// PPC-AIX71:#define _AIX61 1
+// PPC-AIX71:#define _AIX71 1
+// PPC-AIX71-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix6.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX61 %s
+//
+// PPC-AIX61:#define _AIX32 1
+// PPC-AIX61:#define _AIX41 1
+// PPC-AIX61:#define _AIX43 1
+// PPC-AIX61:#define _AIX50 1
+// PPC-AIX61:#define _AIX51 1
+// PPC-AIX61:#define _AIX52 1
+// PPC-AIX61:#define _AIX53 1
+// PPC-AIX61:#define _AIX61 1
+// PPC-AIX61-NOT:#define _AIX71 1
+// PPC-AIX61-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.3.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX53 %s
+// PPC-AIX53:#define _AIX32 1
+// PPC-AIX53:#define _AIX41 1
+// PPC-AIX53:#define _AIX43 1
+// PPC-AIX53:#define _AIX50 1
+// PPC-AIX53:#define _AIX51 1
+// PPC-AIX53:#define _AIX52 1
+// PPC-AIX53:#define _AIX53 1
+// PPC-AIX53-NOT:#define _AIX61 1
+// PPC-AIX53-NOT:#define _AIX71 1
+// PPC-AIX53-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.2.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX52 %s
+// PPC-AIX52:#define _AIX32 1
+// PPC-AIX52:#define _AIX41 1
+// PPC-AIX52:#define _AIX43 1
+// PPC-AIX52:#define _AIX50 1
+// PPC-AIX52:#define _AIX51 1
+// PPC-AIX52:#define _AIX52 1
+// PPC-AIX52-NOT:#define _AIX53 1
+// PPC-AIX52-NOT:#define _AIX61 1
+// PPC-AIX52-NOT:#define _AIX71 1
+// PPC-AIX52-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX51 %s
+// PPC-AIX51:#define _AIX32 1
+// PPC-AIX51:#define _AIX41 1
+// PPC-AIX51:#define _AIX43 1
+// PPC-AIX51:#define _AIX50 1
+// PPC-AIX51:#define _AIX51 1
+// PPC-AIX51-NOT:#define _AIX52 1
+// PPC-AIX51-NOT:#define _AIX53 1
+// PPC-AIX51-NOT:#define _AIX61 1
+// PPC-AIX51-NOT:#define _AIX71 1
+// PPC-AIX51-NOT:#define _AIX72 1
+//
+//RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.0.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX50 %s
+// PPC-AIX50:#define _AIX32 1
+// PPC-AIX50:#define _AIX41 1
+// PPC-AIX50:#define _AIX43 1
+// PPC-AIX50:#define _AIX50 1
+// PPC-AIX50-NOT:#define _AIX51 1
+// PPC-AIX50-NOT:#define _AIX52 1
+// PPC-AIX50-NOT:#define _AIX53 1
+// PPC-AIX50-NOT:#define _AIX61 1
+// PPC-AIX50-NOT:#define _AIX71 1
+// PPC-AIX50-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix4.3.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX43 %s
+// PPC-AIX43:#define _AIX32 1
+// PPC-AIX43:#define _AIX41 1
+// PPC-AIX43:#define _AIX43 1
+// PPC-AIX43-NOT:#define _AIX50 1
+// PPC-AIX43-NOT:#define _AIX51 1
+// PPC-AIX43-NOT:#define _AIX52 1
+// PPC-AIX43-NOT:#define _AIX53 1
+// PPC-AIX43-NOT:#define _AIX61 1
+// PPC-AIX43-NOT:#define _AIX71 1
+// PPC-AIX43-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix4.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX41 %s
+// PPC-AIX41:#define _AIX32 1
+// PPC-AIX41:#define _AIX41 1
+// PPC-AIX41-NOT:#define _AIX43 1
+// PPC-AIX41-NOT:#define _AIX50 1
+// PPC-AIX41-NOT:#define _AIX51 1
+// PPC-AIX41-NOT:#define _AIX52 1
+// PPC-AIX41-NOT:#define _AIX53 1
+// PPC-AIX41-NOT:#define _AIX61 1
+// PPC-AIX41-NOT:#define _AIX71 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D59474#1490235 , @lildmh wrote:

> Hi Alexey,
>
> Let's discuss your runtime data mapping scheme next week first. After that we 
> will continue the review of this.


That would be good, thanks!




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8530
+  // dynamically.
+  QualType MapArrayType = Ctx.getConstantArrayType(
+  Ctx.getIntTypeForBitwidth(/*DestWidth*/ 64, /*Signed*/ true),

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > Hmm, how could you calculate the required size of the array for the 
> > > > mapper? Or this is constant?
> > > I'm not sure I understand your question here.
> > > Do you mean the size when an OpenMP array section is mapped? If that's 
> > > the case, it is not constant. Existing code can already handle it.
> > > Or do you mean the size of mapper array (i.e., `MapArrayType`)? This is 
> > > constant and depends on how many map clauses exist in the declare mapper 
> > > directive.
> > Yes, it is the question about the size of mapper array. It is the part of 
> > our discussion about mappers generation we had before. You said that it is 
> > hard to generate the sizes of the arrays since we don't know the number of 
> > map clauses at the codegen phase. bu there we have this number.
> Sorry there was probably some miscommunication. What I meant is that after 
> fully expanded, for example, from `map(mapper(id):a[0:n])`, eventually to 
> `map(a.b.c[0:e]) map(a.k) ...`, the number of things in the results is 
> unknown at compile time.
> 
> Here, we only do one level of expansion of one instance based on the `declare 
> mapper` directive, for example, the mapper is `declare mapper(class A a) 
> map(a.b[0:a.n]) map(a.c)`
> In this case, the size of mapper array is 2, because there are 2 map clauses 
> (actually it's more than 2 because the first map clause maps an array). This 
> number can be decided at compile time easily.
Let's discuss the runtime at first, later we can return to this.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:351
+  /// is the asynchronous version.
+  llvm::DenseMap>

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > You should be very careful with this map. If the mapper is declared 
> > > > > > in the function context, it must be removed from this map as soon 
> > > > > > as the function processing is completed. All local declarations are 
> > > > > > removed after this and their address might be used again.
> > > > > Yes, I plan to have this part in the next patch, which will implement 
> > > > > to look up the corresponding mapper function for map. to, from clauses
> > > > Noope, this must be the part of this patch, because it may cause the 
> > > > crash of the compiler during testing.
> > > It will not crash the compiler, because this UDMap is only written in 
> > > this patch, never read.
> > Still, you should clear it in this patch. Otherwise, you're breaking 
> > data-dependency between the patches and this is not good at all.
> Sure, if you think that's absolutely necessary, I can add it to this patch.
It is necessary, so, please, add it, thanks!


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

https://reviews.llvm.org/D59474



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


[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-05-03 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked 3 inline comments as done.
lildmh added a comment.

Hi Alexey,

Let's discuss your runtime data mapping scheme next week first. After that we 
will continue the review of this.




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8530
+  // dynamically.
+  QualType MapArrayType = Ctx.getConstantArrayType(
+  Ctx.getIntTypeForBitwidth(/*DestWidth*/ 64, /*Signed*/ true),

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > Hmm, how could you calculate the required size of the array for the 
> > > mapper? Or this is constant?
> > I'm not sure I understand your question here.
> > Do you mean the size when an OpenMP array section is mapped? If that's the 
> > case, it is not constant. Existing code can already handle it.
> > Or do you mean the size of mapper array (i.e., `MapArrayType`)? This is 
> > constant and depends on how many map clauses exist in the declare mapper 
> > directive.
> Yes, it is the question about the size of mapper array. It is the part of our 
> discussion about mappers generation we had before. You said that it is hard 
> to generate the sizes of the arrays since we don't know the number of map 
> clauses at the codegen phase. bu there we have this number.
Sorry there was probably some miscommunication. What I meant is that after 
fully expanded, for example, from `map(mapper(id):a[0:n])`, eventually to 
`map(a.b.c[0:e]) map(a.k) ...`, the number of things in the results is unknown 
at compile time.

Here, we only do one level of expansion of one instance based on the `declare 
mapper` directive, for example, the mapper is `declare mapper(class A a) 
map(a.b[0:a.n]) map(a.c)`
In this case, the size of mapper array is 2, because there are 2 map clauses 
(actually it's more than 2 because the first map clause maps an array). This 
number can be decided at compile time easily.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8795
+  // Generate an asynchronous mapper function.
+  llvm::Function *AsyncFn = emitUDMapperFunc(D, /*NoWait=*/true);
+  // Add the generated mapper functions to UDMMap.

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > With the buffering-based implementation we need only function.
> > Yes, in either case, we only generate functions here. Is there a problem?
> Sorry, I meant we'll need only one function.
Yes, in that case, only one is needed.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:351
+  /// is the asynchronous version.
+  llvm::DenseMap>

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > You should be very careful with this map. If the mapper is declared 
> > > > > in the function context, it must be removed from this map as soon as 
> > > > > the function processing is completed. All local declarations are 
> > > > > removed after this and their address might be used again.
> > > > Yes, I plan to have this part in the next patch, which will implement 
> > > > to look up the corresponding mapper function for map. to, from clauses
> > > Noope, this must be the part of this patch, because it may cause the 
> > > crash of the compiler during testing.
> > It will not crash the compiler, because this UDMap is only written in this 
> > patch, never read.
> Still, you should clear it in this patch. Otherwise, you're breaking 
> data-dependency between the patches and this is not good at all.
Sure, if you think that's absolutely necessary, I can add it to this patch.


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

https://reviews.llvm.org/D59474



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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

> It is up to you. I don't have strong objections if you think this will work 
> as required. Just the tests must be fixed, especially codegen tests.

Thanks, Alexey. I think this will work as required, and then we'll be able to 
update it when we get declare variant. Agreed on the tests (on all points).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Thanks for the context. Sounds great :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61269



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


r359918 - Ensure there is stack usage in stack size warning test

2019-05-03 Thread Matt Arsenault via cfe-commits
Author: arsenm
Date: Fri May  3 12:04:14 2019
New Revision: 359918

URL: http://llvm.org/viewvc/llvm-project?rev=359918=rev
Log:
Ensure there is stack usage in stack size warning test

r359906 broke this because the only stack usage was from a spill which
can be avoided since the only block is a return.

Modified:
cfe/trunk/test/Misc/backend-stack-frame-diagnostics-fallback.cpp

Modified: cfe/trunk/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/backend-stack-frame-diagnostics-fallback.cpp?rev=359918=359917=359918=diff
==
--- cfe/trunk/test/Misc/backend-stack-frame-diagnostics-fallback.cpp (original)
+++ cfe/trunk/test/Misc/backend-stack-frame-diagnostics-fallback.cpp Fri May  3 
12:04:14 2019
@@ -14,5 +14,7 @@ namespace frameSizeThunkWarning {
 
   // CHECK: warning: stack frame size of {{[0-9]+}} bytes in function 
'frameSizeThunkWarning::B::f'
   // CHECK: warning: stack size limit exceeded ({{[0-9]+}}) in {{[^ ]+}}
-  void B::f() { }
+  void B::f() {
+volatile int x = 0; // Ensure there is stack usage.
+  }
 }


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


[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

2019-05-03 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359917: [CommandLine] Enable Grouping for short options by 
default.  Part 4 of 5 (authored by dhinton, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D61270?vs=197272=198059#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61270

Files:
  llvm/trunk/include/llvm/Support/CommandLine.h
  llvm/trunk/lib/Support/CommandLine.cpp
  llvm/trunk/test/tools/llvm-readobj/merged.test


Index: llvm/trunk/include/llvm/Support/CommandLine.h
===
--- llvm/trunk/include/llvm/Support/CommandLine.h
+++ llvm/trunk/include/llvm/Support/CommandLine.h
@@ -1200,7 +1200,11 @@
 };
 
 template <> struct applicator {
-  static void opt(MiscFlags MF, Option ) { O.setMiscFlag(MF); }
+  static void opt(MiscFlags MF, Option ) {
+assert((MF != Grouping || O.ArgStr.size() == 1) &&
+   "cl::Grouping can only apply to single charater Options.");
+O.setMiscFlag(MF);
+  }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.
Index: llvm/trunk/test/tools/llvm-readobj/merged.test
===
--- llvm/trunk/test/tools/llvm-readobj/merged.test
+++ llvm/trunk/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | 
FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/trunk/lib/Support/CommandLine.cpp
===
--- llvm/trunk/lib/Support/CommandLine.cpp
+++ llvm/trunk/lib/Support/CommandLine.cpp
@@ -421,6 +421,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {


Index: llvm/trunk/include/llvm/Support/CommandLine.h
===
--- llvm/trunk/include/llvm/Support/CommandLine.h
+++ llvm/trunk/include/llvm/Support/CommandLine.h
@@ -1200,7 +1200,11 @@
 };
 
 template <> struct applicator {
-  static void opt(MiscFlags MF, Option ) { O.setMiscFlag(MF); }
+  static void opt(MiscFlags MF, Option ) {
+assert((MF != Grouping || O.ArgStr.size() == 1) &&
+   "cl::Grouping can only apply to single charater Options.");
+O.setMiscFlag(MF);
+  }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.
Index: llvm/trunk/test/tools/llvm-readobj/merged.test
===
--- llvm/trunk/test/tools/llvm-readobj/merged.test
+++ llvm/trunk/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/trunk/lib/Support/CommandLine.cpp
===
--- llvm/trunk/lib/Support/CommandLine.cpp
+++ llvm/trunk/lib/Support/CommandLine.cpp
@@ -421,6 +421,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359916 - Attempt to fix the clang-sphinx-docs bot after r358797

2019-05-03 Thread Nico Weber via cfe-commits
Author: nico
Date: Fri May  3 11:54:18 2019
New Revision: 359916

URL: http://llvm.org/viewvc/llvm-project?rev=359916=rev
Log:
Attempt to fix the clang-sphinx-docs bot after r358797

Modified:
cfe/trunk/docs/analyzer/checkers.rst

Modified: cfe/trunk/docs/analyzer/checkers.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/checkers.rst?rev=359916=359915=359916=diff
==
--- cfe/trunk/docs/analyzer/checkers.rst (original)
+++ cfe/trunk/docs/analyzer/checkers.rst Fri May  3 11:54:18 2019
@@ -340,7 +340,7 @@ optin
 Checkers for portability, performance or coding style specific rules.
 
 optin.cplusplus.UninitializedObject (C++)
-"""
+"
 
 This checker reports uninitialized fields in objects created after a 
constructor
 call. It doesn't only find direct uninitialized fields, but rather makes a deep


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


[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8021
+  /// the extracted map clauses.
+  void generateAllInfoForMapper(MapBaseValuesArrayTy ,
+MapValuesArrayTy ,

lildmh wrote:
> ABataev wrote:
> > This code has too many common parts with the existing one. Is it possible 
> > to merge it somehow or outline into a function?
> I tried to merge it with `generateAllInfo`. The problem is `generateAllInfo` 
> also generates information for clauses including `to, from, is_device_ptr, 
> use_device_ptr`, which don't exist for `declare mapper`. There is no clear 
> way to extract them separately. For example, every 4 or 5 lines, the code is 
> intended to address a different clause type.
> At last, I think the most clear way is to extract all code related to map 
> clauses into this function `generateAllInfoForMapper`. It's ~70 lines of code 
> so not too much.
If those clauses do not exist for the declare mapper, it is fine, no problems 
with them. If they don't exist, we can't generate anything for them, no?
But if you think, that it would be better to extract some common parts into a 
separate function, this also works for me.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8530
+  // dynamically.
+  QualType MapArrayType = Ctx.getConstantArrayType(
+  Ctx.getIntTypeForBitwidth(/*DestWidth*/ 64, /*Signed*/ true),

lildmh wrote:
> ABataev wrote:
> > Hmm, how could you calculate the required size of the array for the mapper? 
> > Or this is constant?
> I'm not sure I understand your question here.
> Do you mean the size when an OpenMP array section is mapped? If that's the 
> case, it is not constant. Existing code can already handle it.
> Or do you mean the size of mapper array (i.e., `MapArrayType`)? This is 
> constant and depends on how many map clauses exist in the declare mapper 
> directive.
Yes, it is the question about the size of mapper array. It is the part of our 
discussion about mappers generation we had before. You said that it is hard to 
generate the sizes of the arrays since we don't know the number of map clauses 
at the codegen phase. bu there we have this number.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8588
+
+  if (IsMapper) {
+// Combine the map type inherited from user-defined mapper with that

lildmh wrote:
> ABataev wrote:
> > I think it would be good to move this part to the runtime. We should just 
> > pass the mapper function to the runtime functions and the runtime should 
> > call those mapper functions and get base pointers, pointers, sizes and 
> > maptypes.
> This part cannot be moved into the runtime, because the runtime does not know 
> the map type associated with the mapper. Another argument can be potentially 
> added to the runtime API, but that will be more work and I don't think it's 
> necessary
I think it is better again discuss the runtime part of the patch, because 
everything else depends on the runtime. I would suggest to try to implement the 
solution we discussed before, where the required data is stored in the runtime 
dynamic arrays and only after that it is used to transfer the data.




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8795
+  // Generate an asynchronous mapper function.
+  llvm::Function *AsyncFn = emitUDMapperFunc(D, /*NoWait=*/true);
+  // Add the generated mapper functions to UDMMap.

lildmh wrote:
> ABataev wrote:
> > With the buffering-based implementation we need only function.
> Yes, in either case, we only generate functions here. Is there a problem?
Sorry, I meant we'll need only one function.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:351
+  /// is the asynchronous version.
+  llvm::DenseMap>

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > You should be very careful with this map. If the mapper is declared in 
> > > > the function context, it must be removed from this map as soon as the 
> > > > function processing is completed. All local declarations are removed 
> > > > after this and their address might be used again.
> > > Yes, I plan to have this part in the next patch, which will implement to 
> > > look up the corresponding mapper function for map. to, from clauses
> > Noope, this must be the part of this patch, because it may cause the crash 
> > of the compiler during testing.
> It will not crash the compiler, because this UDMap is only written in this 
> patch, never read.
Still, you should clear it in this patch. Otherwise, you're breaking 
data-dependency between the patches and this is not good at all.


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

https://reviews.llvm.org/D59474



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment.

In D61269#1490176 , @thakis wrote:

> I happened to see this go by. Is there an explanation of the overall goal 
> somewhere? In general, requiring -- for long flags sounds like a great change 
> to me, but there are a few exceptions: For example. lld-link should keep 
> accepting long flags with a single dash for link.exe compatibility.


The 5th and final patch, D61294 , 
"[CommandLine] Add long option flag for cl::ParseCommandLineOptions . Part 5 of 
5", adds a flag that requires long options use the `--`.  It's an opt-in 
strategy, so only tools that want the GNU `getopt_long` behavior would want to 
set it.  Otherwise, everything works the way it does currently, with the 
exception of help printing `--` for long options.

As noted in D61294 , these changes were 
motivated by this discussion:  
http://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html

Of course, this doesn't change tblgen generated options which are a different 
animal.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61269



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


[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5784
 Expr::EvalResult Result;
-if (CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext()))
+if (!CollapseLoopCountExpr->isValueDependent() &&
+!CollapseLoopCountExpr->isTypeDependent() &&

I would suggest to modify the code of this function if we cannot get the value 
of the loops.
```
if (CollapseLoopCountExpr->isValueDependent() || 
CollapseLoopCountExpr->isTypeDependent() || 
OrderedLoopCountExpr->isValueDependent() || 
OrderedLoopCountExpr->isTypeDependent()) {
  Built.clear(/* size */0);
  return 1;
}
```
at the beginning of the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61522



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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D61399#1490189 , @hfinkel wrote:

> > Still, I think we need to prvide the default implementation of those 
> > non-standard functions (they can be very simple, maybe reporting error is 
> > going to be enough), which can be overriden by user.
>
> I appreciate your motivation, and I agree with you to some extent. I don't 
> object to having generic versions of useful math functions, but I don't think 
> they should be required. It's not reasonable to make someone add generic 
> versions of every function which happens to appear in a 
> system/target-specific math.h header. NVPTX won't be the only target that has 
> target-optimized functions that get pulled in, even from our own headers, but 
> system headers also have differences anyway depending on what preprocessor 
> macros are defined. In the end, people can write portable code if they stick 
> to what's in the standard, and we should make it reasonably easy for them to 
> step outside of the standard to do what they need to do when the standard 
> subset of available functionality doesn't meet their needs for whatever 
> reason. This is what we do for C/C++, where we provide intrinsics and other 
> system functions for those who can't write their code only using the 
> facilities that C/C++ provide.
>
> In any case, I think that we can figure out how to add generic versions of 
> non-standard math functions in a separate thread. I think that we should move 
> forward with this and then make progress on generic versions separately. It's 
> also possible that we want to fold this discussion into the discussion on an 
> LLVM math library (we've talked about this for some time in the context of 
> vector math libraries, and I'd not thought about accelerators in this 
> context, but maybe this is all related).


It is up to you. I don't have strong objections if you think this will work as 
required. Just the tests must be fixed, especially codegen tests.




Comment at: test/CodeGen/nvptx_device_cmath_functions.c:1
+// Test calling of device math functions.
+///==///

1. Provide tests for C++ too.
2. Do not use driver to generate the code, use frontend.
3. Do not include real system header files. Use some stubs instead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


r359913 - Remove else-after-return

2019-05-03 Thread David Blaikie via cfe-commits
Author: dblaikie
Date: Fri May  3 11:11:31 2019
New Revision: 359913

URL: http://llvm.org/viewvc/llvm-project?rev=359913=rev
Log:
Remove else-after-return

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359913=359912=359913=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri May  3 11:11:31 2019
@@ -8269,17 +8269,16 @@ bool IntExprEvaluator::VisitBuiltinCallE
 
   case Builtin::BI__builtin_constant_p: {
 const Expr *Arg = E->getArg(0);
-if (EvaluateBuiltinConstantP(Info, Arg)) {
+if (EvaluateBuiltinConstantP(Info, Arg))
   return Success(true, E);
-} else if (Info.InConstantContext || Arg->HasSideEffects(Info.Ctx)) {
+if (Info.InConstantContext || Arg->HasSideEffects(Info.Ctx)) {
   // Outside a constant context, eagerly evaluate to false in the presence
   // of side-effects in order to avoid -Wunsequenced false-positives in
   // a branch on __builtin_constant_p(expr).
   return Success(false, E);
-} else {
-  Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
-  return false;
 }
+Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
+return false;
   }
 
   case Builtin::BI__builtin_is_constant_evaluated:


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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

> Still, I think we need to prvide the default implementation of those 
> non-standard functions (they can be very simple, maybe reporting error is 
> going to be enough), which can be overriden by user.

I appreciate your motivation, and I agree with you to some extent. I don't 
object to having generic versions of useful math functions, but I don't think 
they should be required. It's not reasonable to make someone add generic 
versions of every function which happens to appear in a system/target-specific 
math.h header. NVPTX won't be the only target that has target-optimized 
functions that get pulled in, even from our own headers, but system headers 
also have differences anyway depending on what preprocessor macros are defined. 
In the end, people can write portable code if they stick to what's in the 
standard, and we should make it reasonably easy for them to step outside of the 
standard to do what they need to do when the standard subset of available 
functionality doesn't meet their needs for whatever reason. This is what we do 
for C/C++, where we provide intrinsics and other system functions for those who 
can't write their code only using the facilities that C/C++ provide.

In any case, I think that we can figure out how to add generic versions of 
non-standard math functions in a separate thread. I think that we should move 
forward with this and then make progress on generic versions separately. It's 
also possible that we want to fold this discussion into the discussion on an 
LLVM math library (we've talked about this for some time in the context of 
vector math libraries, and I'd not thought about accelerators in this context, 
but maybe this is all related).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

The OpenMP part looks good.


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

https://reviews.llvm.org/D61509



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


[PATCH] D61474: [CUDA][Clang][Bugfix] Add missing CUDA 9.2 case

2019-05-03 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359910: [CUDA][Clang][Bugfix] Add missing CUDA 9.2 case 
(authored by gbercea, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D61474

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -656,6 +656,9 @@
 case CudaVersion::CUDA_100:
   PtxFeature = "+ptx63";
   break;
+case CudaVersion::CUDA_92:
+  PtxFeature = "+ptx61";
+  break;
 case CudaVersion::CUDA_91:
   PtxFeature = "+ptx61";
   break;


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -656,6 +656,9 @@
 case CudaVersion::CUDA_100:
   PtxFeature = "+ptx63";
   break;
+case CudaVersion::CUDA_92:
+  PtxFeature = "+ptx61";
+  break;
 case CudaVersion::CUDA_91:
   PtxFeature = "+ptx61";
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

>>> That's only true for subobjects of an enclosing aggregate before that 
>>> aggregate's initialization is complete though, right? So it doesn't seem 
>>> like that much of an inconsistency, just mimicking what we would be doing 
>>> if an exception was thrown in, say, the body of the ctor before the 
>>> object's initialization is completed.
>> 
>> Conceptually yes, but formally no.  The standard *could* write this rule as 
>> "all currently-initialized subobjects must be separately destroyed when an 
>> exception aborts initialization of the containing aggregate, including 
>> constructor bodies and aggregate initialization", but it doesn't actually do 
>> so; instead it has specific rules covering the behavior when an exception is 
>> thrown out of the body of a constructor, and those rules simply do not apply 
>> to aggregate initialization.
> 
> Right, I was just trying to draw an analogy. Can you be more specific about 
> the inconsistency you mentioned above? What objects with static storage 
> duration have to be destroyed when an exception is thrown? Just subobjects of 
> static aggregates that had their initialization aborted by an exception? If 
> so, that obviously doesn't seem inconsistent.

I see it as inconsistent that a destructor for a temporary can abort the 
initialization of an automatic object but not a static one.  I have seen 
programs that build idioms reliant on this kind of destructor ordering, e.g. as 
a way to commit / abort a "transaction".

>> Even if it did, though, that wouldn't tell us how to resolve this because 
>> this is fundamentally about when exactly the initialization of an object is 
>> "complete", which doesn't seem to be clearly defined in the standard.  
>> There's a rule for when a *constructor* is complete, but among other things, 
>> not all initializations involve constructors.
> 
> Yeah, I agree that this is the fundamental problem here, and that the 
> standard isn't much help. I'm trying to understand why you think this choice 
> of semantics is better (or at least, good enough to make us hedge our bets 
> here at the cost of keeping this feature simple and useful). I'm not seeing 
> the aggregate initialization inconsistency you mentioned above. If the 
> standard wanted us to call the destructor before the object's initialization 
> was formally complete, then that seems like something they would mention.  
> Other implementations (well, GCC) seem to have made the opposite decision, 
> which I think makes more intuitive sense.

I think the intuitive rule is that initialization is complete when the 
full-expression performing the initialization is complete because that's the 
normal unit of sequencing.  Note that the standard does use both "constructed" 
and "initialized" in different contexts, although this might just be an 
editorial choice.

>>> So what should that path forward be here? I'd really like to get this crash 
>>> fixed soon. If we want to consider a static local no_destroy dtor 
>>> potentially-invoked in Sema if the initializer has a temporary with a 
>>> throwing dtor, then we could do that, but it'd be unfortunate for 98/03 
>>> where I believe a dtor isn't noexcept by default, so we'd have to assume 
>>> the worst. I guess it'd be easier to change our minds in the future if we 
>>> treat the dtor as potentially-invoked, but I'm not really seeing the 
>>> argument that we shouldn't just use this rule.
>> 
>> I think the simplest rule would be to say that the destructor must still be 
>> accessible for static or thread-local locals and that it'll be used in 
>> certain cases when initialization is aborted.
> 
> If we did this the source compat problem would be a lot less theoretical.

That's a good point, but there really can't be *that* many uses of this feature 
yet, especially in exceptions-enabled code.  We can fix semantic mistakes.


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

https://reviews.llvm.org/D61165



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


[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete added a comment.

Can you add tests for the bugs you fixed? thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61522



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


[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-05-03 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 198050.
lildmh added a comment.

Fix code format


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

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,770 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd 

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 198046.
jdenny edited the summary of this revision.
jdenny added a comment.
Herald added a subscriber: jfb.

As discussed, implement OpenMP solution #1 (one-line change in 
`PragmaOpenMPHandler::HandlePragma` in `ParsePragma.cpp`), and update tests.


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

https://reviews.llvm.org/D61509

Files:
  clang-tools-extra/test/clang-tidy/openmp-exception-escape.cpp
  clang-tools-extra/test/clang-tidy/openmp-use-default-none.cpp
  clang/include/clang/Lex/Pragma.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/test/AST/ast-dump-openmp-atomic.c
  clang/test/AST/ast-dump-openmp-barrier.c
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-critical.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-flush.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-master.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-taskwait.c
  clang/test/AST/ast-dump-openmp-taskyield.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/dump.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/threadprivate_codegen.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp



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


[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

I happened to see this go by. Is there an explanation of the overall goal 
somewhere? In general, requiring -- for long flags sounds like a great change 
to me, but there are a few exceptions: For example. lld-link should keep 
accepting long flags with a single dash for link.exe compatibility.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61269



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


[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-05-03 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked 7 inline comments as done.
lildmh added a comment.

Hi Alexey,

Again, thanks for your review! Sorry I didn't get back to you in time because I 
was distracted by other things. Please see the comments inline.




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8021
+  /// the extracted map clauses.
+  void generateAllInfoForMapper(MapBaseValuesArrayTy ,
+MapValuesArrayTy ,

ABataev wrote:
> This code has too many common parts with the existing one. Is it possible to 
> merge it somehow or outline into a function?
I tried to merge it with `generateAllInfo`. The problem is `generateAllInfo` 
also generates information for clauses including `to, from, is_device_ptr, 
use_device_ptr`, which don't exist for `declare mapper`. There is no clear way 
to extract them separately. For example, every 4 or 5 lines, the code is 
intended to address a different clause type.
At last, I think the most clear way is to extract all code related to map 
clauses into this function `generateAllInfoForMapper`. It's ~70 lines of code 
so not too much.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8530
+  // dynamically.
+  QualType MapArrayType = Ctx.getConstantArrayType(
+  Ctx.getIntTypeForBitwidth(/*DestWidth*/ 64, /*Signed*/ true),

ABataev wrote:
> Hmm, how could you calculate the required size of the array for the mapper? 
> Or this is constant?
I'm not sure I understand your question here.
Do you mean the size when an OpenMP array section is mapped? If that's the 
case, it is not constant. Existing code can already handle it.
Or do you mean the size of mapper array (i.e., `MapArrayType`)? This is 
constant and depends on how many map clauses exist in the declare mapper 
directive.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8588
+
+  if (IsMapper) {
+// Combine the map type inherited from user-defined mapper with that

ABataev wrote:
> I think it would be good to move this part to the runtime. We should just 
> pass the mapper function to the runtime functions and the runtime should call 
> those mapper functions and get base pointers, pointers, sizes and maptypes.
This part cannot be moved into the runtime, because the runtime does not know 
the map type associated with the mapper. Another argument can be potentially 
added to the runtime API, but that will be more work and I don't think it's 
necessary



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8795
+  // Generate an asynchronous mapper function.
+  llvm::Function *AsyncFn = emitUDMapperFunc(D, /*NoWait=*/true);
+  // Add the generated mapper functions to UDMMap.

ABataev wrote:
> With the buffering-based implementation we need only function.
Yes, in either case, we only generate functions here. Is there a problem?



Comment at: lib/CodeGen/CGOpenMPRuntime.h:351
+  /// is the asynchronous version.
+  llvm::DenseMap>

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > You should be very careful with this map. If the mapper is declared in 
> > > the function context, it must be removed from this map as soon as the 
> > > function processing is completed. All local declarations are removed 
> > > after this and their address might be used again.
> > Yes, I plan to have this part in the next patch, which will implement to 
> > look up the corresponding mapper function for map. to, from clauses
> Noope, this must be the part of this patch, because it may cause the crash of 
> the compiler during testing.
It will not crash the compiler, because this UDMap is only written in this 
patch, never read.


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

https://reviews.llvm.org/D59474



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


r359910 - [CUDA][Clang][Bugfix] Add missing CUDA 9.2 case

2019-05-03 Thread Gheorghe-Teodor Bercea via cfe-commits
Author: gbercea
Date: Fri May  3 10:59:18 2019
New Revision: 359910

URL: http://llvm.org/viewvc/llvm-project?rev=359910=rev
Log:
[CUDA][Clang][Bugfix] Add missing CUDA 9.2 case

Summary:
The bug was reported on the OpenMP-dev list:

.../obj-release/lib/clang/9.0.0/include/__clang_cuda_intrinsics.h:173:35: 
error: '__nvvm_shfl_sync_idx_i32' needs target feature ptx60|ptx61|ptx63|ptx64
__MAKE_SYNC_SHUFFLES(__shfl_sync, __nvvm_shfl_sync_idx_i32,

This problem occurs when trying to compile a .cu file that requires a newer ptx 
version (>ptx60 in this case) than ptx42.



Reviewers: tra, ABataev, caomhin

Reviewed By: tra

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

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

Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.cpp?rev=359910=359909=359910=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.cpp Fri May  3 10:59:18 2019
@@ -656,6 +656,9 @@ void CudaToolChain::addClangTargetOption
 case CudaVersion::CUDA_100:
   PtxFeature = "+ptx63";
   break;
+case CudaVersion::CUDA_92:
+  PtxFeature = "+ptx61";
+  break;
 case CudaVersion::CUDA_91:
   PtxFeature = "+ptx61";
   break;


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


[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359909: [CommandLine] Change help output to prefix long 
options with `--` instead of `… (authored by dhinton, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D61269?vs=197466=198044#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61269

Files:
  cfe/trunk/test/Driver/clang-offload-bundler.c
  llvm/trunk/lib/Support/CommandLine.cpp
  llvm/trunk/test/FileCheck/dump-input-enable.txt
  llvm/trunk/test/Support/check-default-options.txt
  llvm/trunk/unittests/Support/CommandLineTest.cpp

Index: llvm/trunk/test/Support/check-default-options.txt
===
--- llvm/trunk/test/Support/check-default-options.txt
+++ llvm/trunk/test/Support/check-default-options.txt
@@ -8,11 +8,11 @@
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for --help
+# CHECK-OPT-RPT: -h  - Alias for --help
 # CHECK-DWARF:   -h  - Alias for -help
 
 # llvm-dwarfdump declares `-h` option and prints special help in that case,
 # which is weird, but makes for a good test, i.e., shows the default `-h`
 # wasn't used.
-# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
+# CHECK-DWARF-H-NOT: --help-list  - Display list of available options (--help-list-hidden for more)
Index: llvm/trunk/test/FileCheck/dump-input-enable.txt
===
--- llvm/trunk/test/FileCheck/dump-input-enable.txt
+++ llvm/trunk/test/FileCheck/dump-input-enable.txt
@@ -26,7 +26,7 @@
 ; RUN: not FileCheck -dump-input=foobar 2>&1 \
 ; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
 
-BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the -dump-input option: Cannot find option named 'foobar'!
+BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the --dump-input option: Cannot find option named 'foobar'!
 
 ;--
 ; Check -dump-input=help.
Index: llvm/trunk/lib/Support/CommandLine.cpp
===
--- llvm/trunk/lib/Support/CommandLine.cpp
+++ llvm/trunk/lib/Support/CommandLine.cpp
@@ -88,8 +88,37 @@
 
 //===--===//
 
+static StringRef ArgPrefix = "  -";
+static StringRef ArgPrefixLong = "  --";
+static StringRef ArgHelpPrefix = " - ";
+
+static size_t argPlusPrefixesSize(StringRef ArgName) {
+  size_t Len = ArgName.size();
+  if (Len == 1)
+return Len + ArgPrefix.size() + ArgHelpPrefix.size();
+  return Len + ArgPrefixLong.size() + ArgHelpPrefix.size();
+}
+
+static StringRef argPrefix(StringRef ArgName) {
+  if (ArgName.size() == 1)
+return ArgPrefix;
+  return ArgPrefixLong;
+}
+
 namespace {
 
+class PrintArg {
+  StringRef ArgName;
+public:
+  PrintArg(StringRef ArgName) : ArgName(ArgName) {}
+  friend raw_ostream <<(raw_ostream , const PrintArg&);
+};
+
+raw_ostream <<(raw_ostream , const PrintArg& Arg) {
+  OS << argPrefix(Arg.ArgName) << Arg.ArgName;
+  return OS;
+}
+
 class CommandLineParser {
 public:
   // Globals for name and overview of program.  Program name is not a string to
@@ -1339,12 +1368,12 @@
 if (!Handler) {
   if (SinkOpts.empty()) {
 *Errs << ProgramName << ": Unknown command line argument '" << argv[i]
-  << "'.  Try: '" << argv[0] << " -help'\n";
+  << "'.  Try: '" << argv[0] << " --help'\n";
 
 if (NearestHandler) {
   // If we know a near match, report it as well.
-  *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
- << "'?\n";
+  *Errs << ProgramName << ": Did you mean '"
+<< PrintArg(NearestHandlerString) << "'?\n";
 }
 
 ErrorParsing = true;
@@ -1378,14 +1407,14 @@
  << ": Not enough positional command line arguments specified!\n"
  << "Must specify at least " << NumPositionalRequired
  << " positional argument" << (NumPositionalRequired > 1 ? "s" : "")
- << ": See: " << argv[0] << " -help\n";
+ << ": See: " << argv[0] << " --help\n";
 
 ErrorParsing = true;
   } else if (!HasUnlimitedPositionals &&
  PositionalVals.size() > PositionalOpts.size()) {
 *Errs << ProgramName << ": Too many positional arguments specified!\n"
   << "Can specify at most " << PositionalOpts.size()
-  << " positional arguments: See: " << argv[0] << " -help\n";
+  << " positional arguments: See: " << argv[0] << " --help\n";
 ErrorParsing = true;
 
   } else if (!ConsumeAfterOpt) {
@@ -1498,7 +1527,7 @@
   if (ArgName.empty())

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2019-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:135-137
+const UnaryExprOrTypeTraitExpr *LeftUnaryExpr =
+cast(Left);
+const UnaryExprOrTypeTraitExpr *RightUnaryExpr =

`const auto *` since you already spell the type in the initializer.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:832
 
-
 void RedundantExpressionCheck::checkBitwiseExpr(

Not that I oppose the cleanup, but there are a bunch of these kinds of changes 
that are orthogonal to the patch. Feel free to commit these cleanups as an NFC 
commit and then rebase on top of it if you'd like, but they shouldn't be in 
this patch.


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

https://reviews.llvm.org/D55125



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


r359909 - [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

2019-05-03 Thread Don Hinton via cfe-commits
Author: dhinton
Date: Fri May  3 10:47:29 2019
New Revision: 359909

URL: http://llvm.org/viewvc/llvm-project?rev=359909=rev
Log:
[CommandLine] Change help output to prefix long options with `--` instead of 
`-`. NFC . Part 3 of 5

Summary:
By default, `parseCommandLineOptions()` will accept either a
`-` or `--` prefix for long options -- options with names longer than
a single character.

While this change does not affect behavior, it will be helpful with a
subsequent change that requires long options use the `--` prefix.

Reviewers: rnk, thopre

Reviewed By: thopre

Subscribers: thopre, cfe-commits, hiraditya, llvm-commits

Tags: #llvm, #clang

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

Modified:
cfe/trunk/test/Driver/clang-offload-bundler.c

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=359909=359908=359909=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Fri May  3 10:47:29 2019
@@ -74,10 +74,10 @@
 // CK-ERR6: error: invalid file type specified.
 
 // RUN: not clang-offload-bundler 2>&1 | FileCheck %s --check-prefix CK-ERR7
-// CK-ERR7-DAG: clang-offload-bundler: for the -type option: must be specified 
at least once!
-// CK-ERR7-DAG: clang-offload-bundler: for the -inputs option: must be 
specified at least once!
-// CK-ERR7-DAG: clang-offload-bundler: for the -outputs option: must be 
specified at least once!
-// CK-ERR7-DAG: clang-offload-bundler: for the -targets option: must be 
specified at least once!
+// CK-ERR7-DAG: clang-offload-bundler: for the --type option: must be 
specified at least once!
+// CK-ERR7-DAG: clang-offload-bundler: for the --inputs option: must be 
specified at least once!
+// CK-ERR7-DAG: clang-offload-bundler: for the --outputs option: must be 
specified at least once!
+// CK-ERR7-DAG: clang-offload-bundler: for the --targets option: must be 
specified at least once!
 
 // RUN: not clang-offload-bundler -type=i 
-targets=hxst-powerpcxxle-ibm-linux-gnu,openxp-pxxerpc64le-ibm-linux-gnu,xpenmp-x86_xx-pc-linux-gnu
 -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s 
--check-prefix CK-ERR8
 // CK-ERR8: error: invalid target 'hxst-powerpcxxle-ibm-linux-gnu', unknown 
offloading kind 'hxst', unknown target triple 'powerpcxxle-ibm-linux-gnu'.


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


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

2019-05-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Done in r359907, thanks!


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48292



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


[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

2019-05-03 Thread Jim Ingham via Phabricator via cfe-commits
jingham added a comment.

IIUC, when Expected returns fails, it returns an Error object that might have 
information about what went wrong.  Would it be possible to include the 
contents of that error n the log message?  We often get "I can't run an 
expression in a really complex proprietary code base" and need to try to sort 
out what went wrong from these logs.  So every crumb of information we can 
preserve is useful...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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


[PATCH] D61281: [clang-format] Fixed self assignment

2019-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D61281#1488022 , @RKSimon wrote:

> In D61281#1485833 , @MyDeveloperDay 
> wrote:
>
> > Did this cause some issue? Does this fix something if so can we add a test, 
> > because maybe the line isn't needed
> >
> > I would think we'd want to keep this as an identifier. we are just treating 
> > arg? the same as we would arg
>
>
> @MyDeveloperDay didn't you write this code in D58404 
> ?


I did, but I think the correct code is to remove line 249 completely and  not 
to set the token type to the question mark, unless there was an issue which 
suggested the otherway in which case it should be added as  a test, but I think 
something like PVS-Studio was used to detect this as an issue but then actually 
the wrong assumption is being made as to what was intended. (which is more 
dangerous than the self assignment in the first place)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61281



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


[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

2019-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276



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


[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2019-05-03 Thread Daniel Krupp via Phabricator via cfe-commits
dkrupp updated this revision to Diff 198041.
dkrupp marked 6 inline comments as done.
dkrupp added a comment.

I have fixed all your comments and rebased the patch to the latest master.


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

https://reviews.llvm.org/D55125

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -106,6 +106,7 @@
 
 #define COND_OP_MACRO 9
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {
   int k = 0;
   k += (y < 0) ? x : x;
@@ -114,16 +115,33 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent
   k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO;
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? COND_OP_MACRO + COND_OP_OTHER_MACRO : COND_OP_MACRO + COND_OP_OTHER_MACRO;
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'true' and 'false' expressions are equivalent
 
   // Do not match for conditional operators with a macro and a const.
   k += (y < 0) ? COND_OP_MACRO : 9;
   // Do not match for conditional operators with expressions from different macros.
   k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO;
+  // Do not match for conditional operators when a macro is defined to another macro
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+#define   COND_OP_THIRD_MACRO 8
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+
+  k += (y < 0) ? sizeof(I64) : sizeof(I64);
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(TestConditional(k,y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'true' and 'false' expressions are equivalent
+  // No warning if the expression arguments are different.
+  k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(Valid(k,y));
+
   return k;
 }
 #undef COND_OP_MACRO
 #undef COND_OP_OTHER_MACRO
 
+
 // Overloaded operators that compare two instances of a struct.
 struct MyStruct {
   int x;  
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -131,6 +131,20 @@
   case Stmt::BinaryOperatorClass:
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
+  case Stmt::UnaryExprOrTypeTraitExprClass:
+const UnaryExprOrTypeTraitExpr *LeftUnaryExpr =
+cast(Left);
+const UnaryExprOrTypeTraitExpr *RightUnaryExpr =
+cast(Right);
+if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
+  return LeftUnaryExpr->getArgumentType() ==
+ RightUnaryExpr->getArgumentType();
+else if (!LeftUnaryExpr->isArgumentType() &&
+ !RightUnaryExpr->isArgumentType())
+  return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(),
+   RightUnaryExpr->getArgumentExpr());
+
+return false;
   }
 }
 
@@ -513,7 +527,8 @@
 Value = APSInt(32, false);
   } else if (const auto *OverloadedOperatorExpr =
  Result.Nodes.getNodeAs(OverloadId)) {
-const auto *OverloadedFunctionDecl = dyn_cast_or_null(OverloadedOperatorExpr->getCalleeDecl());
+const auto *OverloadedFunctionDecl =
+dyn_cast_or_null(OverloadedOperatorExpr->getCalleeDecl());
 if (!OverloadedFunctionDecl)
   return false;
 
@@ -529,7 +544,8 @@
 
 Symbol = OverloadedOperatorExpr->getArg(0);
 OperandExpr = OverloadedOperatorExpr;
-Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+Opcode = BinaryOperator::getOverloadedOpcode(
+OverloadedOperatorExpr->getOperator());
 
 return BinaryOperator::isComparisonOp(Opcode);
   } else {
@@ -547,7 +563,8 @@
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *, const ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *,
+   const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast(BinOp->getRHS());
 
@@ -597,23 +614,60 @@
   return true;
 }
 
+static bool isSameToken(const Token , const Token ,
+const SourceManager ) {
+  if (T1.getKind() != T2.getKind())
+return false;
+  if (T1.isNot(tok::raw_identifier))
+return true;
+  if (T1.getLength() != T2.getLength())
+return false;
+  return strncmp(SM.getCharacterData(T1.getLocation()),
+ 

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

2019-05-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Constant evaluator does not work on value-dependent or type-dependent
expressions.

Also fixed bugs uncovered by these assertions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61522

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp

Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6366,7 +6366,9 @@
 APValue Result;
 // FIXME: This doesn't consider value-dependent cases, because doing so is
 // very difficult. Ideally, we should handle them more gracefully.
-if (!EIA->getCond()->EvaluateWithSubstitution(
+if (EIA->getCond()->isValueDependent() ||
+EIA->getCond()->isTypeDependent() ||
+!EIA->getCond()->EvaluateWithSubstitution(
 Result, Context, Function, llvm::makeArrayRef(ConvertedArgs)))
   return EIA;
 
@@ -9547,7 +9549,9 @@
 const FunctionDecl *FD) {
   for (auto *EnableIf : FD->specific_attrs()) {
 bool AlwaysTrue;
-if (!EnableIf->getCond()->EvaluateAsBooleanCondition(AlwaysTrue, Ctx))
+if (EnableIf->getCond()->isValueDependent() ||
+EnableIf->getCond()->isTypeDependent() ||
+!EnableIf->getCond()->EvaluateAsBooleanCondition(AlwaysTrue, Ctx))
   return false;
 if (!AlwaysTrue)
   return false;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -5781,14 +5781,19 @@
   if (CollapseLoopCountExpr) {
 // Found 'collapse' clause - calculate collapse number.
 Expr::EvalResult Result;
-if (CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext()))
+if (!CollapseLoopCountExpr->isValueDependent() &&
+!CollapseLoopCountExpr->isTypeDependent() &&
+CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext()))
   NestedLoopCount = Result.Val.getInt().getLimitedValue();
   }
   unsigned OrderedLoopCount = 1;
   if (OrderedLoopCountExpr) {
 // Found 'ordered' clause - calculate collapse number.
 Expr::EvalResult EVResult;
-if (OrderedLoopCountExpr->EvaluateAsInt(EVResult, SemaRef.getASTContext())) {
+if (!OrderedLoopCountExpr->isValueDependent() &&
+!OrderedLoopCountExpr->isTypeDependent() &&
+OrderedLoopCountExpr->EvaluateAsInt(EVResult,
+SemaRef.getASTContext())) {
   llvm::APSInt Result = EVResult.Val.getInt();
   if (Result.getLimitedValue() < NestedLoopCount) {
 SemaRef.Diag(OrderedLoopCountExpr->getExprLoc(),
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12452,7 +12452,7 @@
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
 CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+  if (!IsConstexpr && !E->isValueDependent() && !E->isTypeDependent())
 CheckForIntOverflow(E);
   DiagnoseMisalignedMembers();
 }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11088,6 +11088,8 @@
 /// will be applied to the result.
 bool Expr::EvaluateAsRValue(EvalResult , const ASTContext ,
 bool InConstantContext) const {
+  assert(!isValueDependent() && !isTypeDependent() &&
+ "Expression evaluator can't be called on a dependent expression.");
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
   Info.InConstantContext = InConstantContext;
   return ::EvaluateAsRValue(this, Result, Ctx, Info);
@@ -11095,6 +11097,8 @@
 
 bool Expr::EvaluateAsBooleanCondition(bool ,
   const ASTContext ) const {
+  assert(!isValueDependent() && !isTypeDependent() &&
+ "Expression evaluator can't be called on a dependent expression.");
   EvalResult Scratch;
   return EvaluateAsRValue(Scratch, Ctx) &&
  HandleConversionToBool(Scratch.Val, Result);
@@ -11102,18 +11106,25 @@
 
 bool Expr::EvaluateAsInt(EvalResult , const ASTContext ,
  SideEffectsKind AllowSideEffects) const {
+  assert(!isValueDependent() && !isTypeDependent() &&
+ "Expression evaluator can't be called on a dependent expression.");
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
   return ::EvaluateAsInt(this, Result, Ctx, AllowSideEffects, Info);
 }
 
 bool Expr::EvaluateAsFixedPoint(EvalResult , const ASTContext ,
 

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

2019-05-03 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In D48292#1490060 , @thakis wrote:

> This test is broken with Xcode 9:
>
>   [...]
>
>
> Does this just need an `// XFAIL: apple-clang-9` or is this unexpected?


This seems to have been fixed in LLVM Clang trunk since the 8 release, and 
AppleClang 9 just doesn't have that fix. So I think it's reasonable to XFAIL it.

Clang 8: https://wandbox.org/permlink/nung9OHLrvObZGjq
Clang 9 (trunk): https://wandbox.org/permlink/y8e3UUx6CDf8x7im


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48292



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


[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In D61165#1488657 , @rjmccall wrote:

> In D61165#1488553 , @erik.pilkington 
> wrote:
>
> > In D61165#1487328 , @rjmccall 
> > wrote:
> >
> > > In D61165#1487100 , 
> > > @erik.pilkington wrote:
> > >
> > > > It seems like the most common sense interpretation here is to just 
> > > > treat the initialization of `G` as completed at the point when the 
> > > > constructor finishes (this appears to be what GCC implements: 
> > > > https://wandbox.org/permlink/R3C9pPhoT4efAdL1).
> > >
> > >
> > > Your example doesn't actually demonstrate that that's what GCC implements 
> > > because it doesn't try to execute the declaration twice.  But you're 
> > > right, GCC does appear to consider the initialization complete as soon as 
> > > the static object's constructor returns normally.  On the other hand, GCC 
> > > gets the array case here wrong: if a static local has array type, and a 
> > > destructor for a temporary required by the first element initializer 
> > > throws, then it does not destroy the element but also (correctly) does 
> > > not mark the variable as fully initialized, and so a second attempt to 
> > > run the initializer will simply construct a new object on top of an 
> > > already-constructed one.  This is arguably correct under the standard — 
> > > the first array element is not a previously-constructed object of 
> > > automatic duration — but I hope it's obvious that that's a defect.
> > >
> > > > So if it was static it would just get destroyed at exit-time, and 
> > > > therefore should be disable-able with `no_destroy`. If the standard 
> > > > implies that we should be doing something else, then we should do that, 
> > > > but I can't seem to find any reference to the rule you're describing.
> > >
> > > Like I said, this is a poorly-specified part of the standard, because at 
> > > least *some* objects with static storage duration have to be destroyed 
> > > when an exception is thrown (because of aggregate initialization), but 
> > > the standard wants to pretend that this isn't true.  I think that 
> > > allowing temporary destructors to cancel initialization uniformly across 
> > > all kinds of objects is the most consistent rule,
> >
> >
> > That's only true for subobjects of an enclosing aggregate before that 
> > aggregate's initialization is complete though, right? So it doesn't seem 
> > like that much of an inconsistency, just mimicking what we would be doing 
> > if an exception was thrown in, say, the body of the ctor before the 
> > object's initialization is completed.
>
>
> Conceptually yes, but formally no.  The standard *could* write this rule as 
> "all currently-initialized subobjects must be separately destroyed when an 
> exception aborts initialization of the containing aggregate, including 
> constructor bodies and aggregate initialization", but it doesn't actually do 
> so; instead it has specific rules covering the behavior when an exception is 
> thrown out of the body of a constructor, and those rules simply do not apply 
> to aggregate initialization.


Right, I was just trying to draw an analogy. Can you be more specific about the 
inconsistency you mentioned above? What objects with static storage duration 
have to be destroyed when an exception is thrown? Just subobjects of static 
aggregates that had their initialization aborted by an exception? If so, that 
obviously doesn't seem inconsistent.

> Even if it did, though, that wouldn't tell us how to resolve this because 
> this is fundamentally about when exactly the initialization of an object is 
> "complete", which doesn't seem to be clearly defined in the standard.  
> There's a rule for when a *constructor* is complete, but among other things, 
> not all initializations involve constructors.

Yeah, I agree that this is the fundamental problem here, and that the standard 
isn't much help. I'm trying to understand why you think this choice of 
semantics is better (or at least, good enough to make us hedge our bets here at 
the cost of keeping this feature simple and useful). I'm not seeing the 
aggregate initialization inconsistency you mentioned above. If the standard 
wanted us to call the destructor before the object's initialization was 
formally complete, then that seems like something they would mention. Other 
implementations (well, GCC) seem to have made the opposite decision, which I 
think makes more intuitive sense.

>>> but if the standard wants to use a rule that non-aggregate initialization 
>>> of static and thread-local locals is complete as soon as the constructor 
>>> (or assignment) completes, as opposed to the end of the full-expression, 
>>> that's also implementable.
>> 
>> So what should that path forward be here? I'd really like to get this crash 
>> fixed soon. If we want to consider 

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

2019-05-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This test is broken with Xcode 9:

  -- Testing: 53613 tests, 8 threads --
  Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
  FAIL: libc++ :: 
std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp
 (50845 of 53613)
   TEST 'libc++ :: 
std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp'
 FAILED 
  Command: 
['/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++',
 '-o', 
'/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/Output/is_trivially_destructible.pass.cpp.o',
 '-x', 'c++', 
'/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp',
 '-c', '-D_LIBCPP_DISABLE_AVAILABILITY', '-v', '-arch', 'x86_64', 
'-mmacosx-version-min=10.12', '-ftemplate-depth=270', '-Werror=thread-safety', 
'-std=c++1z', '-include', 
'/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/nasty_macros.hpp',
 '-nostdinc++', 
'-I/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/include', 
'-I/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/include/c++build',
 '-isysroot', 
'/Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk',
 '-I/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support', 
'-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"',
 
'-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/filesystem/Output/dynamic_env"',
 
'-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
 
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/filesystem_dynamic_test_helper.py"',
 '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', 
'-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', 
'-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', 
'-Wno-user-defined-literals', '-Wsign-compare', '-Wunused-variable', 
'-Wunused-parameter', '-Wunreachable-code', '-c']
  Exit Code: 1
  Standard Error:
  --
  Apple LLVM version 9.0.0 (clang-900.0.37)
  Target: x86_64-apple-darwin16.7.0
  Thread model: posix
  InstalledDir: 
/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
   
"/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
 -cc1 -triple x86_64-apple-macosx10.12.0 -Wdeprecated-objc-isa-usage 
-Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -disable-free 
-disable-llvm-verifier -discard-value-names -main-file-name 
is_trivially_destructible.pass.cpp -mrelocation-model pic -pic-level 2 
-mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose 
-munwind-tables -target-cpu penryn -target-linker-version 302.3 -v 
-dwarf-column-info -debugger-tuning=lldb -coverage-notes-file 
/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/Output/is_trivially_destructible.pass.cpp.gcno
 -nostdinc++ -resource-dir 
/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0
 -include 
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/nasty_macros.hpp
 -isysroot 
/Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk
 -D _LIBCPP_DISABLE_AVAILABILITY -I 
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/include -I 
/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/include/c++build
 -I /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support -D 
"LIBCXX_FILESYSTEM_STATIC_TEST_ROOT=\"/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/input.output/filesystems/Inputs/static_test_env\""
 -D 
"LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT=\"/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/filesystem/Output/dynamic_env\""
 -D 
"LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER=\"/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
 
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/filesystem_dynamic_test_helper.py\""
 -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -stdlib=libc++ -Werror=thread-safety 
-Wall -Wextra -Werror -Wuser-defined-warnings -Wshadow 
-Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move 
-Wno-c++11-extensions -Wno-user-defined-literals -Wsign-compare 
-Wunused-variable -Wunused-parameter -Wunreachable-code -std=c++1z 
-fdeprecated-macro -fdebug-compilation-dir 

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

2019-05-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:10407-10409
   bool IsAllowedCUDACall(const FunctionDecl *Caller,
  const FunctionDecl *Callee) {
+if (llvm::any_of(ExprEvalContexts,

hliao wrote:
> tra wrote:
> > One more thing. The idea of this function is that we're checking if the 
> > `Caller` is allowed to call the `Callee`.
> > However here, you're checking the current context, which may not 
> > necessarily be the same as the caller's. I.e. someone could potentially 
> > call it way after the context is gone.
> > 
> > Currently all uses of this function obtain the caller from `CurContext`, 
> > but if we start relying on other properties of the current context other 
> > than the caller function, then we may neet to pass the context explicitly, 
> > or only pass the Callee and check if it's callable from the current context.
> > 
> > ```
> > 
> as the expression within `decltype` may be quite complicated, the idea here 
> is to relax that rule within `decltype` context, not only for a particular 
> pair of caller/callee.
I understand the idea, but in this case the argument was more about the code 
style. 

Currently the contract is that the function's decision is derived from its 
arguments (and could, perhaps, be a static method). With this patch you start 
relying on the context, but it's not obvious from the function signature. 
Replacing Caller with context, or removing the caller altogether would bring 
the function signature closer to what the function does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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


[PATCH] D61318: [Sema] Prevent binding references with mismatching address spaces to temporaries

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D61318



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


[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

2019-05-03 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D60454



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


[PATCH] D61519: [clangd] Support -fallback-style, similar to clang-format.

2019-05-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61519

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -16,6 +16,7 @@
 #include "index/Background.h"
 #include "index/Serialization.h"
 #include "clang/Basic/Version.h"
+#include "clang/Format/Format.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -231,6 +232,12 @@
 llvm::cl::desc("Enable clang-tidy diagnostics."),
 llvm::cl::init(true));
 
+static llvm::cl::opt
+FallbackStyle("fallback-style",
+  llvm::cl::desc("clang-format style to apply by default when "
+ "no .clang-format file is found"),
+  llvm::cl::init(clang::format::DefaultFallbackStyle));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -352,6 +359,8 @@
   llvm::errs() << "Ignoring -j because -run-synchronously is set.\n";
 WorkerThreadsCount = 0;
   }
+  if (FallbackStyle.getNumOccurrences())
+clang::format::DefaultFallbackStyle = FallbackStyle.c_str();
 
   // Validate command line arguments.
   llvm::Optional InputMirrorStream;


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -16,6 +16,7 @@
 #include "index/Background.h"
 #include "index/Serialization.h"
 #include "clang/Basic/Version.h"
+#include "clang/Format/Format.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -231,6 +232,12 @@
 llvm::cl::desc("Enable clang-tidy diagnostics."),
 llvm::cl::init(true));
 
+static llvm::cl::opt
+FallbackStyle("fallback-style",
+  llvm::cl::desc("clang-format style to apply by default when "
+ "no .clang-format file is found"),
+  llvm::cl::init(clang::format::DefaultFallbackStyle));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -352,6 +359,8 @@
   llvm::errs() << "Ignoring -j because -run-synchronously is set.\n";
 WorkerThreadsCount = 0;
   }
+  if (FallbackStyle.getNumOccurrences())
+clang::format::DefaultFallbackStyle = FallbackStyle.c_str();
 
   // Validate command line arguments.
   llvm::Optional InputMirrorStream;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

Thanks for explaining.  I'll proceed with solution #1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D61509#1489845 , @jdenny wrote:

> In D61509#1489771 , @ABataev wrote:
>
> > In D61509#1489761 , @jdenny wrote:
> >
> > > In D61509#1489752 , @ABataev 
> > > wrote:
> > >
> > > > If the patch is going to be accepted, then definitely it must be 
> > > > solution #1.
> > >
> > >
> > > I certainly have no objection to that and will be happy to implement it, 
> > > but can you help me to understand the rationale?  (Thanks for your quick 
> > > response!)
> >
> >
> > Another one annotation token may significantly change the parsing process. 
> > It will require a lot of rework in the parsing of OpenMP pragmas plus may 
> > lead to some unpredictable results like endless parsing in some cases etc.
>
>
> It seems simple to just consistently replace the one token with two 
> consecutive tokens.  That much is easy enough for me to try out (without yet 
> integrating the locations into the AST, which would take much longer).  Would 
> you expect the test suite to catch the parsing problems you anticipate?
>
> > It is much easier to change the tests rather than modify the whole parsing 
> > procedure.
>
> I presented that argument poorly.  My main concern for solution #1 is that it 
> might break backward compatibility for existing users of the AST.  However, I 
> don't actually have evidence any user cares.  Do we generally offer backward 
> compatibility guarantees here at all?  If this is not an important concern, 
> then solution #1 seems obviously right to me too.


I rather doubt there was many users. Plus, this change will change just the 
source location, nothing else. So I still suggest to use approach #1.
About test suite, we have some tests to try to catch the endless parsing case 
(it was reported already earlier and eas fixed), but the testing is not full, 
of course.
Also, the task of replacing one anotation token by 2 otehrs is not so 
straightforward, it might require some additional code changes and logic 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

In D61509#1489771 , @ABataev wrote:

> In D61509#1489761 , @jdenny wrote:
>
> > In D61509#1489752 , @ABataev wrote:
> >
> > > If the patch is going to be accepted, then definitely it must be solution 
> > > #1.
> >
> >
> > I certainly have no objection to that and will be happy to implement it, 
> > but can you help me to understand the rationale?  (Thanks for your quick 
> > response!)
>
>
> Another one annotation token may significantly change the parsing process. It 
> will require a lot of rework in the parsing of OpenMP pragmas plus may lead 
> to some unpredictable results like endless parsing in some cases etc.


It seems simple to just consistently replace the one token with two consecutive 
tokens.  That much is easy enough for me to try out (without yet integrating 
the locations into the AST, which would take much longer).  Would you expect 
the test suite to catch the parsing problems you anticipate?

> It is much easier to change the tests rather than modify the whole parsing 
> procedure.

I presented that argument poorly.  My main concern for solution #1 is that it 
might break backward compatibility for existing users of the AST.  However, I 
don't actually have evidence any user cares.  Do we generally offer backward 
compatibility guarantees here at all?  If this is not an important concern, 
then solution #1 seems obviously right to me too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

2019-05-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested review of this revision.
Szelethus marked an inline comment as done.
Szelethus added a comment.

I changed the testfiles quite a bit, surprisingly, I'd appreciate another 
accept on this one please :) Seems to me that the changes describe the actual 
reports far better...




Comment at: test/clang-tidy/bugprone-branch-clone.cpp:348
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: switch has 3 consecutive identical 
branches [bugprone-branch-clone]
+// CHECK-MESSAGES: :[[@LINE-6]]:24: note: expanded from macro 'CASE'
+CASE(1)

...except the removal of these -- I guess
```
+if (EndLoc.isMacroID())
+  EndLoc = Context.getSourceManager().getExpansionLoc(EndLoc);
```
is responsible for this, but I'm not really sure how else can I avoid the 
assertation failure.


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

https://reviews.llvm.org/D54757



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


[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

2019-05-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 198025.
Szelethus added a comment.

FIx the above mentioned crash.

  diff --git a/clang-tidy/bugprone/BranchCloneCheck.cpp 
b/clang-tidy/bugprone/BranchCloneCheck.cpp
  index f371231..a898311 100644
  --- a/clang-tidy/bugprone/BranchCloneCheck.cpp
  +++ b/clang-tidy/bugprone/BranchCloneCheck.cpp
  @@ -197,8 +197,19 @@ void BranchCloneCheck::check(const 
MatchFinder::MatchResult ) {
   diag(BeginCurrent->front()->getBeginLoc(),
"switch has %0 consecutive identical branches")
   << static_cast(std::distance(BeginCurrent, EndCurrent));
  -diag(Lexer::getLocForEndOfToken((EndCurrent - 
1)->back()->getEndLoc(),
  -0, *Result.SourceManager,
  +
  +SourceLocation EndLoc = (EndCurrent - 1)->back()->getEndLoc();
  +// If the case statement is generated from a macro, it's 
SourceLocation
  +// may be invalid, resuling in an assertation failure down the line.
  +// While not optimal, try the begin location in this case, it's still
  +// better then nothing.
  +if (EndLoc.isInvalid())
  +  EndLoc = (EndCurrent - 1)->back()->getBeginLoc();
  +
  +if (EndLoc.isMacroID())
  +  EndLoc = Context.getSourceManager().getExpansionLoc(EndLoc);
  +
  +diag(Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
   getLangOpts()),
"last of these clones ends here", DiagnosticIDs::Note);
 }


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

https://reviews.llvm.org/D54757

Files:
  clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tidy/bugprone/BranchCloneCheck.h
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-branch-clone.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-branch-clone.cpp

Index: test/clang-tidy/bugprone-branch-clone.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-branch-clone.cpp
@@ -0,0 +1,1026 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t
+
+void test_basic1(int in, int ) {
+  if (in > 77)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+out++;
+  else
+// CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+out++;
+
+  out++;
+}
+
+void test_basic2(int in, int ) {
+  if (in > 77) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+out++;
+  }
+  else {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+out++;
+  }
+
+  out++;
+}
+
+void test_basic3(int in, int ) {
+  if (in > 77) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+out++;
+  }
+  else
+// CHECK-MESSAGES: :[[@LINE-1]]:3: note: else branch starts here
+out++;
+
+  out++;
+}
+
+void test_basic4(int in, int ) {
+  if (in > 77) {
+out--;
+  }
+  else {
+out++;
+  }
+}
+
+void test_basic5(int in, int ) {
+  if (in > 77) {
+out++;
+  }
+  else {
+out++;
+out++;
+  }
+}
+
+void test_basic6(int in, int ) {
+  if (in > 77) {
+out++;
+  }
+  else {
+out++, out++;
+  }
+}
+
+void test_basic7(int in, int ) {
+  if (in > 77) {
+out++;
+out++;
+  }
+  else
+out++;
+
+  out++;
+}
+
+void test_basic8(int in, int ) {
+  if (in > 77) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+out++;
+out++;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+out++;
+out++;
+  }
+
+  if (in % 2)
+out++;
+}
+
+void test_basic9(int in, int ) {
+  if (in > 77) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+if (in % 2)
+  out++;
+else
+  out--;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+if (in % 2)
+  out++;
+else
+  out--;
+  }
+}
+
+// If we remove the braces from the previous example, the check recognizes it
+// as an `else if`.
+void test_basic10(int in, int ) {
+  if (in > 77)
+if (in % 2)
+  out++;
+else
+  out--;
+  else
+if (in % 2)
+  out++;
+else
+  out--;
+
+}
+
+void test_basic11(int in, int ) {
+  if (in > 77) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
+if (in % 2)
+  out++;
+else
+  out--;
+if (in % 3)
+  out++;
+else
+  out--;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here
+if (in % 2)
+  out++;
+else
+  out--;
+if (in % 3)
+  out++;
+else
+  out--;
+  }
+}
+
+void test_basic12(int in, int ) {
+  

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

2019-05-03 Thread Richard Townsend (Arm) via Phabricator via cfe-commits
richard.townsend.arm added a comment.

Just completed testing... everything seems to be working correctly. So long as 
the all the tests pass, let's commit! :D


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

https://reviews.llvm.org/D60349



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


[PATCH] D61518: [clangd] add CLANG_ENABLE_CLANGD option to build clangd. Require threads.

2019-05-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: gribozavr.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov, mgorny.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61518

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

2019-05-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/misc/HeaderGuardCheck.cpp:21
+ StringRef OldGuard) {
+  if (OldGuard.size()) {
+return OldGuard;

Please use early return.



Comment at: docs/clang-tidy/checks/misc-header-guard.rst:14
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no

Please use single back-tick instead of quotes for option values. Same below.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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


[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

2019-05-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

In D61335#1489680 , @ilya-biryukov 
wrote:

> > As for naming, agreed, but does that concern drop away once we have only a 
> > single RewriteRule definition?
>
> Sure, that won't be an issue.
>
> The use-cases make sense, thanks for the examples. Could you add one or two 
> to the code? Would probably make sense to express them in code as matchers, 
> rather than explaining what we're trying to do in a natural language.


Sounds good, will do.

> Let's proceed with making `RewriteRule` the vocabulary type that we use for 
> transformations like this if we both agree that's a good idea.
>  There are obviously multiple ways to tackle this, which one you had in mind?

Something like this, although if you have a better name than `RewriteAction` 
I'm open to alternatives. e.g. `RewriteCase`?

  struct RewriteAction {
SmallVector Edits;
TextGenerator Explanation;
  };
  
  struct RewriteRule {
ast_matchers::internal::DynTypedMatcher Matcher;
std::vector Actions;
static constexpr llvm::StringLiteral RootId = "___root___";
  };

Or, nest the definition:

  struct RewriteRule {
struct Action {
  SmallVector Edits;
  TextGenerator Explanation;
};
  
ast_matchers::internal::DynTypedMatcher Matcher;
std::vector Actions;
  
static constexpr llvm::StringLiteral RootId = "___root___";
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335



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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

In D61399#1489762 , @ABataev wrote:

> In D61399#1489757 , @gtbercea wrote:
>
> > @ABataev this patch works for both C and C++ and for both math.h and cmath 
> > headers.
>
>
> Did you test it for the builtins? like `pow`, `powf` and `powl`? How are the 
> builtins resolved with this patch?


I have. There are tests for this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

2019-05-03 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment.

> At [nvcc] from CUDA 10, that's not acceptable as we are declaring two 
> functions only differ from the return type. It seems CUDA attributes do not 
> contribute to the function signature. clang is quite different here.

Yes, this is an intentional and more relaxed semantics in clang.  It's also 
sort of the linchpin of our mixed-mode compilation strategy, which is very 
different from nvcc's source-to-source splitting strategy.

Back in the day you could trick nvcc into allowing host/device overloading on 
same-signature functions by slapping a `template` on one or both of them.  
Checking just now it seems they fixed this, but I suspect there are still dark 
corners where nvcc relies on effectively the same behavior as we get in clang 
via true overloading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D61509#1489761 , @jdenny wrote:

> In D61509#1489752 , @ABataev wrote:
>
> > If the patch is going to be accepted, then definitely it must be solution 
> > #1.
>
>
> I certainly have no objection to that and will be happy to implement it, but 
> can you help me to understand the rationale?  (Thanks for your quick 
> response!)


Another one annotation token may significantly change the parsing process. It 
will require a lot of rework in the parsing of OpenMP pragmas plus may lead to 
some unpredictable results like endless parsing in some cases etc. It is much 
easier to change the tests rather than modify the whole parsing procedure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment.

Seems so.


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

https://reviews.llvm.org/D60678



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


[PATCH] D60379: Make precompiled headers reproducible

2019-05-03 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Can this be closed now due to commit r358674?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60379



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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D61399#1489757 , @gtbercea wrote:

> @ABataev this patch works for both C and C++ and for both math.h and cmath 
> headers.


Did you test it for the builtins? like `pow`, `powf` and `powl`? How are the 
builtins resolved with this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

In D61509#1489752 , @ABataev wrote:

> If the patch is going to be accepted, then definitely it must be solution #1.


I certainly have no objection to that and will be happy to implement it, but 
can you help me to understand the rationale?  (Thanks for your quick response!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D61399: [OpenMP][Clang] Support for target math functions

2019-05-03 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

@ABataev this patch works for both C and C++ and for both math.h and cmath 
headers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

If the patch is going to be accepted, then definitely it must be solution #1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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


[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

2019-05-03 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

rjmccall wrote:
> Sorry I didn't catch this before, but I don't see why this test is expected 
> to work.  We can't actually pass a pointer in address space 10 to that 
> constructor.
Ok, I have created a bug to follow up on this issues:
https://bugs.llvm.org/show_bug.cgi?id=41730

It seems that the check is missing here for allowing the address space 
conversions implicitly, but I am afraid if I add it now addr spaces will become 
less usable because implicit conversions can't be setup by the target yet. And 
everyone is using no address space as some sort of `__generic` but 
unofficially. :(

I have some thoughts about adding something like `__generic` address space to 
C++ that can either be resolved by the compiler or supported by HW. I think it 
might help to implement those cases correctly without modifying too much of 
code base. I just struggled to find enough bandwidth to send an RFC but I will 
try to progress on this asap.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



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


[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

2019-05-03 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny created this revision.
jdenny added reviewers: ABataev, rnk, Eugene.Zelenko, akyrtzi, rsmith.
Herald added subscribers: jdoerfert, dexonsmith, guansong.
Herald added a project: clang.

Currently, an OpenMP AST node's recorded location starts at the `omp`
token after the `#pragma` token, and the `#pragma` location isn't
available anywhere that I have found.  However, the `#pragma` location
can be useful when, for example, rewriting a directive using Clang's
Rewrite facility.

This patch makes `#pragma` locations available in any `PragmaHandler`.
However, this patch is incomplete as it does not actually use those
locations.  I'd like to extend the OpenMP implementation to use it, 
but I see two possible approaches, and I need feedback to choose one:

1. Extend `PragmaOpenMPHandler` to set the location of 
`tok::annot_pragma_openmp` to the `#pragma` location so that the `#pragma` 
location then becomes the start location of the OpenMP AST node.  The drawback 
here is that locations in many tests, especially `-ast-dump` tests, will need 
to be updated, and it's not clear to me if any external AST user depends on the 
existing locations.

2. Extend `PragmaOpenMPHandler` to insert a new token, perhaps named 
`tok::annot_pragma_openmp_intro`, before `tok::annot_pragma_openmp` and store 
both their locations in each OpenMP AST node.  The new location would be 
accessed by new member functions so that users of the old location wouldn't see 
a change.  The drawback here is that a lot of the OpenMP AST node construction 
code will need to be updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61509

Files:
  clang/include/clang/Lex/Pragma.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParsePragma.cpp

Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -27,32 +27,32 @@
 struct PragmaAlignHandler : public PragmaHandler {
   explicit PragmaAlignHandler() : PragmaHandler("align") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaGCCVisibilityHandler : public PragmaHandler {
   explicit PragmaGCCVisibilityHandler() : PragmaHandler("visibility") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaOptionsHandler : public PragmaHandler {
   explicit PragmaOptionsHandler() : PragmaHandler("options") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaPackHandler : public PragmaHandler {
   explicit PragmaPackHandler() : PragmaHandler("pack") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaClangSectionHandler : public PragmaHandler {
   explicit PragmaClangSectionHandler(Sema )
  : PragmaHandler("section"), Actions(S) {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 private:
   Sema 
 };
@@ -60,38 +60,38 @@
 struct PragmaMSStructHandler : public PragmaHandler {
   explicit PragmaMSStructHandler() : PragmaHandler("ms_struct") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaUnusedHandler : public PragmaHandler {
   PragmaUnusedHandler() : PragmaHandler("unused") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaWeakHandler : public PragmaHandler {
   explicit PragmaWeakHandler() : PragmaHandler("weak") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaRedefineExtnameHandler : public PragmaHandler {
   explicit PragmaRedefineExtnameHandler() : PragmaHandler("redefine_extname") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
-Token ) override;
+SourceLocation IntroducerLoc, Token ) override;
 };
 
 struct PragmaOpenCLExtensionHandler : public PragmaHandler {
   PragmaOpenCLExtensionHandler() : PragmaHandler("EXTENSION") {}
   void HandlePragma(Preprocessor , PragmaIntroducerKind 

[PATCH] D61506: [OpenCL] Switch to C++17

2019-05-03 Thread Kévin Petit via Phabricator via cfe-commits
kpet added inline comments.



Comment at: include/clang/Frontend/LangStandards.def:162
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 
| Digraphs | OpenCL)
 

Suggest you add `HexFloat` as well. It is part of c++17 and all OpenCL versions.


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

https://reviews.llvm.org/D61506



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


  1   2   >