[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

> Also it feels a bit weird to change the ownership semantics in a derived 
> class, I bet that would violate the Liskov substitution principle.

And then we see that llvm::OwningArrayRef inherits from llvm::ArrayRef ... But 
maybe this direction (strengthening a Pointer into an Owner)
is okay. We'll need to go though the call rules, and see if something breaks 
when the caller passes an Owner to a function that takes a base class (i.e. 
Pointer).
My initial reaction is that this should work in general. An Owner is like a 
Pointer that points to {static}.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: ABataev.
Herald added subscribers: cfe-commits, guansong, jholewinski.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Previously critical regions were emitted with the barrier making it a
worksharing construct though it is not. Also, it leads to incorrect
behavior in Cuda9+. Patch fixes this problem.


Repository:
  rC Clang

https://reviews.llvm.org/D66673

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -107,6 +107,10 @@
   /// Call to void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
   /// global_tid);
   OMPRTL__kmpc_barrier_simple_spmd,
+  /// Call to int32_t __kmpc_warp_active_thread_mask(void);
+  OMPRTL_NVPTX__kmpc_warp_active_thread_mask,
+  /// Call to void __kmpc_syncwarp(int32_t Mask);
+  OMPRTL_NVPTX__kmpc_syncwarp,
 };
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
@@ -1794,6 +1798,20 @@
 ->addFnAttr(llvm::Attribute::Convergent);
 break;
   }
+  case OMPRTL_NVPTX__kmpc_warp_active_thread_mask: {
+// Build int32_t __kmpc_warp_active_thread_mask(void);
+auto *FnTy =
+llvm::FunctionType::get(CGM.Int32Ty, llvm::None, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_warp_active_thread_mask");
+break;
+  }
+  case OMPRTL_NVPTX__kmpc_syncwarp: {
+// Build void __kmpc_syncwarp(kmp_int32 Mask);
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, CGM.Int32Ty, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_syncwarp");
+break;
+  }
   }
   return RTLFn;
 }
@@ -2700,6 +2718,9 @@
   llvm::BasicBlock *BodyBB = CGF.createBasicBlock("omp.critical.body");
   llvm::BasicBlock *ExitBB = CGF.createBasicBlock("omp.critical.exit");
 
+  // Get the mask of active threads in the warp.
+  llvm::Value *Mask = CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_warp_active_thread_mask));
   // Fetch team-local id of the thread.
   llvm::Value *ThreadID = getNVPTXThreadID(CGF);
 
@@ -2740,8 +2761,9 @@
   // Block waits for all threads in current team to finish then increments the
   // counter variable and returns to the loop.
   CGF.EmitBlock(SyncBB);
-  emitBarrierCall(CGF, Loc, OMPD_unknown, /*EmitChecks=*/false,
-  /*ForceSimpleCall=*/true);
+  // Reconverge active threads in the warp.
+  (void)CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_syncwarp), Mask);
 
   llvm::Value *IncCounterVal =
   CGF.Builder.CreateNSWAdd(CounterVal, CGF.Builder.getInt32(1));


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -107,6 +107,10 @@
   /// Call to void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
   /// global_tid);
   OMPRTL__kmpc_barrier_simple_spmd,
+  /// Call to int32_t __kmpc_warp_active_thread_mask(void);
+  OMPRTL_NVPTX__kmpc_warp_active_thread_mask,
+  /// Call to void __kmpc_syncwarp(int32_t Mask);
+  OMPRTL_NVPTX__kmpc_syncwarp,
 };
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
@@ -1794,6 +1798,20 @@
 ->addFnAttr(llvm::Attribute::Convergent);
 break;
   }
+  case OMPRTL_NVPTX__kmpc_warp_active_thread_mask: {
+// Build int32_t __kmpc_warp_active_thread_mask(void);
+auto *FnTy =
+llvm::FunctionType::get(CGM.Int32Ty, llvm::None, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_warp_active_thread_mask");
+break;
+  }
+  case OMPRTL_NVPTX__kmpc_syncwarp: {
+// Build void __kmpc_syncwarp(kmp_int32 Mask);
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, CGM.Int32Ty, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_syncwarp");
+break;
+  }
   }
   return RTLFn;
 }
@@ -2700,6 +2718,9 @@
   llvm::BasicBlock *BodyBB = CGF.createBasicBlock("omp.critical.body");
   llvm::BasicBlock *ExitBB = CGF.createBasicBlock("omp.critical.exit");
 
+  // Get the mask of active threads in the warp.
+  llvm::Value *Mask = CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_warp_active_thread_mask));
   // Fetch team-local id of the thread.
   llvm::Value *ThreadID = getNVPTXThreadID(CGF);
 
@@ -2740,8 +2761,9 @@
   // Block waits for all threads in current team to finish then increments the
   // counter variable and returns to the loop.
   CGF.EmitBlock(SyncBB);
-  emitBarrierCall(CGF, Loc, OMPD_unknown, /*EmitChecks=*/false,
-  /*ForceSimpleCall=*/true);
+  // Reconverge active threads in the warp.
+  (void)CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_syncwarp), Mask);
 
   llvm::Value *IncCounterVal =
   

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> Could it be an issue with python? What is the version you are using?

I would assume so...

  $ /usr/bin/python --version
  Python 3.7.4


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 216921.
ymandel marked 2 inline comments as done.
ymandel added a comment.

comments tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66652

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -710,6 +710,57 @@
   testRule(ruleStrlenSize(), Input, Expected);
 }
 
+// Tests that two changes in a single macro expansion do not lead to conflicts
+// in applying the changes.
+TEST_F(TransformerTest, TwoChangesInOneMacroExpansion) {
+  std::string Input = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(3, 4); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(LIT, LIT); }
+  )cc";
+
+  testRule(makeRule(integerLiteral(), change(text("LIT"))), Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being the arg.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNot) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasLHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being inside the macro.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNotAnchoredInMacro) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasRHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
 // No rewrite is applied when the changed text does not encompass the entirety
 // of the expanded text. That is, the edit would have to be applied to the
 // macro's definition to succeed and editing the expansion point would not
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -150,6 +150,21 @@
   return Ms[0];
 }
 
+SourceLocation tooling::detail::getRuleMatchLoc(const MatchResult ) {
+  auto  = Result.Nodes.getMap();
+  auto Root = NodesMap.find(RewriteRule::RootID);
+  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
+  llvm::Optional RootRange = getRangeForEdit(
+  CharSourceRange::getTokenRange(Root->second.getSourceRange()),
+  *Result.Context);
+  if (RootRange)
+return RootRange->getBegin();
+  // The match doesn't have a coherent range, so fall back to the expansion
+  // location as the "beginning" of the match.
+  return Result.SourceManager->getExpansionLoc(
+  Root->second.getSourceRange().getBegin());
+}
+
 // Finds the case that was "selected" -- that is, whose matcher triggered the
 // `MatchResult`.
 const RewriteRule::Case &
@@ -178,12 +193,7 @@
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  // Verify the existence and validity of the AST node that roots this rule.
-  auto  = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
-  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
-  SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
-  Root->second.getSourceRange().getBegin());
+  SourceLocation RootLoc = tooling::detail::getRuleMatchLoc(Result);
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
   RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -254,6 +254,12 @@
 std::vector
 buildMatchers(const RewriteRule );
 
+/// Gets the beginning location of the source matched by a rewrite rule. If the
+/// match occurs within a macro expansion, returns the beginning of the
+/// expansion point. `Result` must come from the matching of a rewrite rule.
+SourceLocation
+getRuleMatchLoc(const ast_matchers::MatchFinder::MatchResult );
+
 /// Returns 

[PATCH] D66676: [clang-tidy] TransformerClangTidyCheck: change choice of location for diagnostic message.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

This patch changes the location specified to the
`ClangTidyCheck::diag()`. Currently, the beginning of the matched range is
used. This patch uses the beginning of the first fix's range.  This change both
simplifies the code and (hopefully) gives a more intuitive result: the reported
location aligns with the fix(es) provided, rather than the (arbitrary) range of
the rule's match.

N.B. this patch will break the line offset numbers in lit tests if the first fix
is not at the beginning of the match.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66676

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -18,18 +18,18 @@
 namespace tidy {
 namespace utils {
 namespace {
+using namespace ::clang::ast_matchers;
+
 using tooling::change;
 using tooling::IncludeFormat;
+using tooling::node;
 using tooling::RewriteRule;
+using tooling::statement;
 using tooling::text;
 using tooling::stencil::cat;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
-  using namespace ::clang::ast_matchers;
-  using tooling::node;
-  using tooling::statement;
-
   StringRef C = "C", T = "T", E = "E";
   RewriteRule Rule = tooling::makeRule(
   ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
@@ -67,6 +67,57 @@
   EXPECT_EQ(Expected, test::runCheckOnCode(Input));
 }
 
+class IntLitCheck : public TransformerClangTidyCheck {
+public:
+  IntLitCheck(StringRef Name, ClangTidyContext *Context)
+  : TransformerClangTidyCheck(tooling::makeRule(integerLiteral(),
+change(text("LIT")),
+text("no message")),
+  Name, Context) {}
+};
+
+// Tests that two changes in a single macro expansion do not lead to conflicts
+// in applying the changes.
+TEST(TransformerClangTidyCheckTest, TwoChangesInOneMacroExpansion) {
+  const std::string Input = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(3, 4); }
+  )cc";
+  const std::string Expected = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(LIT, LIT); }
+  )cc";
+
+  EXPECT_EQ(Expected, test::runCheckOnCode(Input));
+}
+
+class BinOpCheck : public TransformerClangTidyCheck {
+public:
+  BinOpCheck(StringRef Name, ClangTidyContext *Context)
+  : TransformerClangTidyCheck(
+tooling::makeRule(
+binaryOperator(hasOperatorName("+"), hasRHS(expr().bind("r"))),
+change(node("r"), text("RIGHT")), text("no message")),
+Name, Context) {}
+};
+
+// Tests case where the rule's match spans both source from the macro and its
+// argument, while the change spans only the argument AND there are two such
+// matches.  Here, we expect a conflict between the two matches and the second
+// to be ignored.
+TEST(TransformerClangTidyCheckTest, TwoMatchesInMacroExpansion) {
+  const std::string Input = R"cc(
+#define M(a,b) (1 + a) * (1 + b)
+int f() { return M(3, 4); }
+  )cc";
+  const std::string Expected = R"cc(
+#define M(a,b) (1 + a) * (1 + b)
+int f() { return M(RIGHT, RIGHT); }
+  )cc";
+
+  EXPECT_EQ(Expected, test::runCheckOnCode(Input));
+}
+
 // A trivial rewrite-rule generator that requires Objective-C code.
 Optional needsObjC(const LangOptions ,
 const ClangTidyCheck::OptionsView ) {
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -71,14 +71,6 @@
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  // Verify the existence and validity of the AST node that roots this rule.
-  const ast_matchers::BoundNodes::IDToNodeMap  = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
-  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
-  SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
-  Root->second.getSourceRange().getBegin());
-  assert(RootLoc.isValid() && "Invalid location for Root node of match.");
-
   assert(Rule && "check() should not fire if Rule is None");
   RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, *Rule);
   Expected> Transformations =
@@ -99,10 +91,12 @@
  << 

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

When I understand you correctly, you are thinking about
the cases

  OwningArrayRef getRef();
  
  ArrayRef f() {
ArrayRef r = getRef(); // warning: r points into temporary owner
return r;
  }

and

  ArrayRef getRef() {
OwningArrayRef r;
return r;  // warning: returning reference to local variable
  }

which we will both diagnose correctly with the current analysis. In which case 
would our analysis have problems with the fact that ArrayRef is a Pointer and 
the derived OwningArrayRef is a Owner?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

r369801 - [OPENMP5]Use nonmonotonic modifier by default for non-static and

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Aug 23 12:52:05 2019
New Revision: 369801

URL: http://llvm.org/viewvc/llvm-project?rev=369801=rev
Log:
[OPENMP5]Use nonmonotonic modifier by default for non-static and
non-ordered loops.

According to OpenMP 5.0, 2.9.2 Worksharing-Loop Construct, Desription, If the 
static schedule kind is specified or if the ordered clause is specified, and if 
the nonmonotonic modifier is not specified, the effect is as if the monotonic 
modifier is specified. Otherwise, unless the monotonic modifier is specified, 
the effect is as if the nonmonotonic modifier is specified.
The first part of this requirement is implemented in runtime. Patch adds
support for the second, nonmonotonic, part of this requirement.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/for_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=369801=369800=369801=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Aug 23 12:52:05 2019
@@ -3509,7 +3509,7 @@ bool CGOpenMPRuntime::isDynamic(OpenMPSc
   return Schedule != OMP_sch_static;
 }
 
-static int addMonoNonMonoModifier(OpenMPSchedType Schedule,
+static int addMonoNonMonoModifier(CodeGenModule , OpenMPSchedType Schedule,
   OpenMPScheduleClauseModifier M1,
   OpenMPScheduleClauseModifier M2) {
   int Modifier = 0;
@@ -3543,6 +3543,18 @@ static int addMonoNonMonoModifier(OpenMP
   case OMPC_SCHEDULE_MODIFIER_unknown:
 break;
   }
+  // OpenMP 5.0, 2.9.2 Worksharing-Loop Construct, Desription.
+  // If the static schedule kind is specified or if the ordered clause is
+  // specified, and if the nonmonotonic modifier is not specified, the effect 
is
+  // as if the monotonic modifier is specified. Otherwise, unless the monotonic
+  // modifier is specified, the effect is as if the nonmonotonic modifier is
+  // specified.
+  if (CGM.getLangOpts().OpenMP >= 50 && Modifier == 0) {
+if (!(Schedule == OMP_sch_static_chunked || Schedule == OMP_sch_static ||
+  Schedule == OMP_sch_static_balanced_chunked ||
+  Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static))
+  Modifier = OMP_sch_modifier_nonmonotonic;
+  }
   return Schedule | Modifier;
 }
 
@@ -3567,13 +3579,14 @@ void CGOpenMPRuntime::emitForDispatchIni
   llvm::Value *Chunk = DispatchValues.Chunk ? DispatchValues.Chunk
 : CGF.Builder.getIntN(IVSize, 1);
   llvm::Value *Args[] = {
-  emitUpdateLocation(CGF, Loc), getThreadID(CGF, Loc),
+  emitUpdateLocation(CGF, Loc),
+  getThreadID(CGF, Loc),
   CGF.Builder.getInt32(addMonoNonMonoModifier(
-  Schedule, ScheduleKind.M1, ScheduleKind.M2)), // Schedule type
-  DispatchValues.LB,// Lower
-  DispatchValues.UB,// Upper
-  CGF.Builder.getIntN(IVSize, 1),   // Stride
-  Chunk // Chunk
+  CGM, Schedule, ScheduleKind.M1, ScheduleKind.M2)), // Schedule type
+  DispatchValues.LB, // Lower
+  DispatchValues.UB, // Upper
+  CGF.Builder.getIntN(IVSize, 1),// Stride
+  Chunk  // Chunk
   };
   CGF.EmitRuntimeCall(createDispatchInitFunction(IVSize, IVSigned), Args);
 }
@@ -3615,7 +3628,7 @@ static void emitForStaticInitCall(
   llvm::Value *Args[] = {
   UpdateLocation,
   ThreadId,
-  CGF.Builder.getInt32(addMonoNonMonoModifier(Schedule, M1,
+  CGF.Builder.getInt32(addMonoNonMonoModifier(CGF.CGM, Schedule, M1,
   M2)), // Schedule type
   Values.IL.getPointer(),   // 
   Values.LB.getPointer(),   // 

Modified: cfe/trunk/test/OpenMP/for_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_codegen.cpp?rev=369801=369800=369801=diff
==
--- cfe/trunk/test/OpenMP/for_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_codegen.cpp Fri Aug 23 12:52:05 2019
@@ -1,10 +1,14 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm %s -fexceptions -fcxx-exceptions -o - 
-fsanitize-address-use-after-scope | FileCheck %s --check-prefix=CHECK 
--check-prefix=LIFETIME
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm %s -fexceptions -fcxx-exceptions -o - 
-fsanitize-address-use-after-scope | FileCheck %s --check-prefix=CHECK 
--check-prefix=LIFETIME --check-prefix=OMP45
+// 

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I guess IR test should be affected already and it would be good to have the run 
time test that breaks with barriers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66673



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

r369803 - Do a sweep of symbol internalization. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Fri Aug 23 12:59:23 2019
New Revision: 369803

URL: http://llvm.org/viewvc/llvm-project?rev=369803=rev
Log:
Do a sweep of symbol internalization. NFC.

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=369803=369802=369803=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Fri Aug 23 12:59:23 2019
@@ -823,7 +823,7 @@ static void callSpecialFunction(G &,
   Gen.callFunc(FuncName, QT, Addrs, CGF);
 }
 
-template  std::array createNullAddressArray();
+template  static std::array createNullAddressArray();
 
 template <> std::array createNullAddressArray() {
   return std::array({{Address(nullptr, CharUnits::Zero())}});

Modified: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp?rev=369803=369802=369803=diff
==
--- cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp (original)
+++ cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp Fri Aug 23 
12:59:23 2019
@@ -15,6 +15,7 @@
 
 using namespace clang;
 
+namespace {
 class InterfaceStubFunctionsConsumer : public ASTConsumer {
   CompilerInstance 
   StringRef InFile;
@@ -293,6 +294,7 @@ public:
 writeIfsV1(Instance.getTarget().getTriple(), Symbols, context, Format, 
*OS);
   }
 };
+} // namespace
 
 std::unique_ptr
 GenerateInterfaceIfsExpV1Action::CreateASTConsumer(CompilerInstance ,

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=369803=369802=369803=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Fri Aug 23 12:59:23 2019
@@ -86,8 +86,8 @@ static bool isAttributeLateParsed(const
 }
 
 /// Check if the a start and end source location expand to the same macro.
-bool FindLocsWithCommonFileID(Preprocessor , SourceLocation StartLoc,
-  SourceLocation EndLoc) {
+static bool FindLocsWithCommonFileID(Preprocessor , SourceLocation StartLoc,
+ SourceLocation EndLoc) {
   if (!StartLoc.isMacroID() || !EndLoc.isMacroID())
 return false;
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=369803=369802=369803=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Aug 23 12:59:23 2019
@@ -1060,10 +1060,11 @@ static const Stmt *getTerminatorConditio
   return S;
 }
 
-llvm::StringLiteral StrEnteringLoop = "Entering loop body";
-llvm::StringLiteral StrLoopBodyZero = "Loop body executed 0 times";
-llvm::StringLiteral StrLoopRangeEmpty = "Loop body skipped when range is 
empty";
-llvm::StringLiteral StrLoopCollectionEmpty =
+constexpr llvm::StringLiteral StrEnteringLoop = "Entering loop body";
+constexpr llvm::StringLiteral StrLoopBodyZero = "Loop body executed 0 times";
+constexpr llvm::StringLiteral StrLoopRangeEmpty =
+"Loop body skipped when range is empty";
+constexpr llvm::StringLiteral StrLoopCollectionEmpty =
 "Loop body skipped when collection is empty";
 
 static std::unique_ptr


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

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Yes, it means that the automatic inference of Pointer/Owner from base classes 
has the same issues as all of those automatic inferences: Once can construct a 
case where they are not true.
So for having no false-positives at all, we should avoid this inference by 
default and not consider MutableArrayRef to be a gsl::Pointer.
Instead, we disable the analysis when it sees an unannotated class (like here, 
MutableArrayRef).

Eventually, we can explicitly add the correct annotations to the 
MutableArrayRef and OwningArrayRef  source code,
and provide a command line option to opt into this kind of inference with 
possible false-positives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

Re: r369616 - [analyzer] Enable control dependency condition tracking by default

[Alexander Kornienko via cfe-commits]

I suspect that this patch makes analysis much slower in certain cases. For
example, the clang/test/Analysis/pr37802.cpp test has become ~5 times
slower in some configurations in our environment. This happened somewhere
between r369520 and r369679, and your series of patches seems most
suspicious :). Is it expected? Can it be improved?

On Thu, Aug 22, 2019 at 5:07 AM Kristof Umann via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: szelethus
> Date: Wed Aug 21 20:08:48 2019
> New Revision: 369616
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369616=rev
> Log:
> [analyzer] Enable control dependency condition tracking by default
>
> This patch concludes my GSoC'19 project by enabling track-conditions by
> default.
>
> Differential Revision: https://reviews.llvm.org/D66381
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
> cfe/trunk/test/Analysis/analyzer-config.c
> cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
> cfe/trunk/test/Analysis/return-value-guaranteed.cpp
> cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=369616=369615=369616=diff
>
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def Wed
> Aug 21 20:08:48 2019
> @@ -294,7 +294,7 @@ ANALYZER_OPTION(bool, DisplayCTUProgress
>  ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
>  "Whether to track conditions that are a control
> dependency of "
>  "an already tracked variable.",
> -false)
> +true)
>
>  ANALYZER_OPTION(bool, ShouldTrackConditionsDebug,
> "track-conditions-debug",
>  "Whether to place an event at each tracked condition.",
>
> Modified: cfe/trunk/test/Analysis/analyzer-config.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=369616=369615=369616=diff
>
> ==
> --- cfe/trunk/test/Analysis/analyzer-config.c (original)
> +++ cfe/trunk/test/Analysis/analyzer-config.c Wed Aug 21 20:08:48 2019
> @@ -87,7 +87,7 @@
>  // CHECK-NEXT: suppress-c++-stdlib = true
>  // CHECK-NEXT: suppress-inlined-defensive-checks = true
>  // CHECK-NEXT: suppress-null-return-paths = true
> -// CHECK-NEXT: track-conditions = false
> +// CHECK-NEXT: track-conditions = true
>  // CHECK-NEXT: track-conditions-debug = false
>  // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
>  // CHECK-NEXT: unroll-loops = false
>
> Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m?rev=369616=369615=369616=diff
>
> ==
> --- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
> (original)
> +++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m Wed Aug
> 21 20:08:48 2019
> @@ -16,6 +16,7 @@ extern int coin();
>  return 0;
>}
>return 1; // expected-note{{Returning without writing to '*var'}}
> +  // expected-note@-1{{Returning the value 1, which participates in a
> condition later}}
>  }
>  @end
>
>
> Modified: cfe/trunk/test/Analysis/return-value-guaranteed.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/return-value-guaranteed.cpp?rev=369616=369615=369616=diff
>
> ==
> --- cfe/trunk/test/Analysis/return-value-guaranteed.cpp (original)
> +++ cfe/trunk/test/Analysis/return-value-guaranteed.cpp Wed Aug 21
> 20:08:48 2019
> @@ -24,6 +24,7 @@ bool parseFoo(Foo ) {
>// class-note@-1 {{The value 0 is assigned to 'F.Field'}}
>return !MCAsmParser::Error();
>// class-note@-1 {{'MCAsmParser::Error' returns true}}
> +  // class-note@-2 {{Returning zero, which participates in a condition
> later}}
>  }
>
>  bool parseFile() {
> @@ -57,6 +58,7 @@ namespace test_break {
>  struct MCAsmParser {
>static bool Error() {
>  return false; // class-note {{'MCAsmParser::Error' returns false}}
> +// class-note@-1 {{Returning zero, which participates in a condition
> later}}
>}
>  };
>
> @@ -72,6 +74,7 @@ bool parseFoo(Foo ) {
>return MCAsmParser::Error();
>// class-note@-1 {{Calling 'MCAsmParser::Error'}}
>// class-note@-2 {{Returning from 'MCAsmParser::Error'}}
> +  // class-note@-3 {{Returning zero, which participates in a condition
> later}}
>  }
>
>  bool parseFile() {
>
> Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
> 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D63325#1643023 , @nathanchance 
wrote:

> Done, thanks for looking into this!
>
> F9847579: check-time-trace-sections.json 


Hmm, I still didn't manage to reproduce the issue using your json-file:

  [~/llvm] > cat check-time-trace-sections.json | python 
llvm-project/clang/test/Driver/check-time-trace-sections.py
  [~/llvm] > echo $status
  0

The file itself looks good for this test: all Codegen sections are inside any 
Frontend section and all Frontend sections are before all Backend sections.
Also your log shows no error messages, so test should pass ok. Could it be an 
issue with python? What is the version you are using?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

Re: r369616 - [analyzer] Enable control dependency condition tracking by default

[Kristóf Umann via cfe-commits]

Totally possible, thanks for letting me know! There should be plenty of
room for caching, because I do calculate control dependencies in an excess
for the same CFG, and the retrieval of a basic block from an ExplodedNode
is anything but efficient, though I honestly didnt expect a performance hit
that drastic (and havent experienced it either).

I'll roll out some fixes during the weekend. If the problem persists
after that, I'd be happy to dig deeper.


On Fri, 23 Aug 2019, 20:33 Alexander Kornienko,  wrote:

> I suspect that this patch makes analysis much slower in certain cases. For
> example, the clang/test/Analysis/pr37802.cpp test has become ~5 times
> slower in some configurations in our environment. This happened somewhere
> between r369520 and r369679, and your series of patches seems most
> suspicious :). Is it expected? Can it be improved?
>
> On Thu, Aug 22, 2019 at 5:07 AM Kristof Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: szelethus
>> Date: Wed Aug 21 20:08:48 2019
>> New Revision: 369616
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=369616=rev
>> Log:
>> [analyzer] Enable control dependency condition tracking by default
>>
>> This patch concludes my GSoC'19 project by enabling track-conditions by
>> default.
>>
>> Differential Revision: https://reviews.llvm.org/D66381
>>
>> Modified:
>> cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> cfe/trunk/test/Analysis/analyzer-config.c
>> cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> cfe/trunk/test/Analysis/return-value-guaranteed.cpp
>> cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
>>
>> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> (original)
>> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def Wed
>> Aug 21 20:08:48 2019
>> @@ -294,7 +294,7 @@ ANALYZER_OPTION(bool, DisplayCTUProgress
>>  ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
>>  "Whether to track conditions that are a control
>> dependency of "
>>  "an already tracked variable.",
>> -false)
>> +true)
>>
>>  ANALYZER_OPTION(bool, ShouldTrackConditionsDebug,
>> "track-conditions-debug",
>>  "Whether to place an event at each tracked condition.",
>>
>> Modified: cfe/trunk/test/Analysis/analyzer-config.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/analyzer-config.c (original)
>> +++ cfe/trunk/test/Analysis/analyzer-config.c Wed Aug 21 20:08:48 2019
>> @@ -87,7 +87,7 @@
>>  // CHECK-NEXT: suppress-c++-stdlib = true
>>  // CHECK-NEXT: suppress-inlined-defensive-checks = true
>>  // CHECK-NEXT: suppress-null-return-paths = true
>> -// CHECK-NEXT: track-conditions = false
>> +// CHECK-NEXT: track-conditions = true
>>  // CHECK-NEXT: track-conditions-debug = false
>>  // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
>>  // CHECK-NEXT: unroll-loops = false
>>
>> Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> (original)
>> +++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m Wed
>> Aug 21 20:08:48 2019
>> @@ -16,6 +16,7 @@ extern int coin();
>>  return 0;
>>}
>>return 1; // expected-note{{Returning without writing to '*var'}}
>> +  // expected-note@-1{{Returning the value 1, which participates in a
>> condition later}}
>>  }
>>  @end
>>
>>
>> Modified: cfe/trunk/test/Analysis/return-value-guaranteed.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/return-value-guaranteed.cpp?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/return-value-guaranteed.cpp (original)
>> +++ cfe/trunk/test/Analysis/return-value-guaranteed.cpp Wed Aug 21
>> 20:08:48 2019
>> @@ -24,6 +24,7 @@ bool parseFoo(Foo ) {
>>// class-note@-1 {{The value 0 is assigned to 'F.Field'}}
>>return !MCAsmParser::Error();
>>// class-note@-1 {{'MCAsmParser::Error' returns true}}
>> +  // class-note@-2 {{Returning zero, which participates in a condition
>> later}}
>>  }
>>
>>  bool parseFile() {
>> @@ -57,6 +58,7 @@ namespace 

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[James Nagurne via Phabricator via cfe-commits]

JamesNagurne added a comment.
Herald added a subscriber: ributzka.

@arphaman you disabled this test on Windows, but did not specify exactly how it 
fails.
My team works on an embedded ARM compiler (most similar to arm-none-eabi), and 
we're now seeing failures from DependencyScannerTest. I can't find a buildbot 
failure for this test so I can't cross-reference to see if we have the same 
issue.

Does this failure look similar to what you saw on Windows, or could it be an 
option we're adding as part of the Compilation setup?

  [ RUN  ] DependencyScanner.ScanDepsReuseFilemanager
  .../clang/unittests/Tooling/DependencyScannerTest.cpp:100: Failure
Expected: Deps[1]
Which is: "symlink.h"
  To be equal to: "/root/symlink.h"
  .../clang/unittests/Tooling/DependencyScannerTest.cpp:101: Failure
Expected: Deps[2]
Which is: "header.h"
  To be equal to: "/root/header.h"
  .../clang/unittests/Tooling/DependencyScannerTest.cpp:113: Failure
Expected: Deps[1]
Which is: "symlink.h"
  To be equal to: "/root/symlink.h"
  .../clang/unittests/Tooling/DependencyScannerTest.cpp:114: Failure
Expected: Deps[2]
Which is: "header.h"
  To be equal to: "/root/header.h"
  [  FAILED  ] DependencyScanner.ScanDepsReuseFilemanager (5 ms)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65907



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

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D66486#1643374 , @mgehre wrote:

> > Also it feels a bit weird to change the ownership semantics in a derived 
> > class, I bet that would violate the Liskov substitution principle.
>
> And then we see that llvm::OwningArrayRef inherits from llvm::ArrayRef ... 
> But maybe this direction (strengthening a Pointer into an Owner)
>  is okay.


I am not sure. Consider the following code:

  ArrayRef f() {
ArrayRef r = getRef();
return r;
  }

According to the substitution principle I should be able to replace `r` with a 
derived class:

  ArrayRef f() {
OwningArrayRef r = getOwningRef();
return r;
  }

But this will introduce a lifetime issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Richard Smith via cfe-commits]

On Thu, 22 Aug 2019 at 13:05, Matthias Gehre via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Diana,
> hi Richard,
>
> thank you for the feedback!
>
> Diana,
> I remember that some gcc version had issues with raw string literals
> inside macros. I'll fix that to use normal
> string literals instead.
>
> Richard,
> We are definitely want the gsl::Pointer-based warnings that are enabled by
> default to be free of any false-positives.
> As you expected, this is showing a problem we have in another part of our
> analysis, and we will fix it before landing
> this PR again.
>

It looks like this revert wasn't enough to unblock us. We're currently
unable to release compilers due to the scale of the new enabled-by-default
diagnostics produced by these warnings, and we're not happy about turning
off the existing (zero false positives) warning flags here in order to
unblock our releases, because they're so valuable in catching errors. I'd
expect others will hit similar issues when upgrading Clang. Even if there
were no false positives in the new warning, it appears to be undeployable
as-is because the new warning is behind the same warning flag as an
existing high-value warning. So I think we need the new warnings to be
moved behind different warning flags (a subgroup of ReturnStackAddress
would be OK, but it needs to be independently controllable).

If this can be done imminently, that would be OK, but otherwise I think we
should temporarily roll this back until it can be moved to a separate
warning group.

Both, sorry for the breakage!
>
> Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
> rich...@metafoo.co.uk>:
>
>> Reverted in r369677.
>>
>> On Thu, 22 Aug 2019 at 10:34, Richard Smith 
>> wrote:
>>
>>> Hi Matthias,
>>>
>>> This introduces false positives into -Wreturn-stack-address for an
>>> example such as:
>>>
>>> #include 
>>>
>>> std::vector::iterator downcast_to(std::vector::iterator value)
>>> {
>>>   return *
>>> }
>>>
>>> This breaks an internal build bot for us, so I'm going to revert this
>>> for now (though I expect this isn't the cause of the problem, but is rather
>>> exposing a problem elsewhere).
>>>
>>> If we can make the gsl::Pointer diagnostics false-positive-free, that's
>>> great, but otherwise we should use a different warning flag for the
>>> warnings that involve these annotations and use -Wreturn-stack-address for
>>> only the zero-false-positive cases that it historically diagnosed.
>>>
>>> Thanks, and sorry for the trouble.
>>>
>>> On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: mgehre
 Date: Wed Aug 21 15:08:59 2019
 New Revision: 369591

 URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
 Log:
 [LifetimeAnalysis] Support more STL idioms (template forward
 declaration and DependentNameType)

 Summary:
 This fixes inference of gsl::Pointer on std::set::iterator with
 libstdc++ (the typedef for iterator
 on the template is a DependentNameType - we can only put the
 gsl::Pointer attribute
 on the underlaying record after instantiation)

 inference of gsl::Pointer on std::vector::iterator with libc++ (the
 class was forward-declared,
 we added the gsl::Pointer on the canonical decl (the forward decl), and
 later when the
 template was instantiated, there was no attribute on the definition so
 it was not instantiated).

 and a duplicate gsl::Pointer on some class with libstdc++ (we first
 added an attribute to
 a incomplete instantiation, and then another was copied from the
 template definition
 when the instantiation was completed).

 We now add the attributes to all redeclarations to fix thos issues and
 make their usage easier.

 Reviewers: gribozavr

 Subscribers: Szelethus, xazax.hun, cfe-commits

 Tags: #clang

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

 Added:
 cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp
 Modified:
 cfe/trunk/lib/Sema/SemaAttr.cpp
 cfe/trunk/lib/Sema/SemaDeclAttr.cpp
 cfe/trunk/lib/Sema/SemaInit.cpp
 cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
 cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
 cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp
 cfe/trunk/unittests/Sema/CMakeLists.txt

 Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=369591=369590=369591=diff

 ==
 --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaAttr.cpp Wed Aug 21 15:08:59 2019
 @@ -88,13 +88,11 @@ void Sema::AddMsStructLayoutForRecord(Re
  template 
  static void addGslOwnerPointerAttributeIfNotExisting(ASTContext
 ,
  

Re: r369616 - [analyzer] Enable control dependency condition tracking by default

[Artem Dergachev via cfe-commits]

I had a look and i don't observe any regressions here. Both this test 
alone and the whole test suite in general take as much time as on 
r369520 for me. Additionally, -analyzer-stats doesn't indicate that any 
significant amount of time was spent in bug report post-processing.


On 8/23/19 11:41 AM, Kristóf Umann wrote:
Totally possible, thanks for letting me know! There should be plenty 
of room for caching, because I do calculate control dependencies in an 
excess for the same CFG, and the retrieval of a basic block from an 
ExplodedNode is anything but efficient, though I honestly didnt expect 
a performance hit that drastic (and havent experienced it either).


I'll roll out some fixes during the weekend. If the problem persists 
after that, I'd be happy to dig deeper.



On Fri, 23 Aug 2019, 20:33 Alexander Kornienko, > wrote:


I suspect that this patch makes analysis much slower in certain
cases. For example, the clang/test/Analysis/pr37802.cpp test has
become ~5 times slower in some configurations in our environment.
This happened somewhere between r369520 and r369679, and your
series of patches seems most suspicious :). Is it expected? Can it
be improved?

On Thu, Aug 22, 2019 at 5:07 AM Kristof Umann via cfe-commits
mailto:cfe-commits@lists.llvm.org>>
wrote:

Author: szelethus
Date: Wed Aug 21 20:08:48 2019
New Revision: 369616

URL: http://llvm.org/viewvc/llvm-project?rev=369616=rev
Log:
[analyzer] Enable control dependency condition tracking by default

This patch concludes my GSoC'19 project by enabling
track-conditions by default.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
    cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
cfe/trunk/test/Analysis/return-value-guaranteed.cpp
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=369616=369615=369616=diff

==
---
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
(original)
+++
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
Wed Aug 21 20:08:48 2019
@@ -294,7 +294,7 @@ ANALYZER_OPTION(bool, DisplayCTUProgress
 ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
                 "Whether to track conditions that are a
control dependency of "
                 "an already tracked variable.",
-                false)
+                true)

 ANALYZER_OPTION(bool, ShouldTrackConditionsDebug,
"track-conditions-debug",
                 "Whether to place an event at each tracked
condition.",

Modified: cfe/trunk/test/Analysis/analyzer-config.c
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=369616=369615=369616=diff

==
--- cfe/trunk/test/Analysis/analyzer-config.c (original)
+++ cfe/trunk/test/Analysis/analyzer-config.c Wed Aug 21
20:08:48 2019
@@ -87,7 +87,7 @@
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
-// CHECK-NEXT: track-conditions = false
+// CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false

Modified:
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m?rev=369616=369615=369616=diff

==
---
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
(original)
+++
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
Wed Aug 21 20:08:48 2019
@@ -16,6 +16,7 @@ extern int coin();
     return 0;
   }
   return 1; // expected-note{{Returning without writing to
'*var'}}
+  // expected-note@-1{{Returning the value 1, which
participates in a condition later}}
 }
 @end


Modified: 

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fpga/StructPackAlignCheck.h:1
+//===--- StructPackAlignCheck.h - clang-tidy-*- C++ 
-*-===//
+//

Please add space after clang-tidy. Same in source file.



Comment at: docs/clang-tidy/checks/fpga-struct-pack-align.rst:12
+
+Based on the "Altera SDK for OpenCL: Best Practices Guide" found here:
+https://www.altera.com/en_US/pdfs/literature/hb/opencl-sdk/aocl_optimization_guide.pdf

You could use link syntax. See Fuchsia checks as example.


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

https://reviews.llvm.org/D66564



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

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 216934.
ABataev added a comment.

Fix the test


Repository:
  rC Clang

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

https://reviews.llvm.org/D66673

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_parallel_codegen.cpp


Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -343,6 +343,7 @@
 
 // CHECK-LABEL: define internal void @{{.+}}(i32* noalias %{{.+}}, i32* 
noalias %{{.+}}, i32* dereferenceable{{.*}})
 // CHECK:  [[CC:%.+]] = alloca i32,
+// CHECK:  [[MASK:%.+]] = call i32 @__kmpc_warp_active_thread_mask()
 // CHECK:  [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 // CHECK:  [[NUM_THREADS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
 // CHECK:  store i32 0, i32* [[CC]],
@@ -362,7 +363,7 @@
 // CHECK:  store i32
 // CHECK:  call void @__kmpc_end_critical(
 
-// CHECK:  call void @__kmpc_barrier(%struct.ident_t* @{{.+}}, i32 %{{.+}})
+// CHECK:  call void @__kmpc_syncwarp(i32 [[MASK]])
 // CHECK:  [[NEW_CC_VAL:%.+]] = add nsw i32 [[CC_VAL]], 1
 // CHECK:  store i32 [[NEW_CC_VAL]], i32* [[CC]],
 // CHECK:  br label
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -107,6 +107,10 @@
   /// Call to void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
   /// global_tid);
   OMPRTL__kmpc_barrier_simple_spmd,
+  /// Call to int32_t __kmpc_warp_active_thread_mask(void);
+  OMPRTL_NVPTX__kmpc_warp_active_thread_mask,
+  /// Call to void __kmpc_syncwarp(int32_t Mask);
+  OMPRTL_NVPTX__kmpc_syncwarp,
 };
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
@@ -1794,6 +1798,20 @@
 ->addFnAttr(llvm::Attribute::Convergent);
 break;
   }
+  case OMPRTL_NVPTX__kmpc_warp_active_thread_mask: {
+// Build int32_t __kmpc_warp_active_thread_mask(void);
+auto *FnTy =
+llvm::FunctionType::get(CGM.Int32Ty, llvm::None, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_warp_active_thread_mask");
+break;
+  }
+  case OMPRTL_NVPTX__kmpc_syncwarp: {
+// Build void __kmpc_syncwarp(kmp_int32 Mask);
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, CGM.Int32Ty, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_syncwarp");
+break;
+  }
   }
   return RTLFn;
 }
@@ -2700,6 +2718,9 @@
   llvm::BasicBlock *BodyBB = CGF.createBasicBlock("omp.critical.body");
   llvm::BasicBlock *ExitBB = CGF.createBasicBlock("omp.critical.exit");
 
+  // Get the mask of active threads in the warp.
+  llvm::Value *Mask = CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_warp_active_thread_mask));
   // Fetch team-local id of the thread.
   llvm::Value *ThreadID = getNVPTXThreadID(CGF);
 
@@ -2740,8 +2761,9 @@
   // Block waits for all threads in current team to finish then increments the
   // counter variable and returns to the loop.
   CGF.EmitBlock(SyncBB);
-  emitBarrierCall(CGF, Loc, OMPD_unknown, /*EmitChecks=*/false,
-  /*ForceSimpleCall=*/true);
+  // Reconverge active threads in the warp.
+  (void)CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_syncwarp), Mask);
 
   llvm::Value *IncCounterVal =
   CGF.Builder.CreateNSWAdd(CounterVal, CGF.Builder.getInt32(1));


Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -343,6 +343,7 @@
 
 // CHECK-LABEL: define internal void @{{.+}}(i32* noalias %{{.+}}, i32* noalias %{{.+}}, i32* dereferenceable{{.*}})
 // CHECK:  [[CC:%.+]] = alloca i32,
+// CHECK:  [[MASK:%.+]] = call i32 @__kmpc_warp_active_thread_mask()
 // CHECK:  [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 // CHECK:  [[NUM_THREADS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
 // CHECK:  store i32 0, i32* [[CC]],
@@ -362,7 +363,7 @@
 // CHECK:  store i32
 // CHECK:  call void @__kmpc_end_critical(
 
-// CHECK:  call void @__kmpc_barrier(%struct.ident_t* @{{.+}}, i32 %{{.+}})
+// CHECK:  call void @__kmpc_syncwarp(i32 [[MASK]])
 // CHECK:  [[NEW_CC_VAL:%.+]] = add nsw i32 [[CC_VAL]], 1
 // CHECK:  store i32 [[NEW_CC_VAL]], i32* [[CC]],
 // CHECK:  br label
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -107,6 +107,10 @@
   /// Call to void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
   /// global_tid);
   OMPRTL__kmpc_barrier_simple_spmd,
+  /// Call to int32_t __kmpc_warp_active_thread_mask(void);
+  OMPRTL_NVPTX__kmpc_warp_active_thread_mask,
+  

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D66673#1643544 , @jdoerfert wrote:

> I guess IR test should be affected already and it would be good to have the 
> run time test that breaks with barriers.


Runtime test is 
libomptarget/deviceRTLs/nvptx/test/parallel/spmd_parallel_regions.cpp


Repository:
  rC Clang

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

https://reviews.llvm.org/D66673



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

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Gábor Horváth via cfe-commits]

Hi Richard,

I'll move these behind a flag today. Moving forward, it would be great to
have a way to dogfood those warnings without blocking you. We do run them
over ~340 open source projects regularly, but clearly that is not enough :)

Thanks,
Gabor

On Fri, 23 Aug 2019 at 13:03, Richard Smith  wrote:

> On Thu, 22 Aug 2019 at 13:05, Matthias Gehre via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Hi Diana,
>> hi Richard,
>>
>> thank you for the feedback!
>>
>> Diana,
>> I remember that some gcc version had issues with raw string literals
>> inside macros. I'll fix that to use normal
>> string literals instead.
>>
>> Richard,
>> We are definitely want the gsl::Pointer-based warnings that are enabled
>> by default to be free of any false-positives.
>> As you expected, this is showing a problem we have in another part of our
>> analysis, and we will fix it before landing
>> this PR again.
>>
>
> It looks like this revert wasn't enough to unblock us. We're currently
> unable to release compilers due to the scale of the new enabled-by-default
> diagnostics produced by these warnings, and we're not happy about turning
> off the existing (zero false positives) warning flags here in order to
> unblock our releases, because they're so valuable in catching errors. I'd
> expect others will hit similar issues when upgrading Clang. Even if there
> were no false positives in the new warning, it appears to be undeployable
> as-is because the new warning is behind the same warning flag as an
> existing high-value warning. So I think we need the new warnings to be
> moved behind different warning flags (a subgroup of ReturnStackAddress
> would be OK, but it needs to be independently controllable).
>
> If this can be done imminently, that would be OK, but otherwise I think we
> should temporarily roll this back until it can be moved to a separate
> warning group.
>
> Both, sorry for the breakage!
>>
>> Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
>> rich...@metafoo.co.uk>:
>>
>>> Reverted in r369677.
>>>
>>> On Thu, 22 Aug 2019 at 10:34, Richard Smith 
>>> wrote:
>>>
 Hi Matthias,

 This introduces false positives into -Wreturn-stack-address for an
 example such as:

 #include 

 std::vector::iterator downcast_to(std::vector::iterator
 value) {
   return *
 }

 This breaks an internal build bot for us, so I'm going to revert this
 for now (though I expect this isn't the cause of the problem, but is rather
 exposing a problem elsewhere).

 If we can make the gsl::Pointer diagnostics false-positive-free, that's
 great, but otherwise we should use a different warning flag for the
 warnings that involve these annotations and use -Wreturn-stack-address for
 only the zero-false-positive cases that it historically diagnosed.

 Thanks, and sorry for the trouble.

 On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: mgehre
> Date: Wed Aug 21 15:08:59 2019
> New Revision: 369591
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
> Log:
> [LifetimeAnalysis] Support more STL idioms (template forward
> declaration and DependentNameType)
>
> Summary:
> This fixes inference of gsl::Pointer on std::set::iterator with
> libstdc++ (the typedef for iterator
> on the template is a DependentNameType - we can only put the
> gsl::Pointer attribute
> on the underlaying record after instantiation)
>
> inference of gsl::Pointer on std::vector::iterator with libc++ (the
> class was forward-declared,
> we added the gsl::Pointer on the canonical decl (the forward decl),
> and later when the
> template was instantiated, there was no attribute on the definition so
> it was not instantiated).
>
> and a duplicate gsl::Pointer on some class with libstdc++ (we first
> added an attribute to
> a incomplete instantiation, and then another was copied from the
> template definition
> when the instantiation was completed).
>
> We now add the attributes to all redeclarations to fix thos issues and
> make their usage easier.
>
> Reviewers: gribozavr
>
> Subscribers: Szelethus, xazax.hun, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66179
>
> Added:
> cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaAttr.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
> cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp
> cfe/trunk/unittests/Sema/CMakeLists.txt
>
> Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
> URL:
> 

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Yeah, the analysis would work fine in this case. But that would mean that we 
should not propagate the Pointer annotations to derived classes as some of them 
in the wild do not follow the same ownership model.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 216928.
ffrankies added a comment.

Implemented changes requested by Eugene.Zelenko


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

https://reviews.llvm.org/D66564

Files:
  clang-tidy/fpga/CMakeLists.txt
  clang-tidy/fpga/FPGATidyModule.cpp
  clang-tidy/fpga/StructPackAlignCheck.cpp
  clang-tidy/fpga/StructPackAlignCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fpga-struct-pack-align.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/fpga-struct-pack-align.cpp

Index: test/clang-tidy/fpga-struct-pack-align.cpp
===
--- /dev/null
+++ test/clang-tidy/fpga-struct-pack-align.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s fpga-struct-pack-align %t -- -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c 
+
+// Struct needs both alignment and packing
+struct error {
+char a;
+double b;
+char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'error' has inefficient access due to padding, only needs 10 bytes but is using 24 bytes, use "__attribute((packed))" [fpga-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: struct 'error' has inefficient access due to poor alignment. Currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+char a;
+double b;
+char c;
+} __attribute((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'error_packed' has inefficient access due to poor alignment. Currently aligned to 1 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+char a;
+char b;
+char c;
+char d;
+int e;
+double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: struct 'align_only' has inefficient access due to poor alignment. Currently aligned to 8 bytes, but size 16 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+char a;
+double b;
+char c;
+} __attribute((packed)) __attribute((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align' has inefficient access due to poor alignment. Currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+struct bad_align2 {
+char a;
+double b;
+char c;
+} __attribute((packed)) __attribute((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align2' has inefficient access due to poor alignment. Currently aligned to 32 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+struct bad_align3 {
+char a;
+double b;
+char c;
+} __attribute((packed)) __attribute((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align3' has inefficient access due to poor alignment. Currently aligned to 4 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is both perfectly packed and aligned
+struct success {
+char a;
+double b;
+char c;
+} __attribute((packed)) __attribute((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+int a;
+int b;
+int c;
+} __attribute((aligned(16)));
+
+// If struct is properly aligned, packing not needed
+struct error_aligned {
+char a;
+double b;
+char c;
+} __attribute((aligned(16)));
+
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -64,6 +64,7 @@
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``fpga-``  Checks related to OpenCL programming for FPGAs.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.
 ``hicpp-`` Checks related to High Integrity C++ Coding Standard.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -210,6 +210,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+   fpga-struct-pack-align
fuchsia-default-arguments-calls
fuchsia-default-arguments-declarations
fuchsia-header-anon-namespaces (redirects to google-build-namespaces) 
Index: docs/clang-tidy/checks/fpga-struct-pack-align.rst

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Sounds good! Let's do that :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66486



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

[PATCH] D66681: [clang-doc] Bump BitcodeWriter max line number to 32U

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a reviewer: phosek.
juliehockett added a project: clang-tools-extra.

b43039 reports hitting the assert on a very large file, so bumping this to 
allow for larger files.


https://reviews.llvm.org/D66681

Files:
  clang-tools-extra/clang-doc/BitcodeWriter.h


Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;


Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r369829 - PR40674: fix assertion failure if a structured binding declaration has a

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 23 18:23:57 2019
New Revision: 369829

URL: http://llvm.org/viewvc/llvm-project?rev=369829=rev
Log:
PR40674: fix assertion failure if a structured binding declaration has a
tuple-like decomposition that produces value-dependent reference
bindings.

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369829=369828=369829=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Aug 23 18:23:57 2019
@@ -1225,7 +1225,8 @@ static bool checkTupleLikeDecomposition(
 if (E.isInvalid())
   return true;
 RefVD->setInit(E.get());
-RefVD->checkInitIsICE();
+if (!E.get()->isValueDependent())
+  RefVD->checkInitIsICE();
 
 E = S.BuildDeclarationNameExpr(CXXScopeSpec(),
DeclarationNameInfo(B->getDeclName(), Loc),

Modified: cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp?rev=369829=369828=369829=diff
==
--- cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp (original)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp Fri Aug 23 18:23:57 2019
@@ -127,7 +127,7 @@ void referenced_type() {
   using ConstInt3 = decltype(bcr2);
 }
 
-struct C { template int get(); };
+struct C { template int get() const; };
 template<> struct std::tuple_size { static const int value = 1; };
 template<> struct std::tuple_element<0, C> { typedef int type; };
 
@@ -138,6 +138,12 @@ int member_get() {
   return c;
 }
 
+constexpr C c = C();
+template void dependent_binding_PR40674() {
+  const auto &[c] = *p;
+  (void)c;
+}
+
 struct D {
   // FIXME: Emit a note here explaining why this was ignored.
   template struct get {};


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

r369830 - NFC: Rename some sanitizer related lifetime checks

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Fri Aug 23 18:31:38 2019
New Revision: 369830

URL: http://llvm.org/viewvc/llvm-project?rev=369830=rev
Log:
NFC: Rename some sanitizer related lifetime checks

Added:
cfe/trunk/test/CodeGen/lifetime-sanitizer.c
cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp
Removed:
cfe/trunk/test/CodeGen/lifetime-asan.c
cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp

Removed: cfe/trunk/test/CodeGen/lifetime-asan.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-asan.c?rev=369829=auto
==
--- cfe/trunk/test/CodeGen/lifetime-asan.c (original)
+++ cfe/trunk/test/CodeGen/lifetime-asan.c (removed)
@@ -1,21 +0,0 @@
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | FileCheck 
%s -check-prefix=CHECK-O0
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
-// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
-// RUN: FileCheck %s -check-prefix=CHECK-ASAN-USE-AFTER-SCOPE
-
-extern int bar(char *A, int n);
-
-// CHECK-O0-NOT: @llvm.lifetime.start
-int foo(int n) {
-  if (n) {
-// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 10, i8* 
{{.*}})
-char A[10];
-return bar(A, 1);
-// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 10, i8* {{.*}})
-  } else {
-// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 20, i8* 
{{.*}})
-char A[20];
-return bar(A, 2);
-// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 20, i8* {{.*}})
-  }
-}

Added: cfe/trunk/test/CodeGen/lifetime-sanitizer.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-sanitizer.c?rev=369830=auto
==
--- cfe/trunk/test/CodeGen/lifetime-sanitizer.c (added)
+++ cfe/trunk/test/CodeGen/lifetime-sanitizer.c Fri Aug 23 18:31:38 2019
@@ -0,0 +1,21 @@
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | FileCheck 
%s -check-prefix=CHECK-O0
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
+
+extern int bar(char *A, int n);
+
+// CHECK-O0-NOT: @llvm.lifetime.start
+int foo(int n) {
+  if (n) {
+// LIFETIME: @llvm.lifetime.start.p0i8(i64 10, i8* {{.*}})
+char A[10];
+return bar(A, 1);
+// LIFETIME: @llvm.lifetime.end.p0i8(i64 10, i8* {{.*}})
+  } else {
+// LIFETIME: @llvm.lifetime.start.p0i8(i64 20, i8* {{.*}})
+char A[20];
+return bar(A, 2);
+// LIFETIME: @llvm.lifetime.end.p0i8(i64 20, i8* {{.*}})
+  }
+}

Removed: cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp?rev=369829=auto
==
--- cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (removed)
@@ -1,42 +0,0 @@
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 
%s | FileCheck %s -check-prefixes=CHECK,CHECK-O0 
--implicit-check-not=llvm.lifetime
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 
\
-// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
-// RUN: FileCheck %s -check-prefixes=CHECK,CHECK-ASAN-USE-AFTER-SCOPE
-
-extern int bar(char *A, int n);
-
-struct X { X(); ~X(); int *p; };
-struct Y { Y(); int *p; };
-
-extern "C" void a(), b(), c(), d();
-
-// CHECK-LABEL: @_Z3foo
-void foo(int n) {
-  // CHECK: call void @a()
-  a();
-
-  // CHECK: call void @b()
-  // CHECK-ASAN-USE-AFTER-SCOPE: store i1 false
-  // CHECK-ASAN-USE-AFTER-SCOPE: store i1 false
-  // CHECK: br i1
-  //
-  // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start
-  // CHECK-ASAN-USE-AFTER-SCOPE: store i1 true
-  // CHECK: call void @_ZN1XC
-  // CHECK: br label
-  //
-  // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start
-  // CHECK-ASAN-USE-AFTER-SCOPE: store i1 true
-  // CHECK: call void @_ZN1YC
-  // CHECK: br label
-  //
-  // CHECK: call void @c()
-  // CHECK-ASAN-USE-AFTER-SCOPE: br i1
-  // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end
-  // CHECK-ASAN-USE-AFTER-SCOPE: br i1
-  // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end
-  b(), (n ? X().p : Y().p), c();
-
-  // CHECK: call void @d()
-  d();
-}

Added: cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp?rev=369830=auto
==
--- cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp Fri Aug 23 18:31:38 2019
@@ -0,0 +1,50 @@
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
-O0 %s | \
+// RUN:  FileCheck %s 

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 216991.
vitalybuka added a comment.

return hwasan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66695

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/lifetime-sanitizer.c
  clang/test/CodeGenCXX/lifetime-sanitizer.cpp
  compiler-rt/test/msan/loop-scope.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll

Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
===
--- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
+++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
@@ -34,6 +34,21 @@
   ret void
 }
 
+define void @msan() sanitize_memory {
+entry:
+  ; CHECK-LABEL: @msan(
+  %text = alloca i8, align 1
+
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %text)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* %text)
+  ; CHECK: call void @llvm.lifetime.start
+  ; CHECK-NEXT: call void @llvm.lifetime.end
+
+  call void @foo(i8* %text) ; Keep alloca alive
+
+  ret void
+}
+
 define void @no_asan() {
 entry:
   ; CHECK-LABEL: @no_asan(
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3885,6 +3885,7 @@
 // Asan needs to poison memory to detect invalid access which is possible
 // even for empty lifetime range.
 if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) ||
+II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) ||
 II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress))
   break;
 
Index: compiler-rt/test/msan/loop-scope.cpp
===
--- /dev/null
+++ compiler-rt/test/msan/loop-scope.cpp
@@ -0,0 +1,18 @@
+// RUN: %clangxx_msan -O2 %s -o %t && \
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include 
+
+int *p;
+
+int main() {
+  for (int i = 0; i < 3; i++) {
+int x;
+if (i == 0)
+  x = 0;
+p = 
+  }
+  return *p; // BOOM
+  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK:  #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]]
+}
Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -3,6 +3,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -2,6 +2,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -47,13 +47,9 @@
   if (CGOpts.DisableLifetimeMarkers)
 return false;
 
-  // Disable lifetime markers in msan builds.
-  // FIXME: Remove this when msan works with lifetime markers.
-  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
-return false;
-
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
+  // Sanitizers may use markers.
+  if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::Memory)
 return true;
 
   // For now, only in optimized builds.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -523,6 +523,7 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+  !SanOpts.has(SanitizerKind::Memory) &&
   !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
 OldConditional = OutermostConditional;
 

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 216992.
vitalybuka added a comment.

fix compilation error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66695

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/lifetime-sanitizer.c
  clang/test/CodeGenCXX/lifetime-sanitizer.cpp
  compiler-rt/test/msan/loop-scope.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll

Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
===
--- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
+++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
@@ -34,6 +34,21 @@
   ret void
 }
 
+define void @msan() sanitize_memory {
+entry:
+  ; CHECK-LABEL: @msan(
+  %text = alloca i8, align 1
+
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %text)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* %text)
+  ; CHECK: call void @llvm.lifetime.start
+  ; CHECK-NEXT: call void @llvm.lifetime.end
+
+  call void @foo(i8* %text) ; Keep alloca alive
+
+  ret void
+}
+
 define void @no_asan() {
 entry:
   ; CHECK-LABEL: @no_asan(
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3885,6 +3885,7 @@
 // Asan needs to poison memory to detect invalid access which is possible
 // even for empty lifetime range.
 if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) ||
+II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) ||
 II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress))
   break;
 
Index: compiler-rt/test/msan/loop-scope.cpp
===
--- /dev/null
+++ compiler-rt/test/msan/loop-scope.cpp
@@ -0,0 +1,18 @@
+// RUN: %clangxx_msan -O2 %s -o %t && \
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include 
+
+int *p;
+
+int main() {
+  for (int i = 0; i < 3; i++) {
+int x;
+if (i == 0)
+  x = 0;
+p = 
+  }
+  return *p; // BOOM
+  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK:  #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]]
+}
Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -3,6 +3,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -2,6 +2,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -47,13 +47,9 @@
   if (CGOpts.DisableLifetimeMarkers)
 return false;
 
-  // Disable lifetime markers in msan builds.
-  // FIXME: Remove this when msan works with lifetime markers.
-  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
-return false;
-
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
+  // Sanitizers may use markers.
+  if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::Memory))
 return true;
 
   // For now, only in optimized builds.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -523,6 +523,7 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+  !SanOpts.has(SanitizerKind::Memory) &&
   !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
 OldConditional = OutermostConditional;

r369832 - Re-enable DependencyScannerTest on windows with the right fixes

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 23 18:53:40 2019
New Revision: 369832

URL: http://llvm.org/viewvc/llvm-project?rev=369832=rev
Log:
Re-enable DependencyScannerTest on windows with the right fixes

It should now pass.

Modified:
cfe/trunk/unittests/Tooling/DependencyScannerTest.cpp

Modified: cfe/trunk/unittests/Tooling/DependencyScannerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/DependencyScannerTest.cpp?rev=369832=369831=369832=diff
==
--- cfe/trunk/unittests/Tooling/DependencyScannerTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/DependencyScannerTest.cpp Fri Aug 23 18:53:40 
2019
@@ -16,6 +16,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
@@ -26,8 +27,6 @@
 namespace clang {
 namespace tooling {
 
-#ifndef _WIN32
-
 namespace {
 
 /// Prints out all of the gathered dependencies into a string.
@@ -82,9 +81,14 @@ TEST(DependencyScanner, ScanDepsReuseFil
 
   auto VFS = new llvm::vfs::InMemoryFileSystem();
   VFS->setCurrentWorkingDirectory(CWD);
-  VFS->addFile("/root/header.h", 0, llvm::MemoryBuffer::getMemBuffer("\n"));
-  VFS->addHardLink("/root/symlink.h", "/root/header.h");
-  VFS->addFile("/root/test.cpp", 0,
+  auto Sept = llvm::sys::path::get_separator();
+  std::string HeaderPath = llvm::formatv("{0}root{0}header.h", Sept);
+  std::string SymlinkPath = llvm::formatv("{0}root{0}symlink.h", Sept);
+  std::string TestPath = llvm::formatv("{0}root{0}test.cpp", Sept);
+
+  VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addHardLink(SymlinkPath, HeaderPath);
+  VFS->addFile(TestPath, 0,
llvm::MemoryBuffer::getMemBuffer(
"#include \"symlink.h\"\n#include \"header.h\"\n"));
 
@@ -94,11 +98,12 @@ TEST(DependencyScanner, ScanDepsReuseFil
   std::vector Deps;
   TestDependencyScanningAction Action(Deps);
   Tool.run();
+  using llvm::sys::path::convert_to_slash;
   // The first invocation should return dependencies in order of access.
   ASSERT_EQ(Deps.size(), 3u);
-  EXPECT_EQ(Deps[0], "/root/test.cpp");
-  EXPECT_EQ(Deps[1], "/root/symlink.h");
-  EXPECT_EQ(Deps[2], "/root/header.h");
+  EXPECT_EQ(convert_to_slash(Deps[0]), "/root/test.cpp");
+  EXPECT_EQ(convert_to_slash(Deps[1]), "/root/symlink.h");
+  EXPECT_EQ(convert_to_slash(Deps[2]), "/root/header.h");
 
   // The file manager should still have two FileEntries, as one file is a
   // hardlink.
@@ -109,14 +114,12 @@ TEST(DependencyScanner, ScanDepsReuseFil
   Tool.run();
   // The second invocation should have the same order of dependencies.
   ASSERT_EQ(Deps.size(), 3u);
-  EXPECT_EQ(Deps[0], "/root/test.cpp");
-  EXPECT_EQ(Deps[1], "/root/symlink.h");
-  EXPECT_EQ(Deps[2], "/root/header.h");
+  EXPECT_EQ(convert_to_slash(Deps[0]), "/root/test.cpp");
+  EXPECT_EQ(convert_to_slash(Deps[1]), "/root/symlink.h");
+  EXPECT_EQ(convert_to_slash(Deps[2]), "/root/header.h");
 
   EXPECT_EQ(Files.getNumUniqueRealFiles(), 2u);
 }
 
-#endif
-
 } // end namespace tooling
 } // end namespace clang


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

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
vitalybuka added a reviewer: eugenis.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, hiraditya.
Herald added projects: clang, Sanitizers, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66695

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/lifetime-sanitizer.c
  clang/test/CodeGenCXX/lifetime-sanitizer.cpp
  compiler-rt/test/msan/loop-scope.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll

Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
===
--- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
+++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
@@ -34,6 +34,21 @@
   ret void
 }
 
+define void @msan() sanitize_memory {
+entry:
+  ; CHECK-LABEL: @msan(
+  %text = alloca i8, align 1
+
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %text)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* %text)
+  ; CHECK: call void @llvm.lifetime.start
+  ; CHECK-NEXT: call void @llvm.lifetime.end
+
+  call void @foo(i8* %text) ; Keep alloca alive
+
+  ret void
+}
+
 define void @no_asan() {
 entry:
   ; CHECK-LABEL: @no_asan(
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3885,7 +3885,7 @@
 // Asan needs to poison memory to detect invalid access which is possible
 // even for empty lifetime range.
 if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) ||
-II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress))
+II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory))
   break;
 
 if (removeTriviallyEmptyRange(*II, Intrinsic::lifetime_start,
Index: compiler-rt/test/msan/loop-scope.cpp
===
--- /dev/null
+++ compiler-rt/test/msan/loop-scope.cpp
@@ -0,0 +1,18 @@
+// RUN: %clangxx_msan -O2 %s -o %t && \
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include 
+
+int *p;
+
+int main() {
+  for (int i = 0; i < 3; i++) {
+int x;
+if (i == 0)
+  x = 0;
+p = 
+  }
+  return *p; // BOOM
+  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK:  #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]]
+}
Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -3,6 +3,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -2,6 +2,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -47,13 +47,9 @@
   if (CGOpts.DisableLifetimeMarkers)
 return false;
 
-  // Disable lifetime markers in msan builds.
-  // FIXME: Remove this when msan works with lifetime markers.
-  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
-return false;
-
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
+  // Sanitizers may use markers.
+  if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::Memory)
 return true;
 
   // For now, only in optimized builds.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -523,6 +523,7 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+  !SanOpts.has(SanitizerKind::Memory) &&
   

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: erik.pilkington, ahatanak.
Herald added subscribers: ributzka, dexonsmith, jkorous.

When a category/extension doesn't repeat a type bound, corresponding
type parameter is substituted with `id` when used as a type argument. As
a result, in the added test case it was causing errors like

> type argument 'T' (aka 'id') does not satisfy the bound ('id') of 
> type parameter 'T'

We are already checking that type parameters should be consistent
everywhere (see `checkTypeParamListConsistency`) and update
`ObjCTypeParamDecl` to have correct underlying type. And when we use the
type parameter as a method return type or a method parameter type, it is
substituted to the bounded type. But when we use the type parameter as a
type argument, we check `ObjCTypeParamType` that ignores the updated
underlying type and remains `id`.

Fix by desugaring `ObjCTypeParamType` to the underlying type, the same
way we are doing with `TypedefType`.

rdar://problem/54329242


https://reviews.llvm.org/D66696

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/Type.cpp
  clang/test/SemaObjC/parameterized_classes_subst.m


Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@
 - (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting parameter 
types in implementation}}
 }
 @end
+
+// --
+// Use a type parameter as a type argument.
+// --
+// Type bounds in a category/extension are omitted. rdar://problem/54329242
+@interface ParameterizedContainer>
+- (ParameterizedContainer *)inInterface;
+@end
+@interface ParameterizedContainer (Cat)
+- (ParameterizedContainer *)inCategory;
+@end
+@interface ParameterizedContainer ()
+- (ParameterizedContainer *)inExtension;
+@end
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -611,6 +611,10 @@
   initialize(protocols);
 }
 
+QualType ObjCTypeParamType::desugar() const {
+  return getDecl()->getUnderlyingType();
+}
+
 ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base,
ArrayRef typeArgs,
ArrayRef protocols,
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5554,7 +5554,7 @@
 
 public:
   bool isSugared() const { return true; }
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const;
 
   static bool classof(const Type *T) {
 return T->getTypeClass() == ObjCTypeParam;


Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@
 - (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting parameter types in implementation}}
 }
 @end
+
+// --
+// Use a type parameter as a type argument.
+// --
+// Type bounds in a category/extension are omitted. rdar://problem/54329242
+@interface ParameterizedContainer>
+- (ParameterizedContainer *)inInterface;
+@end
+@interface ParameterizedContainer (Cat)
+- (ParameterizedContainer *)inCategory;
+@end
+@interface ParameterizedContainer ()
+- (ParameterizedContainer *)inExtension;
+@end
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -611,6 +611,10 @@
   initialize(protocols);
 }
 
+QualType ObjCTypeParamType::desugar() const {
+  return getDecl()->getUnderlyingType();
+}
+
 ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base,
ArrayRef typeArgs,
ArrayRef protocols,
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5554,7 +5554,7 @@
 
 public:
   bool isSugared() const { return true; }
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const;
 
   static bool classof(const Type *T) {
 return T->getTypeClass() == ObjCTypeParam;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 216994.
vitalybuka added a comment.

update comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66695

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/lifetime-sanitizer.c
  clang/test/CodeGenCXX/lifetime-sanitizer.cpp
  compiler-rt/test/msan/loop-scope.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll

Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
===
--- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
+++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll
@@ -34,6 +34,21 @@
   ret void
 }
 
+define void @msan() sanitize_memory {
+entry:
+  ; CHECK-LABEL: @msan(
+  %text = alloca i8, align 1
+
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %text)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* %text)
+  ; CHECK: call void @llvm.lifetime.start
+  ; CHECK-NEXT: call void @llvm.lifetime.end
+
+  call void @foo(i8* %text) ; Keep alloca alive
+
+  ret void
+}
+
 define void @no_asan() {
 entry:
   ; CHECK-LABEL: @no_asan(
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3885,6 +3885,7 @@
 // Asan needs to poison memory to detect invalid access which is possible
 // even for empty lifetime range.
 if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) ||
+II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) ||
 II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress))
   break;
 
Index: compiler-rt/test/msan/loop-scope.cpp
===
--- /dev/null
+++ compiler-rt/test/msan/loop-scope.cpp
@@ -0,0 +1,18 @@
+// RUN: %clangxx_msan -O2 %s -o %t && \
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include 
+
+int *p;
+
+int main() {
+  for (int i = 0; i < 3; i++) {
+int x;
+if (i == 0)
+  x = 0;
+p = 
+  }
+  return *p; // BOOM
+  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK:  #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]]
+}
Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -3,6 +3,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -2,6 +2,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=memory %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -47,13 +47,9 @@
   if (CGOpts.DisableLifetimeMarkers)
 return false;
 
-  // Disable lifetime markers in msan builds.
-  // FIXME: Remove this when msan works with lifetime markers.
-  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
-return false;
-
-  // Asan uses markers for use-after-scope checks.
-  if (CGOpts.SanitizeAddressUseAfterScope)
+  // Sanitizers may use markers.
+  if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::Memory))
 return true;
 
   // For now, only in optimized builds.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -516,13 +516,12 @@
 
   // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
   // marker. Instead, start the lifetime of a conditional temporary earlier
-  // so that it's unconditional. Don't do this in ASan's use-after-scope
-  // mode so that it gets the more precise lifetime marks. If the type has
-  // a 

[PATCH] D66697: hwasan, codegen: Keep more lifetime markers used for hwasan

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
vitalybuka added a reviewer: eugenis.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
vitalybuka added a parent revision: D66695: msan, codegen, instcombine: Keep 
more lifetime markers used for msan.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66697

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/lifetime-sanitizer.c
  clang/test/CodeGenCXX/lifetime-sanitizer.cpp


Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
-O0 \
 // RUN: -fsanitize=memory %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target aarch64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
-O0 \
+// RUN: -fsanitize=hwaddress %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -5,6 +5,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=memory %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=hwaddress %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -49,6 +49,7 @@
 
   // Sanitizers may use markers.
   if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::HWAddress) ||
   LangOpts.Sanitize.has(SanitizerKind::Memory))
 return true;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -521,6 +521,7 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+  !SanOpts.has(SanitizerKind::HWAddress) &&
   !SanOpts.has(SanitizerKind::Memory) &&
   !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
 OldConditional = OutermostConditional;


Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp
===
--- clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN: -fsanitize=memory %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target aarch64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN: -fsanitize=hwaddress %s | \
+// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/test/CodeGen/lifetime-sanitizer.c
===
--- clang/test/CodeGen/lifetime-sanitizer.c
+++ clang/test/CodeGen/lifetime-sanitizer.c
@@ -5,6 +5,9 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN: -fsanitize=memory %s | \
 // RUN: FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN: -fsanitize=hwaddress %s | \
+// RUN: FileCheck %s -check-prefix=LIFETIME
 
 extern int bar(char *A, int n);
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -49,6 +49,7 @@
 
   // Sanitizers may use markers.
   if (CGOpts.SanitizeAddressUseAfterScope ||
+  LangOpts.Sanitize.has(SanitizerKind::HWAddress) ||
   LangOpts.Sanitize.has(SanitizerKind::Memory))
 return true;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -521,6 +521,7 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
+  !SanOpts.has(SanitizerKind::HWAddress) &&
   !SanOpts.has(SanitizerKind::Memory) &&
   !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
 OldConditional = OutermostConditional;
___
cfe-commits mailing list

r369834 - PR42513: Enter the proper DeclContext before substituting into an

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 23 19:30:00 2019
New Revision: 369834

URL: http://llvm.org/viewvc/llvm-project?rev=369834=rev
Log:
PR42513: Enter the proper DeclContext before substituting into an
default template argument expression.

We already did this for type template parameters and template template
parameters, but apparently forgot to do so for non-type template
parameters. This causes the substituted default argument expression to
be substituted in the proper context, and in particular to properly mark
its subexpressions as odr-used.

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=369834=369833=369834=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Aug 23 19:30:00 2019
@@ -4693,6 +4693,7 @@ SubstDefaultTemplateArgument(Sema 
   for (unsigned i = 0, e = Param->getDepth(); i != e; ++i)
 TemplateArgLists.addOuterTemplateArguments(None);
 
+  Sema::ContextRAII SavedContext(SemaRef, Template->getDeclContext());
   EnterExpressionEvaluationContext ConstantEvaluated(
   SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated);
   return SemaRef.SubstExpr(Param->getDefaultArgument(), TemplateArgLists);

Modified: cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp?rev=369834=369833=369834=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp 
(original)
+++ cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp Fri Aug 
23 19:30:00 2019
@@ -268,13 +268,16 @@ namespace tuple_tests {
   // Don't get caught by surprise when X<...> doesn't even exist in the
   // selected specialization!
   namespace libcxx_2 {
-template struct tuple { // expected-note {{candidate}}
+template struct tuple {
   template struct X { static const bool value = false; };
+  // Substitution into X::value succeeds but produces the
+  // value-dependent expression
+  //   tuple::X<>::value
+  // FIXME: Is that the right behavior?
   template::value> tuple(U &&...u);
-  // expected-note@-1 {{substitution failure [with T = <>, U = ]: cannot reference member of primary template because deduced class 
template specialization 'tuple<>' is an explicit specialization}}
 };
 template <> class tuple<> {};
-tuple a = {1, 2, 3}; // expected-error {{no viable constructor or 
deduction guide}}
+tuple a = {1, 2, 3}; // expected-error {{excess elements in struct 
initializer}}
   }
 
   namespace libcxx_3 {

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp?rev=369834=369833=369834=diff
==
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp Fri Aug 23 19:30:00 
2019
@@ -48,3 +48,20 @@ void Useage() {
 }
 }
 
+namespace PR42513 {
+  template void f();
+  constexpr int WidgetCtor(struct X1*);
+
+  struct X1 {
+friend constexpr int WidgetCtor(X1*);
+  };
+  template
+  struct StandardWidget {
+friend constexpr int WidgetCtor(X1*) {
+  return 0;
+}
+  };
+  template struct StandardWidget;
+
+  void use() { f(); }
+}


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

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D65907#1643364 , @JamesNagurne 
wrote:

> @arphaman you disabled this test on Windows, but did not specify exactly how 
> it fails.
>  My team works on an embedded ARM compiler (most similar to arm-none-eabi), 
> and we're now seeing failures from DependencyScannerTest. I can't find a 
> buildbot failure for this test so I can't cross-reference to see if we have 
> the same issue.
>
> Does this failure look similar to what you saw on Windows, or could it be an 
> option we're adding as part of the Compilation setup?
>
>   [ RUN  ] DependencyScanner.ScanDepsReuseFilemanager
>   .../clang/unittests/Tooling/DependencyScannerTest.cpp:100: Failure
> Expected: Deps[1]
> Which is: "symlink.h"
>   To be equal to: "/root/symlink.h"
>   .../clang/unittests/Tooling/DependencyScannerTest.cpp:101: Failure
> Expected: Deps[2]
> Which is: "header.h"
>   To be equal to: "/root/header.h"
>   .../clang/unittests/Tooling/DependencyScannerTest.cpp:113: Failure
> Expected: Deps[1]
> Which is: "symlink.h"
>   To be equal to: "/root/symlink.h"
>   .../clang/unittests/Tooling/DependencyScannerTest.cpp:114: Failure
> Expected: Deps[2]
> Which is: "header.h"
>   To be equal to: "/root/header.h"
>   [  FAILED  ] DependencyScanner.ScanDepsReuseFilemanager (5 ms)


No the windows test failure was different, there were no Deps at all. I'm 
currently investigating it on a windows VM.

@JamesNagurne I think there's some issue with the working directory, which is 
not added in your case. Which platform are you running your build/test on? 
Which cmake options are you using?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65907



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

[PATCH] D66681: [clang-doc] Bump BitcodeWriter max line number to 32U

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM (small nit, people usually use PRXXX rather than bXXX to refer to issues)


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

https://reviews.llvm.org/D66681



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

[clang-tools-extra] r369811 - [clang-doc] Bump BitcodeWriter max line number to 32U

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri Aug 23 14:14:05 2019
New Revision: 369811

URL: http://llvm.org/viewvc/llvm-project?rev=369811=rev
Log:
[clang-doc] Bump BitcodeWriter max line number to 32U

PR43039 reports hitting the assert on a very large file, so bumping this
to allow for larger files.

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

Modified:
clang-tools-extra/trunk/clang-doc/BitcodeWriter.h

Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.h?rev=369811=369810=369811=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h Fri Aug 23 14:14:05 2019
@@ -40,7 +40,7 @@ struct BitCodeConstants {
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;


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

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: rsmith, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: llvm-commits, Szelethus, Charusso, gamesh411, jfb, 
dkrupp, rnkovacs, hiraditya, javed.absar.
Herald added a project: LLVM.

This patch introduces quite a bit of plumbing, but I took this approach because 
it seems to be the safest, do not run any related code at all when the warnings 
are turned off.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66686

Files:
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/load-addressing-modes.mir
  llvm/test/CodeGen/AArch64/GlobalISel/store-addressing-modes.mir
  llvm/test/CodeGen/AArch64/arm64-fastisel-gep-promote-before-add.ll

Index: llvm/test/CodeGen/AArch64/arm64-fastisel-gep-promote-before-add.ll
===
--- llvm/test/CodeGen/AArch64/arm64-fastisel-gep-promote-before-add.ll
+++ llvm/test/CodeGen/AArch64/arm64-fastisel-gep-promote-before-add.ll
@@ -1,6 +1,6 @@
 ; fastisel should not fold add with non-pointer bitwidth
 ; sext(a) + sext(b) != sext(a + b)
-; RUN: llc -mtriple=arm64-apple-darwin %s -O0 -o - | FileCheck %s
+; RUN: llc -fast-isel -mtriple=arm64-apple-darwin %s -O0 -o - | FileCheck %s
 
 define zeroext i8 @gep_promotion(i8* %ptr) nounwind uwtable ssp {
 entry:
Index: llvm/test/CodeGen/AArch64/GlobalISel/store-addressing-modes.mir
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/GlobalISel/store-addressing-modes.mir
@@ -0,0 +1,168 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=aarch64-unknown-unknown -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
+
+--- |
+  define void @strxrox(i64* %addr) { ret void }
+  define void @strdrox(i64* %addr) { ret void }
+  define void @strwrox(i64* %addr) { ret void }
+  define void @strsrox(i64* %addr) { ret void }
+  define void @strhrox(i64* %addr) { ret void }
+  define void @strqrox(i64* %addr) { ret void }
+  define void @shl(i64* %addr) { ret void }
+...
+
+---
+name:strxrox
+alignment:   2
+legalized:   true
+regBankSelected: true
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body: |
+  bb.0:
+liveins: $x0, $x1, $x2
+; CHECK-LABEL: name: strxrox
+; CHECK: liveins: $x0, $x1, $x2
+; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
+; CHECK: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
+; CHECK: [[COPY2:%[0-9]+]]:gpr64 = COPY $x2
+; CHECK: STRXroX [[COPY2]], [[COPY]], [[COPY1]], 0, 0 :: (store 8 into %ir.addr)
+%0:gpr(p0) = COPY $x0
+%1:gpr(s64) = COPY $x1
+%ptr:gpr(p0) = G_GEP %0, %1
+%3:gpr(s64) = COPY $x2
+G_STORE %3, %ptr :: (store 8 into %ir.addr)
+...
+---
+name:strdrox
+alignment:   2
+legalized:   true
+regBankSelected: true
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body: |
+  bb.0:
+liveins: $x0, $x1, $d2
+; CHECK-LABEL: name: strdrox
+; CHECK: liveins: $x0, $x1, $d2
+; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
+; CHECK: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
+; CHECK: [[COPY2:%[0-9]+]]:fpr64 = COPY $d2
+; CHECK: STRDroX [[COPY2]], [[COPY]], [[COPY1]], 0, 0 :: (store 8 into %ir.addr)
+%0:gpr(p0) = COPY $x0
+%1:gpr(s64) = COPY $x1
+%ptr:gpr(p0) = G_GEP %0, %1
+%3:fpr(s64) = COPY $d2
+G_STORE %3, %ptr :: (store 8 into %ir.addr)
+...
+---
+name:strwrox
+alignment:   2
+legalized:   true
+regBankSelected: true
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body: |
+  bb.0:
+liveins: $x0, $x1, $w2
+; CHECK-LABEL: name: strwrox
+; CHECK: liveins: $x0, $x1, $w2
+; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
+; CHECK: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
+; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY $w2
+; CHECK: STRWroX [[COPY2]], [[COPY]], [[COPY1]], 0, 0 :: (store 4 into %ir.addr)
+%0:gpr(p0) = COPY $x0
+%1:gpr(s64) = COPY $x1
+%ptr:gpr(p0) = G_GEP %0, %1
+%3:gpr(s32) = COPY $w2
+G_STORE %3, %ptr :: (store 4 into %ir.addr)
+...
+---
+name:strsrox
+alignment:   2
+legalized:   true
+regBankSelected: true
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body: |
+  bb.0:
+liveins: $x0, $x1, $s2
+; CHECK-LABEL: name: strsrox
+; CHECK: liveins: $x0, $x1, $s2
+; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
+; CHECK: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
+; CHECK: [[COPY2:%[0-9]+]]:fpr32 = COPY $s2
+; CHECK: STRSroX [[COPY2]], [[COPY]], [[COPY1]], 0, 0 :: (store 4 into %ir.addr)
+%0:gpr(p0) = COPY $x0
+%1:gpr(s64) = COPY $x1
+%ptr:gpr(p0) = G_GEP %0, %1
+%3:fpr(s32) = COPY $s2
+G_STORE %3, %ptr :: (store 4 into %ir.addr)
+...
+---
+name:strhrox
+alignment:  

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Gábor Horváth via cfe-commits]

Hi Richard,

Sorry for the slow response, unfortunately the compile times are not great
on the machine I have access to at the moment.
Here is a patch, I'll commit it if you agree with the approach:
https://reviews.llvm.org/D66686

Basically, the idea is to not run the new warning related code at all when
it is turned off, so other warning flags like ReturnStackAddress should
never be triggered by the new warnings.
In the future, we can come up with something else but I wanted to go for
the safe solution given the urgency.

Cheers,
Gabor

On Fri, 23 Aug 2019 at 13:31, Gábor Horváth  wrote:

> Hi Richard,
>
> I'll move these behind a flag today. Moving forward, it would be great to
> have a way to dogfood those warnings without blocking you. We do run them
> over ~340 open source projects regularly, but clearly that is not enough :)
>
> Thanks,
> Gabor
>
> On Fri, 23 Aug 2019 at 13:03, Richard Smith  wrote:
>
>> On Thu, 22 Aug 2019 at 13:05, Matthias Gehre via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Hi Diana,
>>> hi Richard,
>>>
>>> thank you for the feedback!
>>>
>>> Diana,
>>> I remember that some gcc version had issues with raw string literals
>>> inside macros. I'll fix that to use normal
>>> string literals instead.
>>>
>>> Richard,
>>> We are definitely want the gsl::Pointer-based warnings that are enabled
>>> by default to be free of any false-positives.
>>> As you expected, this is showing a problem we have in another part of
>>> our analysis, and we will fix it before landing
>>> this PR again.
>>>
>>
>> It looks like this revert wasn't enough to unblock us. We're currently
>> unable to release compilers due to the scale of the new enabled-by-default
>> diagnostics produced by these warnings, and we're not happy about turning
>> off the existing (zero false positives) warning flags here in order to
>> unblock our releases, because they're so valuable in catching errors. I'd
>> expect others will hit similar issues when upgrading Clang. Even if there
>> were no false positives in the new warning, it appears to be undeployable
>> as-is because the new warning is behind the same warning flag as an
>> existing high-value warning. So I think we need the new warnings to be
>> moved behind different warning flags (a subgroup of ReturnStackAddress
>> would be OK, but it needs to be independently controllable).
>>
>> If this can be done imminently, that would be OK, but otherwise I think
>> we should temporarily roll this back until it can be moved to a separate
>> warning group.
>>
>> Both, sorry for the breakage!
>>>
>>> Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
>>> rich...@metafoo.co.uk>:
>>>
 Reverted in r369677.

 On Thu, 22 Aug 2019 at 10:34, Richard Smith 
 wrote:

> Hi Matthias,
>
> This introduces false positives into -Wreturn-stack-address for an
> example such as:
>
> #include 
>
> std::vector::iterator downcast_to(std::vector::iterator
> value) {
>   return *
> }
>
> This breaks an internal build bot for us, so I'm going to revert this
> for now (though I expect this isn't the cause of the problem, but is 
> rather
> exposing a problem elsewhere).
>
> If we can make the gsl::Pointer diagnostics false-positive-free,
> that's great, but otherwise we should use a different warning flag for the
> warnings that involve these annotations and use -Wreturn-stack-address for
> only the zero-false-positive cases that it historically diagnosed.
>
> Thanks, and sorry for the trouble.
>
> On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: mgehre
>> Date: Wed Aug 21 15:08:59 2019
>> New Revision: 369591
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
>> Log:
>> [LifetimeAnalysis] Support more STL idioms (template forward
>> declaration and DependentNameType)
>>
>> Summary:
>> This fixes inference of gsl::Pointer on std::set::iterator with
>> libstdc++ (the typedef for iterator
>> on the template is a DependentNameType - we can only put the
>> gsl::Pointer attribute
>> on the underlaying record after instantiation)
>>
>> inference of gsl::Pointer on std::vector::iterator with libc++ (the
>> class was forward-declared,
>> we added the gsl::Pointer on the canonical decl (the forward decl),
>> and later when the
>> template was instantiated, there was no attribute on the definition
>> so it was not instantiated).
>>
>> and a duplicate gsl::Pointer on some class with libstdc++ (we first
>> added an attribute to
>> a incomplete instantiation, and then another was copied from the
>> template definition
>> when the instantiation was completed).
>>
>> We now add the attributes to all redeclarations to fix thos issues
>> and make their usage easier.
>>

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 216964.
xazax.hun added a comment.

- Added a test.


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

https://reviews.llvm.org/D66686

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -Wno-dangling -Wreturn-stack-address -verify %s
+
+struct [[gsl::Owner(int)]] MyIntOwner {
+  MyIntOwner();
+  int *();
+};
+
+struct [[gsl::Pointer(int)]] MyIntPointer {
+  MyIntPointer(int *p = nullptr);
+  MyIntPointer(const MyIntOwner &);
+  int *();
+  MyIntOwner toOwner();
+};
+
+int () {
+  int i;
+  return i; // expected-warning {{reference to stack memory associated with local variable 'i' returned}}
+}
+
+MyIntPointer g() {
+  MyIntOwner o;
+  return o; // No warning, it is disabled.
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6553,11 +6553,13 @@
 
 static void visitLocalsRetainedByInitializer(IndirectLocalPath ,
  Expr *Init, LocalVisitor Visit,
- bool RevisitSubinits);
+ bool RevisitSubinits,
+ bool EnableLifetimeWarnings);
 
 static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath ,
   Expr *Init, ReferenceKind RK,
-  LocalVisitor Visit);
+  LocalVisitor Visit,
+  bool EnableLifetimeWarnings);
 
 template  static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
@@ -6646,9 +6648,9 @@
 Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, true);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true, true);
 Path.pop_back();
   };
 
@@ -6723,9 +6725,9 @@
 Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, false);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true, false);
 Path.pop_back();
   };
 
@@ -6744,7 +6746,8 @@
 /// glvalue expression \c Init.
 static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath ,
   Expr *Init, ReferenceKind RK,
-  LocalVisitor Visit) {
+  LocalVisitor Visit,
+  bool EnableLifetimeWarnings) {
   RevertToOldSizeRAII RAII(Path);
 
   // Walk past any constructs which we can lifetime-extend across.
@@ -6781,7 +6784,8 @@
   else
 // We can't lifetime extend through this but we might still find some
 // retained temporaries.
-return visitLocalsRetainedByInitializer(Path, Init, Visit, true);
+return visitLocalsRetainedByInitializer(Path, Init, Visit, true,
+EnableLifetimeWarnings);
 }
 
 // Step into CXXDefaultInitExprs so we can diagnose cases where a
@@ -6796,11 +6800,12 @@
   if (auto *MTE = dyn_cast(Init)) {
 if (Visit(Path, Local(MTE), RK))
   visitLocalsRetainedByInitializer(Path, MTE->GetTemporaryExpr(), Visit,
-   true);
+   true, EnableLifetimeWarnings);
   }
 
   if (isa(Init)) {
-handleGslAnnotatedTypes(Path, Init, Visit);
+if (EnableLifetimeWarnings)
+  handleGslAnnotatedTypes(Path, Init, Visit);
 return visitLifetimeBoundArguments(Path, Init, Visit);
   }
 
@@ -6821,7 +6826,8 @@
   } else if (VD->getInit() && !isVarOnPath(Path, VD)) {
 Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
 visitLocalsRetainedByReferenceBinding(Path, VD->getInit(),
-  RK_ReferenceBinding, Visit);
+

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 216962.
xazax.hun added a comment.

- Add the actual diff.


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

https://reviews.llvm.org/D66686

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp

Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6553,11 +6553,13 @@
 
 static void visitLocalsRetainedByInitializer(IndirectLocalPath ,
  Expr *Init, LocalVisitor Visit,
- bool RevisitSubinits);
+ bool RevisitSubinits,
+ bool EnableLifetimeWarnings);
 
 static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath ,
   Expr *Init, ReferenceKind RK,
-  LocalVisitor Visit);
+  LocalVisitor Visit,
+  bool EnableLifetimeWarnings);
 
 template  static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
@@ -6646,9 +6648,9 @@
 Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, true);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true, true);
 Path.pop_back();
   };
 
@@ -6723,9 +6725,9 @@
 Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, false);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true, false);
 Path.pop_back();
   };
 
@@ -6744,7 +6746,8 @@
 /// glvalue expression \c Init.
 static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath ,
   Expr *Init, ReferenceKind RK,
-  LocalVisitor Visit) {
+  LocalVisitor Visit,
+  bool EnableLifetimeWarnings) {
   RevertToOldSizeRAII RAII(Path);
 
   // Walk past any constructs which we can lifetime-extend across.
@@ -6781,7 +6784,8 @@
   else
 // We can't lifetime extend through this but we might still find some
 // retained temporaries.
-return visitLocalsRetainedByInitializer(Path, Init, Visit, true);
+return visitLocalsRetainedByInitializer(Path, Init, Visit, true,
+EnableLifetimeWarnings);
 }
 
 // Step into CXXDefaultInitExprs so we can diagnose cases where a
@@ -6796,11 +6800,12 @@
   if (auto *MTE = dyn_cast(Init)) {
 if (Visit(Path, Local(MTE), RK))
   visitLocalsRetainedByInitializer(Path, MTE->GetTemporaryExpr(), Visit,
-   true);
+   true, EnableLifetimeWarnings);
   }
 
   if (isa(Init)) {
-handleGslAnnotatedTypes(Path, Init, Visit);
+if (EnableLifetimeWarnings)
+  handleGslAnnotatedTypes(Path, Init, Visit);
 return visitLifetimeBoundArguments(Path, Init, Visit);
   }
 
@@ -6821,7 +6826,8 @@
   } else if (VD->getInit() && !isVarOnPath(Path, VD)) {
 Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
 visitLocalsRetainedByReferenceBinding(Path, VD->getInit(),
-  RK_ReferenceBinding, Visit);
+  RK_ReferenceBinding, Visit,
+  EnableLifetimeWarnings);
   }
 }
 break;
@@ -6833,13 +6839,15 @@
 // handling all sorts of rvalues passed to a unary operator.
 const UnaryOperator *U = cast(Init);
 if (U->getOpcode() == UO_Deref)
-  visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true);
+  visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true,
+   EnableLifetimeWarnings);
 break;
   }
 
   case Stmt::OMPArraySectionExprClass: {
-visitLocalsRetainedByInitializer(
-Path, cast(Init)->getBase(), Visit, true);
+visitLocalsRetainedByInitializer(Path,
+ cast(Init)->getBase(),
+

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

In D66364#1638026 , @aaron.ballman 
wrote:

> @rsmith are you fine with implementing the diagnostic for these keywords 
> piecemeal based on the pattern from this patch, or do you want to see an 
> omnibus patch that adds all of them at once?


I'm fine with doing it piecemeal.




Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:130
+def ext_c11_feature : Extension<
   "%0 is a C11-specific feature">, InGroup;
 

Please consider rewording this to "%0 is a C11 extension" to match what we do 
for C++XY extensions. (This will also start looking a little silly once C20 
adoption starts; features aren't C11-specific if they're also part of C20.)

As a separate change, though :)



Comment at: clang/test/Sema/thread-specifier.c:127-160
+// expected-warning@14 {{_Thread_local is a C11-specific feature}}
+// expected-warning@15 {{_Thread_local is a C11-specific feature}}
+// expected-warning@16 {{_Thread_local is a C11-specific feature}}
+// expected-warning@22 {{_Thread_local is a C11-specific feature}}
+// expected-warning@23 {{_Thread_local is a C11-specific feature}}
+// expected-warning@31 {{_Thread_local is a C11-specific feature}}
+// expected-warning@40 {{_Thread_local is a C11-specific feature}}

Hardcoding line numbers like this makes test maintenance painful. Using a 
different verify prefix seems like the easiest way to handle this:

```
// RUN: %clang_cc1 [...] -D__thread=_Thread_local -std=c++98 
-verify=expected,thread-local
[...]
__thread int t1; // thread-local-warning {{_Thread_local is a C11-specific 
feature}}
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364



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

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Richard Smith via cfe-commits]

Thank you for the fast turnaround here!

On Fri, 23 Aug 2019 at 15:26, Gábor Horváth via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I committed this in r369817 to keep things moving. If you have any
> suggestion, complaints let me know and I will do my best to solve it
> post-commit ASAP.
>
> On Fri, 23 Aug 2019 at 14:51, Gábor Horváth  wrote:
>
>> Hi Richard,
>>
>> Sorry for the slow response, unfortunately the compile times are not
>> great on the machine I have access to at the moment.
>> Here is a patch, I'll commit it if you agree with the approach:
>> https://reviews.llvm.org/D66686
>>
>> Basically, the idea is to not run the new warning related code at all
>> when it is turned off, so other warning flags like ReturnStackAddress
>> should never be triggered by the new warnings.
>> In the future, we can come up with something else but I wanted to go for
>> the safe solution given the urgency.
>>
>> Cheers,
>> Gabor
>>
>> On Fri, 23 Aug 2019 at 13:31, Gábor Horváth  wrote:
>>
>>> Hi Richard,
>>>
>>> I'll move these behind a flag today. Moving forward, it would be great
>>> to have a way to dogfood those warnings without blocking you. We do run
>>> them over ~340 open source projects regularly, but clearly that is not
>>> enough :)
>>>
>>> Thanks,
>>> Gabor
>>>
>>> On Fri, 23 Aug 2019 at 13:03, Richard Smith 
>>> wrote:
>>>
 On Thu, 22 Aug 2019 at 13:05, Matthias Gehre via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Hi Diana,
> hi Richard,
>
> thank you for the feedback!
>
> Diana,
> I remember that some gcc version had issues with raw string literals
> inside macros. I'll fix that to use normal
> string literals instead.
>
> Richard,
> We are definitely want the gsl::Pointer-based warnings that are
> enabled by default to be free of any false-positives.
> As you expected, this is showing a problem we have in another part of
> our analysis, and we will fix it before landing
> this PR again.
>

 It looks like this revert wasn't enough to unblock us. We're currently
 unable to release compilers due to the scale of the new enabled-by-default
 diagnostics produced by these warnings, and we're not happy about turning
 off the existing (zero false positives) warning flags here in order to
 unblock our releases, because they're so valuable in catching errors. I'd
 expect others will hit similar issues when upgrading Clang. Even if there
 were no false positives in the new warning, it appears to be undeployable
 as-is because the new warning is behind the same warning flag as an
 existing high-value warning. So I think we need the new warnings to be
 moved behind different warning flags (a subgroup of ReturnStackAddress
 would be OK, but it needs to be independently controllable).

 If this can be done imminently, that would be OK, but otherwise I think
 we should temporarily roll this back until it can be moved to a separate
 warning group.

 Both, sorry for the breakage!
>
> Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
> rich...@metafoo.co.uk>:
>
>> Reverted in r369677.
>>
>> On Thu, 22 Aug 2019 at 10:34, Richard Smith 
>> wrote:
>>
>>> Hi Matthias,
>>>
>>> This introduces false positives into -Wreturn-stack-address for an
>>> example such as:
>>>
>>> #include 
>>>
>>> std::vector::iterator downcast_to(std::vector::iterator
>>> value) {
>>>   return *
>>> }
>>>
>>> This breaks an internal build bot for us, so I'm going to revert
>>> this for now (though I expect this isn't the cause of the problem, but 
>>> is
>>> rather exposing a problem elsewhere).
>>>
>>> If we can make the gsl::Pointer diagnostics false-positive-free,
>>> that's great, but otherwise we should use a different warning flag for 
>>> the
>>> warnings that involve these annotations and use -Wreturn-stack-address 
>>> for
>>> only the zero-false-positive cases that it historically diagnosed.
>>>
>>> Thanks, and sorry for the trouble.
>>>
>>> On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: mgehre
 Date: Wed Aug 21 15:08:59 2019
 New Revision: 369591

 URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
 Log:
 [LifetimeAnalysis] Support more STL idioms (template forward
 declaration and DependentNameType)

 Summary:
 This fixes inference of gsl::Pointer on std::set::iterator with
 libstdc++ (the typedef for iterator
 on the template is a DependentNameType - we can only put the
 gsl::Pointer attribute
 on the underlaying record after instantiation)

 inference of gsl::Pointer on std::vector::iterator with libc++ 

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun abandoned this revision.
xazax.hun added a comment.

Committed in https://reviews.llvm.org/rG6379e5c8a441 due to it was urgent for 
some users. Will address any comments post-commit.


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

https://reviews.llvm.org/D66686



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

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2618
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) && Right.is(tok::l_paren))
+  return true;

`if (Style.isCSharp() && Left.is(tok::kw_using))` would suffice as 
`Right.is(tok::l_paren)` is already checked on Line 2613.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

Maybe set `SpaceBeforeParens` to `Always` first in order to really test the new 
behavior?


Repository:
  rC Clang

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

https://reviews.llvm.org/D2



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

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Nico Weber via cfe-commits]

On Thu, Aug 22, 2019 at 4:05 PM Matthias Gehre via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Diana,
> hi Richard,
>
> thank you for the feedback!
>
> Diana,
> I remember that some gcc version had issues with raw string literals
> inside macros. I'll fix that to use normal
> string literals instead.
>

I think it's only a problem if the raw string is in a macro. Instead of

  FOO(R"(asdf)");

you can do

  const char kStr[] = R"(asdf)";
  FOO(kStr);

and gcc should be happy. (In case raw strings buy you something over normal
string literals.)


>
> Richard,
> We are definitely want the gsl::Pointer-based warnings that are enabled by
> default to be free of any false-positives.
> As you expected, this is showing a problem we have in another part of our
> analysis, and we will fix it before landing
> this PR again.
>
> Both, sorry for the breakage!
>
> Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
> rich...@metafoo.co.uk>:
>
>> Reverted in r369677.
>>
>> On Thu, 22 Aug 2019 at 10:34, Richard Smith 
>> wrote:
>>
>>> Hi Matthias,
>>>
>>> This introduces false positives into -Wreturn-stack-address for an
>>> example such as:
>>>
>>> #include 
>>>
>>> std::vector::iterator downcast_to(std::vector::iterator value)
>>> {
>>>   return *
>>> }
>>>
>>> This breaks an internal build bot for us, so I'm going to revert this
>>> for now (though I expect this isn't the cause of the problem, but is rather
>>> exposing a problem elsewhere).
>>>
>>> If we can make the gsl::Pointer diagnostics false-positive-free, that's
>>> great, but otherwise we should use a different warning flag for the
>>> warnings that involve these annotations and use -Wreturn-stack-address for
>>> only the zero-false-positive cases that it historically diagnosed.
>>>
>>> Thanks, and sorry for the trouble.
>>>
>>> On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: mgehre
 Date: Wed Aug 21 15:08:59 2019
 New Revision: 369591

 URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
 Log:
 [LifetimeAnalysis] Support more STL idioms (template forward
 declaration and DependentNameType)

 Summary:
 This fixes inference of gsl::Pointer on std::set::iterator with
 libstdc++ (the typedef for iterator
 on the template is a DependentNameType - we can only put the
 gsl::Pointer attribute
 on the underlaying record after instantiation)

 inference of gsl::Pointer on std::vector::iterator with libc++ (the
 class was forward-declared,
 we added the gsl::Pointer on the canonical decl (the forward decl), and
 later when the
 template was instantiated, there was no attribute on the definition so
 it was not instantiated).

 and a duplicate gsl::Pointer on some class with libstdc++ (we first
 added an attribute to
 a incomplete instantiation, and then another was copied from the
 template definition
 when the instantiation was completed).

 We now add the attributes to all redeclarations to fix thos issues and
 make their usage easier.

 Reviewers: gribozavr

 Subscribers: Szelethus, xazax.hun, cfe-commits

 Tags: #clang

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

 Added:
 cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp
 Modified:
 cfe/trunk/lib/Sema/SemaAttr.cpp
 cfe/trunk/lib/Sema/SemaDeclAttr.cpp
 cfe/trunk/lib/Sema/SemaInit.cpp
 cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
 cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
 cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp
 cfe/trunk/unittests/Sema/CMakeLists.txt

 Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=369591=369590=369591=diff

 ==
 --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaAttr.cpp Wed Aug 21 15:08:59 2019
 @@ -88,13 +88,11 @@ void Sema::AddMsStructLayoutForRecord(Re
  template 
  static void addGslOwnerPointerAttributeIfNotExisting(ASTContext
 ,
   CXXRecordDecl
 *Record) {
 -  CXXRecordDecl *Canonical = Record->getCanonicalDecl();
 -  if (Canonical->hasAttr() ||
 Canonical->hasAttr())
 +  if (Record->hasAttr() || Record->hasAttr())
  return;

 -  Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context,
 -   /*DerefType*/ nullptr,
 -   /*Spelling=*/0));
 +  for (Decl *Redecl : Record->redecls())
 +Redecl->addAttr(Attribute::CreateImplicit(Context,
 /*DerefType=*/nullptr));
  }

  void 

[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:223
+  using visitor_iterator = VisitorList::iterator;
+  using visitor_range = llvm::iterator_range;
+

Szelethus wrote:
> I think I added a visitor range not long ago?
Yup, i just moved it from `BugReport` to `PathSensitiveBugReport`.
(hint: this yellow bar on the left of the insertion indicates that, you can 
also hover it to see where was the code moved from)


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

https://reviews.llvm.org/D66572



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

Re: [clang-tools-extra] r369763 - [clang-tidy] Possibility of displaying duplicate warnings

[Galina Kistanova via cfe-commits]

Hello Kristof,

This commit broke test to few builders:

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

. . .
Failing Tests (1):
Clang Tools :: clang-tidy/duplicate-reports.cpp

Please have a look ASAP?

Thanks

Galina

On Fri, Aug 23, 2019 at 7:56 AM Kristof Umann via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: szelethus
> Date: Fri Aug 23 07:57:27 2019
> New Revision: 369763
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369763=rev
> Log:
> [clang-tidy] Possibility of displaying duplicate warnings
>
> Summary: In case a checker is registered multiple times as an alias, the
> emitted warnings are uniqued by the report message. However, it is random
> which checker name is included in the warning. When processing the output
> of clang-tidy this behavior caused some problems. In this commit the
> uniquing key contains the checker name too.
>
> Reviewers: alexfh, xazax.hun, Szelethus, aaron.ballman, lebedev.ri,
> JonasToth, gribozavr
>
> Reviewed By: alexfh
>
> Subscribers: dkrupp, whisperity, rnkovacs, mgrang, cfe-commits
>
> Patch by Tibor Brunner!
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D65065
>
> Added:
> clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=369763=369762=369763=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Fri
> Aug 23 07:57:27 2019
> @@ -742,8 +742,9 @@ struct LessClangTidyError {
>  const tooling::DiagnosticMessage  = LHS.Message;
>  const tooling::DiagnosticMessage  = RHS.Message;
>
> -return std::tie(M1.FilePath, M1.FileOffset, M1.Message) <
> -   std::tie(M2.FilePath, M2.FileOffset, M2.Message);
> +return
> +  std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
> M1.Message) <
> +  std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName,
> M2.Message);
>}
>  };
>  struct EqualClangTidyError {
>
> Added: clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp?rev=369763=auto
>
> ==
> --- clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp (added)
> +++ clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp Fri Aug
> 23 07:57:27 2019
> @@ -0,0 +1,15 @@
> +// RUN: %check_clang_tidy %s cert-err09-cpp,cert-err61-cpp %t
> +
> +void alwaysThrows() {
> +  int ex = 42;
> +  // CHECK-MESSAGES: warning: throw expression should throw anonymous
> temporary values instead [cert-err09-cpp]
> +  // CHECK-MESSAGES: warning: throw expression should throw anonymous
> temporary values instead [cert-err61-cpp]
> +  throw ex;
> +}
> +
> +void doTheJob() {
> +  try {
> +alwaysThrows();
> +  } catch (int&) {
> +  }
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66681: [clang-doc] Bump BitcodeWriter max line number to 32U

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369811: [clang-doc] Bump BitcodeWriter max line number to 
32U (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66681?vs=216942=216952#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66681

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -40,7 +40,7 @@
   static constexpr unsigned IntSize = 16U;
   static constexpr unsigned StringLengthSize = 16U;
   static constexpr unsigned FilenameLengthSize = 16U;
-  static constexpr unsigned LineNumberSize = 16U;
+  static constexpr unsigned LineNumberSize = 32U;
   static constexpr unsigned ReferenceTypeSize = 8U;
   static constexpr unsigned USRLengthSize = 6U;
   static constexpr unsigned USRBitLengthSize = 8U;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r369817 - [LifetimeAnalysis] Make it possible to disable the new warnings

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Fri Aug 23 15:21:33 2019
New Revision: 369817

URL: http://llvm.org/viewvc/llvm-project?rev=369817=rev
Log:
[LifetimeAnalysis] Make it possible to disable the new warnings

Added:
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaInit.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=369817=369816=369817=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Aug 23 15:21:33 2019
@@ -289,9 +289,11 @@ def OverloadedShiftOpParentheses: DiagGr
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
+def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
+  DanglingGsl,
   ReturnStackAddress]>;
 def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
 def ExpansionToDefined : DiagGroup<"expansion-to-defined">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=369817=369816=369817=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 23 15:21:33 
2019
@@ -8121,7 +8121,7 @@ def warn_dangling_member : Warning<
 def warn_dangling_lifetime_pointer_member : Warning<
   "initializing pointer member %0 to point to a temporary object "
   "whose lifetime is shorter than the lifetime of the constructed object">,
-  InGroup;
+  InGroup;
 def note_lifetime_extending_member_declared_here : Note<
   "%select{%select{reference|'std::initializer_list'}0 member|"
   "member with %select{reference|'std::initializer_list'}0 subobject}1 "
@@ -8143,7 +8143,7 @@ def warn_new_dangling_reference : Warnin
 def warn_dangling_lifetime_pointer : Warning<
   "object backing the pointer "
   "will be destroyed at the end of the full-expression">,
-  InGroup;
+  InGroup;
 def warn_new_dangling_initializer_list : Warning<
   "array backing "
   "%select{initializer list subobject of the allocated object|"

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=369817=369816=369817=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug 23 15:21:33 2019
@@ -6553,11 +6553,13 @@ static bool pathContainsInit(IndirectLoc
 
 static void visitLocalsRetainedByInitializer(IndirectLocalPath ,
  Expr *Init, LocalVisitor Visit,
- bool RevisitSubinits);
+ bool RevisitSubinits,
+ bool EnableLifetimeWarnings);
 
 static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath ,
   Expr *Init, ReferenceKind RK,
-  LocalVisitor Visit);
+  LocalVisitor Visit,
+  bool EnableLifetimeWarnings);
 
 template  static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
@@ -6646,9 +6648,9 @@ static void handleGslAnnotatedTypes(Indi
 Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, true);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true, true);
 Path.pop_back();
   };
 
@@ -6723,9 +6725,9 @@ static void visitLifetimeBoundArguments(
 Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D});
 if (Arg->isGLValue())
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, false);
 else
-  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+  

r369820 - Fix a test to test what the name suggest.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Fri Aug 23 15:26:49 2019
New Revision: 369820

URL: http://llvm.org/viewvc/llvm-project?rev=369820=rev
Log:
Fix a test to test what the name suggest.

Modified:
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp

Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp?rev=369820=369819=369820=diff
==
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp (original)
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg-disabled.cpp Fri Aug 23 
15:26:49 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wno-dangling -Wreturn-stack-address -verify 
%s
+// RUN: %clang_cc1 -fsyntax-only -Wno-dangling-gsl -Wreturn-stack-address 
-verify %s
 
 struct [[gsl::Owner(int)]] MyIntOwner {
   MyIntOwner();


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

Re: r369591 - [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Gábor Horváth via cfe-commits]

I committed this in r369817 to keep things moving. If you have any
suggestion, complaints let me know and I will do my best to solve it
post-commit ASAP.

On Fri, 23 Aug 2019 at 14:51, Gábor Horváth  wrote:

> Hi Richard,
>
> Sorry for the slow response, unfortunately the compile times are not great
> on the machine I have access to at the moment.
> Here is a patch, I'll commit it if you agree with the approach:
> https://reviews.llvm.org/D66686
>
> Basically, the idea is to not run the new warning related code at all when
> it is turned off, so other warning flags like ReturnStackAddress should
> never be triggered by the new warnings.
> In the future, we can come up with something else but I wanted to go for
> the safe solution given the urgency.
>
> Cheers,
> Gabor
>
> On Fri, 23 Aug 2019 at 13:31, Gábor Horváth  wrote:
>
>> Hi Richard,
>>
>> I'll move these behind a flag today. Moving forward, it would be great to
>> have a way to dogfood those warnings without blocking you. We do run them
>> over ~340 open source projects regularly, but clearly that is not enough :)
>>
>> Thanks,
>> Gabor
>>
>> On Fri, 23 Aug 2019 at 13:03, Richard Smith 
>> wrote:
>>
>>> On Thu, 22 Aug 2019 at 13:05, Matthias Gehre via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Hi Diana,
 hi Richard,

 thank you for the feedback!

 Diana,
 I remember that some gcc version had issues with raw string literals
 inside macros. I'll fix that to use normal
 string literals instead.

 Richard,
 We are definitely want the gsl::Pointer-based warnings that are enabled
 by default to be free of any false-positives.
 As you expected, this is showing a problem we have in another part of
 our analysis, and we will fix it before landing
 this PR again.

>>>
>>> It looks like this revert wasn't enough to unblock us. We're currently
>>> unable to release compilers due to the scale of the new enabled-by-default
>>> diagnostics produced by these warnings, and we're not happy about turning
>>> off the existing (zero false positives) warning flags here in order to
>>> unblock our releases, because they're so valuable in catching errors. I'd
>>> expect others will hit similar issues when upgrading Clang. Even if there
>>> were no false positives in the new warning, it appears to be undeployable
>>> as-is because the new warning is behind the same warning flag as an
>>> existing high-value warning. So I think we need the new warnings to be
>>> moved behind different warning flags (a subgroup of ReturnStackAddress
>>> would be OK, but it needs to be independently controllable).
>>>
>>> If this can be done imminently, that would be OK, but otherwise I think
>>> we should temporarily roll this back until it can be moved to a separate
>>> warning group.
>>>
>>> Both, sorry for the breakage!

 Am Do., 22. Aug. 2019 um 19:47 Uhr schrieb Richard Smith <
 rich...@metafoo.co.uk>:

> Reverted in r369677.
>
> On Thu, 22 Aug 2019 at 10:34, Richard Smith 
> wrote:
>
>> Hi Matthias,
>>
>> This introduces false positives into -Wreturn-stack-address for an
>> example such as:
>>
>> #include 
>>
>> std::vector::iterator downcast_to(std::vector::iterator
>> value) {
>>   return *
>> }
>>
>> This breaks an internal build bot for us, so I'm going to revert this
>> for now (though I expect this isn't the cause of the problem, but is 
>> rather
>> exposing a problem elsewhere).
>>
>> If we can make the gsl::Pointer diagnostics false-positive-free,
>> that's great, but otherwise we should use a different warning flag for 
>> the
>> warnings that involve these annotations and use -Wreturn-stack-address 
>> for
>> only the zero-false-positive cases that it historically diagnosed.
>>
>> Thanks, and sorry for the trouble.
>>
>> On Wed, 21 Aug 2019 at 15:07, Matthias Gehre via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: mgehre
>>> Date: Wed Aug 21 15:08:59 2019
>>> New Revision: 369591
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=369591=rev
>>> Log:
>>> [LifetimeAnalysis] Support more STL idioms (template forward
>>> declaration and DependentNameType)
>>>
>>> Summary:
>>> This fixes inference of gsl::Pointer on std::set::iterator with
>>> libstdc++ (the typedef for iterator
>>> on the template is a DependentNameType - we can only put the
>>> gsl::Pointer attribute
>>> on the underlaying record after instantiation)
>>>
>>> inference of gsl::Pointer on std::vector::iterator with libc++ (the
>>> class was forward-declared,
>>> we added the gsl::Pointer on the canonical decl (the forward decl),
>>> and later when the
>>> template was instantiated, there was no attribute on the definition
>>> so it was not instantiated).
>>>
>>> and 

r369822 - [libclang][index][NFC] Fix test for skipping already parsed function bodies

[Jan Korous via cfe-commits]

Author: jkorous
Date: Fri Aug 23 15:51:23 2019
New Revision: 369822

URL: http://llvm.org/viewvc/llvm-project?rev=369822=rev
Log:
[libclang][index][NFC] Fix test for skipping already parsed function bodies

Modified:
cfe/trunk/test/Index/skip-parsed-bodies/compile_commands.json

Modified: cfe/trunk/test/Index/skip-parsed-bodies/compile_commands.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/skip-parsed-bodies/compile_commands.json?rev=369822=369821=369822=diff
==
--- cfe/trunk/test/Index/skip-parsed-bodies/compile_commands.json (original)
+++ cfe/trunk/test/Index/skip-parsed-bodies/compile_commands.json Fri Aug 23 
15:51:23 2019
@@ -12,7 +12,7 @@
 {
   "directory": ".",
   "command": "/usr/bin/clang++ -fsyntax-only -fno-ms-compatibility 
-fno-delayed-template-parsing t3.cpp -DBLAH",
-  "file": "t2.cpp"
+  "file": "t3.cpp"
 }
 ]
 


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

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[James Nagurne via Phabricator via cfe-commits]

JamesNagurne added a comment.

In D65907#1643650 , @arphaman wrote:

> No the windows test failure was different, there were no Deps at all. I'm 
> currently investigating it on a windows VM.
>
> @JamesNagurne I think there's some issue with the working directory, which is 
> not added in your case. Which platform are you running your build/test on? 
> Which cmake options are you using?


I apologize for not giving such information in the first reply. Unfortunately 
this isn't an easy remote reproduction, as our ToolChain and some integral 
changes aren't upstreamed. This is an embedded ARM cross-compiled on Linux. 
Might be able to reproduce with arm-none-none-eabi.
LLVM is built as an external project. Looking at the build system, it looks 
like we have the CMAKE_ARGS:

  -DLLVM_DEFAULT_TARGET_TRIPLE=arm-ti-none-eabi
  -DLLVM_EXTERNAL_CLANG_SOURCE_DIR=${CMAKE_SOURCE_DIR}/llvm-project/clang
  -DLLVM_TARGETS_TO_BUILD=ARM
  -DCMAKE_BUILD_TYPE=Release
  -DCMAKE_CXX_COMPILER=clang++
  -DCMAKE_C_COMPILER=clang
  -DLLVM_USE_LINKER=gold
  -GNinja

Nothing suspicious, except maybe the default triple and ARM target.
Looking at our (not upstream) toolchain file, we do have some RTLibs, 
LibInternal, libcxx, and System include changes, along with a -nostdsysteminc 
to avoid pulling host includes into our cross compiler. However, none of this 
should affect general "#include" behavior, correct?
Another glance at your changes don't seem to affect any target-specific 
handling either, at least directly.

I might have to just bite the bullet and drop into the test with a debugger.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65907



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

[PATCH] D66699: [PowerPC][Altivec] Fix constant argument for vec_dss

[Jinsong Ji via Phabricator via cfe-commits]

jsji created this revision.
jsji added reviewers: nemanjai, hfinkel.
Herald added subscribers: cfe-commits, shchenz, MaskRay, kbarton.
Herald added a project: clang.
jsji added a reviewer: PowerPC.
Herald added a subscriber: wuzish.

This is similar to vec_ct* in https://reviews.llvm.org/rL304205.

The argument must be a constant, otherwise instruction selection
will fail. always_inline is not enough for isel to always fold
everything away at -O0.

The fix is to turn the function into macros in altivec.h.

Fixes https://bugs.llvm.org/show_bug.cgi?id=43072


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66699

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/altivec-dss.c
  clang/test/CodeGen/builtins-ppc-error.c


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument 
to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 
5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
+
+}
Index: clang/test/CodeGen/altivec-dss.c
===
--- /dev/null
+++ clang/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature 
+altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -3286,9 +3286,7 @@
 
 /* vec_dss */
 
-static __inline__ void __attribute__((__always_inline__)) vec_dss(int __a) {
-  __builtin_altivec_dss(__a);
-}
+#define vec_dss __builtin_altivec_dss
 
 /* vec_dssall */
 
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -55,7 +55,7 @@
 BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "")
 BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "")
 
-BUILTIN(__builtin_altivec_dss, "vUi", "")
+BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
 BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
 BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must be a constant integer}}
+
+}
Index: clang/test/CodeGen/altivec-dss.c
===
--- /dev/null
+++ clang/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature +altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -3286,9 +3286,7 @@
 
 /* vec_dss */
 
-static __inline__ void __attribute__((__always_inline__)) vec_dss(int __a) {
-  __builtin_altivec_dss(__a);
-}
+#define vec_dss __builtin_altivec_dss
 
 /* vec_dssall */
 
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -55,7 +55,7 @@
 BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "")
 BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "")
 
-BUILTIN(__builtin_altivec_dss, "vUi", "")
+BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
 BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
 BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

owenpan wrote:
> Maybe set `SpaceBeforeParens` to `Always` first in order to really test the 
> new behavior?
I meant setting `SpaceBeforeParens` to ~~`Always`~~ `Never`. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D2



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christian Venegas via Phabricator via cfe-commits]

cvenegas added a comment.

I'm testing this patch on our codebase and it is working pretty well. We use 
the Allman style and the lambda problem has been an issue for many years. One 
thing to note in this patch is that some of the files have CRLF line endings 
but should be LF endings, which is why they're showing so many edits. I'm also 
seeing a clang tidy test failing with this patch. The 
readability-braces-around-statements tests seem to fail because the indent 
width is appending double of what it should.
void test() {

 if (cond("if0") /*comment*/) {
do_something("same-line");
  }

Hope this helps as this patch is the only reason why we still need to build 
clang-format instead of using the prebuilt binaries. Thanks!


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

https://reviews.llvm.org/D44609



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

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 217009.
logan-5 added a comment.

Add a missing null check.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66621

Files:
  clang/lib/AST/DeclCXX.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp


Index: clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
===
--- clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
+++ clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
@@ -24,6 +24,20 @@
   }
 }
 
+namespace Test2a {
+  struct A {
+virtual ~A() final {}
+virtual int f();
+  };
+
+  // CHECK-LABEL: define i32 @_ZN6Test2a1fEPNS_1AE
+  int f(A *a) {
+// CHECK: call i32 @_ZN6Test2a1A1fEv
+return a->f();
+  }
+}
+
+
 namespace Test3 {
   struct A {
 virtual int f();
Index: clang/lib/AST/DeclCXX.cpp
===
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -2067,10 +2067,15 @@
   if (DevirtualizedMethod->hasAttr())
 return DevirtualizedMethod;
 
-  // Similarly, if the class itself is marked 'final' it can't be overridden
-  // and we can therefore devirtualize the member function call.
+  // Similarly, if the class itself or its destructor is marked 'final',
+  // the class can't be derived from and we can therefore devirtualize the 
+  // member function call.
   if (BestDynamicDecl->hasAttr())
 return DevirtualizedMethod;
+  if (const auto *dtor = BestDynamicDecl->getDestructor()) {
+if (dtor->hasAttr())
+  return DevirtualizedMethod;
+  }
 
   if (const auto *DRE = dyn_cast(Base)) {
 if (const auto *VD = dyn_cast(DRE->getDecl()))


Index: clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
===
--- clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
+++ clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
@@ -24,6 +24,20 @@
   }
 }
 
+namespace Test2a {
+  struct A {
+virtual ~A() final {}
+virtual int f();
+  };
+
+  // CHECK-LABEL: define i32 @_ZN6Test2a1fEPNS_1AE
+  int f(A *a) {
+// CHECK: call i32 @_ZN6Test2a1A1fEv
+return a->f();
+  }
+}
+
+
 namespace Test3 {
   struct A {
 virtual int f();
Index: clang/lib/AST/DeclCXX.cpp
===
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -2067,10 +2067,15 @@
   if (DevirtualizedMethod->hasAttr())
 return DevirtualizedMethod;
 
-  // Similarly, if the class itself is marked 'final' it can't be overridden
-  // and we can therefore devirtualize the member function call.
+  // Similarly, if the class itself or its destructor is marked 'final',
+  // the class can't be derived from and we can therefore devirtualize the 
+  // member function call.
   if (BestDynamicDecl->hasAttr())
 return DevirtualizedMethod;
+  if (const auto *dtor = BestDynamicDecl->getDestructor()) {
+if (dtor->hasAttr())
+  return DevirtualizedMethod;
+  }
 
   if (const auto *DRE = dyn_cast(Base)) {
 if (const auto *VD = dyn_cast(DRE->getDecl()))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Looks about right




Comment at: clang/lib/Sema/SemaChecking.cpp:8105-8106
+if (ImplicitMatch == analyze_printf::ArgType::Match)
   return true;
+else if (ImplicitMatch == analyze_printf::ArgType::NoMatchPedantic)
+  Pedantic = true;

No else after return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66186



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

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Hi @nathanchance , I could not reproduce your failure using the same revision 
and `build-llvm` script. Could you please attach your json report located at 
`/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

Re: r369641 - [OpenCL] Fix declaration of enqueue_marker

[Hans Wennborg via cfe-commits]

Sure. Merged to release_90 in r369738.

On Thu, Aug 22, 2019 at 5:39 PM Anastasia Stulova
 wrote:
>
> Hi Hans,
>
>
> Can this still be merged into the release branch please.
>
>
> Thanks in advance,
>
> Anastasia
>
>
>
> 
> From: cfe-commits  on behalf of Yaxun Liu 
> via cfe-commits 
> Sent: 22 August 2019 12:18
> To: cfe-commits@lists.llvm.org 
> Subject: r369641 - [OpenCL] Fix declaration of enqueue_marker
>
> Author: yaxunl
> Date: Thu Aug 22 04:18:59 2019
> New Revision: 369641
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369641=rev
> Log:
> [OpenCL] Fix declaration of enqueue_marker
>
> Differential Revision: https://reviews.llvm.org/D66512
>
> Modified:
> cfe/trunk/lib/Headers/opencl-c.h
>
> Modified: cfe/trunk/lib/Headers/opencl-c.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c.h?rev=369641=369640=369641=diff
> ==
> --- cfe/trunk/lib/Headers/opencl-c.h (original)
> +++ cfe/trunk/lib/Headers/opencl-c.h Thu Aug 22 04:18:59 2019
> @@ -15350,7 +15350,7 @@ ndrange_t __ovld ndrange_3D(const size_t
>  ndrange_t __ovld ndrange_3D(const size_t[3], const size_t[3]);
>  ndrange_t __ovld ndrange_3D(const size_t[3], const size_t[3], const 
> size_t[3]);
>
> -int __ovld enqueue_marker(queue_t, uint, const __private clk_event_t*, 
> __private clk_event_t*);
> +int __ovld enqueue_marker(queue_t, uint, const clk_event_t*, clk_event_t*);
>
>  void __ovld retain_event(clk_event_t);
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D66593#1642253 , @Charusso wrote:

> Wow, it is great you could address every of the edge cases. Thanks you so 
> much! I believe only one problem left: we need to `return false` instead of 
> plain `return` in order to let the Analyzer model the function call. I could 
> fix it easily, thanks!


Well, not really. I have tested out and there is no difference. Thanks for the 
fixes!


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

https://reviews.llvm.org/D66593



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

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

In D66538#1641310 , @martong wrote:

> > It looks like that the original code is correct in the decision of 
> > structural equivalence of the original pair. If we have an (A,B) and (A,C) 
> > to compare, B and C are in different decl chain, then (A,B) or (A,C) will 
> > be non-equivalent (unless B and C are equivalent, but what to do in this 
> > case?). The problem was that the code assumed that in this case always A 
> > and B (or A and C) are non-equivalent. If NonEquivalentDecls is not filled 
> > in this case (or not used at all) the problem does not exist.
>
> I think we must not update the NonEquivalentDecls cache at that level, 
> because as we've seen (during the discussion of 
> https://reviews.llvm.org/D62131) it may store the wrong pairs.
>  However, it is still okay to cache the inequivalent decls in one level 
> upper, right after the `Finish` returns.
>  See this diff 
> https://github.com/martong/clang/compare/NonEqDecls_cache_in_an_upper_level~...martong:NonEqDecls_cache_in_an_upper_level?expand=1
>  Right now I started a build on our CI to see if it causes any slowdown.


This is correct, `NonEquivalentDecls` can be filled after `Finish` (in 
`IsEquivalent`?) (still we can have similar cache for equivalence too, maybe 
this is not of so much use). With the new code there is the possibility to get 
more information about non-equivalence, the `NonEquivalentDecls` can be filled 
in `Finish` too like it is done now, and with not much effort (I think) we can 
add some dependency relation (a tree structure) to decide which (already 
visited) Decls become non-equivalent if one non-equivalence is found. If there 
is a `void f(A *, B *)` to check the `f` can be a "parent" of `A` and `B`, if 
`A` or `B` is found to be non-equivalent we can set this for `f` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66538



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

[PATCH] D66631: [clang-tidy] Don't emit google-runtime-references warning for functions defined in macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added reviewers: gribozavr, alexfh.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

The macro are usually defined in the common/base headers which are hard
for normal users to modify it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66631

Files:
  clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
  clang-tools-extra/test/clang-tidy/google-runtime-references.cpp


Index: clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 


Index: clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66631: [clang-tidy] Don't emit google-runtime-references warning for functions defined in macros.

[Dmitri Gribenko via Phabricator via cfe-commits]

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

LGTM, although I'd be more comfortable with a whitelist of macros.




Comment at: clang-tools-extra/test/clang-tidy/google-runtime-references.cpp:156
+DEFINE_F(func) {}
\ No newline at end of file


Please add back the newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66631



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

[PATCH] D66632: [clangd] Link more clang-tidy modules to clangd

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov, 
mgorny, srhines.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

There are two new clang-tidy modules being added recently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66632

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdUnit.cpp


Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -704,22 +704,25 @@
   extern volatile int X##ModuleAnchorSource;   
\
   static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =
\
   X##ModuleAnchorSource
-LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Android);
 LINK_TIDY_MODULE(Boost);
 LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(CppCoreGuidelines);
 LINK_TIDY_MODULE(Fuchsia);
 LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(LinuxKernel);
+LINK_TIDY_MODULE(LLVM);
 LINK_TIDY_MODULE(Misc);
 LINK_TIDY_MODULE(Modernize);
+// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(OpenMP);
 LINK_TIDY_MODULE(Performance);
 LINK_TIDY_MODULE(Portability);
 LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(HICPP);
 LINK_TIDY_MODULE(Zircon);
 #undef LINK_TIDY_MODULE
 } // namespace tidy
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -122,10 +122,12 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule


Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -704,22 +704,25 @@
   extern volatile int X##ModuleAnchorSource;   \
   static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =\
   X##ModuleAnchorSource
-LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Android);
 LINK_TIDY_MODULE(Boost);
 LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(CppCoreGuidelines);
 LINK_TIDY_MODULE(Fuchsia);
 LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(LinuxKernel);
+LINK_TIDY_MODULE(LLVM);
 LINK_TIDY_MODULE(Misc);
 LINK_TIDY_MODULE(Modernize);
+// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(OpenMP);
 LINK_TIDY_MODULE(Performance);
 LINK_TIDY_MODULE(Portability);
 LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(HICPP);
 LINK_TIDY_MODULE(Zircon);
 #undef LINK_TIDY_MODULE
 } // namespace tidy
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -122,10 +122,12 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66631: [clang-tidy] Don't emit google-runtime-references warning for functions defined in macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 216777.
hokein marked an inline comment as done.
hokein added a comment.

Add the missing back newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66631

Files:
  clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
  clang-tools-extra/test/clang-tidy/google-runtime-references.cpp


Index: clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}
Index: clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 


Index: clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}
Index: clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r369739 - [clang-tidy] Don't emit google-runtime-references warning for functions defined in macros.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Aug 23 01:47:27 2019
New Revision: 369739

URL: http://llvm.org/viewvc/llvm-project?rev=369739=rev
Log:
[clang-tidy] Don't emit google-runtime-references warning for functions defined 
in macros.

Summary:
The macro are usually defined in the common/base headers which are hard
for normal users to modify it.

Reviewers: gribozavr, alexfh

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp?rev=369739=369738=369739=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp Fri Aug 23 
01:47:27 2019
@@ -52,6 +52,9 @@ void NonConstReferences::check(const Mat
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 

Modified: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp?rev=369739=369738=369739=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp Fri 
Aug 23 01:47:27 2019
@@ -149,3 +149,7 @@ void f8(B &);
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}


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

[PATCH] D66631: [clang-tidy] Don't emit google-runtime-references warning for functions defined in macros.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369739: [clang-tidy] Dont emit 
google-runtime-references warning for functions defined… (authored by hokein, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66631?vs=216777=216778#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66631

Files:
  clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
  clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp


Index: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 
Index: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}


Index: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
@@ -52,6 +52,9 @@
   if (Function == nullptr || Function->isImplicit())
 return;
 
+  if (Function->getLocation().isMacroID())
+return;
+
   if (!Function->isCanonicalDecl())
 return;
 
Index: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
@@ -149,3 +149,7 @@
 }
 void f9(whitelist::A &);
 void f10(whitelist::B &);
+
+#define DEFINE_F(name) void name(int& a)
+
+DEFINE_F(func) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The link for the diff went off, sorry about that.
Here is the new link which is going to work:
https://github.com/martong/clang/compare/NonEqDecls_cache_in_an_upper_level_0...martong:NonEqDecls_cache_in_an_upper_level?expand=1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66538



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

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

In D36562#1641930 , @wmi wrote:

> In D36562#1639441 , @chill wrote:
>
> > Shouldn't we disable `OPT_ffine_grained_bitfield_accesses` only if TSAN is 
> > active?
>
>
> I don't remember why it is disabled for all sanitizer modes. Seems you are 
> right that the disabling the option is only necessary for TSAN. Do you have 
> actual needs for the option to be functioning on other sanitizer modes?


Well, yes and no. We have the option enabled by default and it causes a warning 
when we use it together with `-fsanitize=memtag` (we aren't really concerned 
with other sanitizers). That warning broke a few builds (e.g. CMake doing tests 
and not wanting to see *any* diagnostics. We can work around that in a number 
of ways, e.g. we can leave the default off for AArch64.

I'd prefer though to have an upstream solution, if that's considered beneficial 
for all LLVM users and this one seems like such a case: let anyone use the 
option with sanitizers, unless it's known that some sanitizers'utility is 
affected negatively (as with TSAN).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D36562



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

[clang-tools-extra] r369741 - [clangd] Link more clang-tidy modules to clangd

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Aug 23 02:13:23 2019
New Revision: 369741

URL: http://llvm.org/viewvc/llvm-project?rev=369741=rev
Log:
[clangd] Link more clang-tidy modules to clangd

Summary: There are two new clang-tidy modules being added recently.

Reviewers: sammccall, jdoerfert

Subscribers: srhines, mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, 
kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdUnit.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=369741=369740=369741=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Fri Aug 23 02:13:23 2019
@@ -122,10 +122,12 @@ add_clang_library(clangDaemon
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=369741=369740=369741=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Aug 23 02:13:23 2019
@@ -704,22 +704,25 @@ namespace tidy {
   extern volatile int X##ModuleAnchorSource;   
\
   static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =
\
   X##ModuleAnchorSource
-LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Android);
 LINK_TIDY_MODULE(Boost);
 LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(CppCoreGuidelines);
 LINK_TIDY_MODULE(Fuchsia);
 LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(LinuxKernel);
+LINK_TIDY_MODULE(LLVM);
 LINK_TIDY_MODULE(Misc);
 LINK_TIDY_MODULE(Modernize);
+// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(OpenMP);
 LINK_TIDY_MODULE(Performance);
 LINK_TIDY_MODULE(Portability);
 LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(HICPP);
 LINK_TIDY_MODULE(Zircon);
 #undef LINK_TIDY_MODULE
 } // namespace tidy


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

[PATCH] D66632: [clangd] Link more clang-tidy modules to clangd

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369741: [clangd] Link more clang-tidy modules to clangd 
(authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66632?vs=216776=216785#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66632

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp


Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -704,22 +704,25 @@
   extern volatile int X##ModuleAnchorSource;   
\
   static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =
\
   X##ModuleAnchorSource
-LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Android);
 LINK_TIDY_MODULE(Boost);
 LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(CppCoreGuidelines);
 LINK_TIDY_MODULE(Fuchsia);
 LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(LinuxKernel);
+LINK_TIDY_MODULE(LLVM);
 LINK_TIDY_MODULE(Misc);
 LINK_TIDY_MODULE(Modernize);
+// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(OpenMP);
 LINK_TIDY_MODULE(Performance);
 LINK_TIDY_MODULE(Portability);
 LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(HICPP);
 LINK_TIDY_MODULE(Zircon);
 #undef LINK_TIDY_MODULE
 } // namespace tidy
Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -122,10 +122,12 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule


Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -704,22 +704,25 @@
   extern volatile int X##ModuleAnchorSource;   \
   static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =\
   X##ModuleAnchorSource
-LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Android);
 LINK_TIDY_MODULE(Boost);
 LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CERT);
 LINK_TIDY_MODULE(CppCoreGuidelines);
 LINK_TIDY_MODULE(Fuchsia);
 LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(LinuxKernel);
+LINK_TIDY_MODULE(LLVM);
 LINK_TIDY_MODULE(Misc);
 LINK_TIDY_MODULE(Modernize);
+// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(OpenMP);
 LINK_TIDY_MODULE(Performance);
 LINK_TIDY_MODULE(Portability);
 LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(HICPP);
 LINK_TIDY_MODULE(Zircon);
 #undef LINK_TIDY_MODULE
 } // namespace tidy
Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -122,10 +122,12 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> more closely match the sort order

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D66505#1641776 , @aaron.ballman 
wrote:

> Given that the alphabetization we want really is based on the string literal, 
> would it make sense to look for that rather than the check name? Adding a few 
> more reviewers for a better mix of opinions.


Ideally, it would be nice to sort the list of registrations by the check name. 
But given that the call can span two lines, that may be slightly trickier to 
implement. It's worth giving it a try though ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

The structural equivalency check works (with this patch) the following way: To 
check a `(A1,A2)` pair we collect further `(X1,X2)` pairs that should be 
checked and put these in `DeclsToCheck` (if not already there). The check 
succeeds if every pair is equivalent. The pairs `(A1,X2)` and `(A1,Y2)` are 
checked independently. A `true` return value from any of the 
`IsStructurallyEquivalent` functions means that we did not found 
non-equivalence in the internal node data but the pairs in `DeclsToCheck` 
should be checked too. Return value of `false` means a non-equivalence in the 
internal data only is found with the `(X1,X2)` pair that is currently checked, 
therefore we found non-equivalence for the original pair and for `(X1,X2)` too. 
Again, with the new code the test `CheckCommonEquivalence(D1, D2) && 
CheckKindSpecificEquivalence(D1, D2)` fails only if we found difference in the 
internal node data and we can be sure that `D1` and `D2` are non-equivalent. 
The old code does not have this property.

With the old code (that uses `TentativeEquivalences`) return value `false` for 
`IsStructurallyEquivalent` for `(X1,X2)` pair to check can mean that we found a 
`(X1,Y2)` to check and `X2!=Y2` (decl chain different). But we can not say that 
`(X1,X2)` are non-equivalent because it may be that `(X1,Y2)` are 
non-equivalent and `(X1,X2)` are equivalent. (But it is sure that the original 
pair `(A1,A2)` is non-equivalent.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66538



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

First patch for propogating multifile changes from tweak outputs to LSP
WorkspaceEdits.

Uses FS to convert tooling::Replacements to TextEdits except the MainFile.
Errors out if there are any inconsistencies between the draft version and the
version generated the edits.

We exempt MainFile from this check to not regress existing use cases, since
clangd can refactor the mainfile even if there are unsaved changes in the
editor. But for other files it will make use of on-disk state rather than
on-editor state.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66637

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -113,11 +113,15 @@
   auto Effect = apply(ID, Input);
   if (!Effect)
 return Effect.takeError();
-  if (!Effect->ApplyEdit)
+  if (!Effect->ApplyEdits)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No replacements");
+  auto Edits = *Effect->ApplyEdits;
+  if (Edits.size() > 1)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Multi-file edits");
   Annotations Code(Input);
-  return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+  return applyAllReplacements(Code.code(), Edits.begin()->second.Reps);
 }
 
 void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -9,8 +9,8 @@
 #include "TweakTesting.h"
 
 #include "Annotations.h"
-#include "refactor/Tweak.h"
 #include "SourceCode.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
 
@@ -98,9 +98,12 @@
 return "fail: " + llvm::toString(Result.takeError());
   if (Result->ShowMessage)
 return "message:\n" + *Result->ShowMessage;
-  if (Result->ApplyEdit) {
+  if (Result->ApplyEdits) {
+if (Result->ApplyEdits->size() > 1)
+  return "received multi-file edits";
+auto ApplyEdit = Result->ApplyEdits->begin()->second;
 if (auto NewText =
-tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
+tooling::applyAllReplacements(Input.code(), ApplyEdit.Reps))
   return unwrap(Context, *NewText);
 else
   return "bad edits: " + llvm::toString(NewText.takeError());
Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -90,7 +90,8 @@
  ElseRng->getBegin(),
  ElseCode.size(), ThenCode)))
 return std::move(Err);
-  return Effect::applyEdit(Result);
+  return Effect::applyEdit(SrcMgr.getFilename(Inputs.Cursor),
+   Edit::generateEdit(Inputs.Code, std::move(Result)));
 }
 
 } // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
@@ -88,10 +88,13 @@
 }
 
 Expected RawStringLiteral::apply(const Selection ) {
-  return Effect::applyEdit(tooling::Replacements(
+  auto  = Inputs.AST.getSourceManager();
+  auto Reps = tooling::Replacements(
   tooling::Replacement(Inputs.AST.getSourceManager(), Str,
("R\"(" + Str->getBytes() + ")\"").str(),
-   Inputs.AST.getASTContext().getLangOpts(;
+   Inputs.AST.getASTContext().getLangOpts()));
+  return 

[PATCH] D66588: [ARM NEON] Avoid duplicated decarations

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio updated this revision to Diff 216801.
dnsampaio added a comment.

- Consider BigEndianSafe intrinsics that all inputs and outputs are scalar or 
single element vectors


Repository:
  rC Clang

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

https://reviews.llvm.org/D66588

Files:
  utils/TableGen/NeonEmitter.cpp


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -332,6 +332,17 @@
   NeonEmitter 
   std::stringstream OS;
 
+  bool isBigEndianSafe() const {
+if (BigEndianSafe)
+  return true;
+
+for (const auto  : Types){
+  if (T.isVector() && T.getNumElements() > 1)
+return false;
+}
+return true;
+  }
+
 public:
   Intrinsic(Record *R, StringRef Name, StringRef Proto, TypeSpec OutTS,
 TypeSpec InTS, ClassKind CK, ListInit *Body, NeonEmitter ,
@@ -1293,7 +1304,7 @@
 }
 
 void Intrinsic::emitArgumentReversal() {
-  if (BigEndianSafe)
+  if (isBigEndianSafe())
 return;
 
   // Reverse all vector arguments.
@@ -1314,7 +1325,7 @@
 }
 
 void Intrinsic::emitReturnReversal() {
-  if (BigEndianSafe)
+  if (isBigEndianSafe())
 return;
   if (!getReturnType().isVector() || getReturnType().isVoid() ||
   getReturnType().getNumElements() == 1)
@@ -1889,6 +1900,11 @@
 }
 
 std::string Intrinsic::generate() {
+  // Avoid duplicated code for big and small endians
+  if (isBigEndianSafe()) {
+generateImpl(false, "", "");
+return OS.str();
+  }
   // Little endian intrinsics are simple and don't require any argument
   // swapping.
   OS << "#ifdef __LITTLE_ENDIAN__\n";


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -332,6 +332,17 @@
   NeonEmitter 
   std::stringstream OS;
 
+  bool isBigEndianSafe() const {
+if (BigEndianSafe)
+  return true;
+
+for (const auto  : Types){
+  if (T.isVector() && T.getNumElements() > 1)
+return false;
+}
+return true;
+  }
+
 public:
   Intrinsic(Record *R, StringRef Name, StringRef Proto, TypeSpec OutTS,
 TypeSpec InTS, ClassKind CK, ListInit *Body, NeonEmitter ,
@@ -1293,7 +1304,7 @@
 }
 
 void Intrinsic::emitArgumentReversal() {
-  if (BigEndianSafe)
+  if (isBigEndianSafe())
 return;
 
   // Reverse all vector arguments.
@@ -1314,7 +1325,7 @@
 }
 
 void Intrinsic::emitReturnReversal() {
-  if (BigEndianSafe)
+  if (isBigEndianSafe())
 return;
   if (!getReturnType().isVector() || getReturnType().isVoid() ||
   getReturnType().getNumElements() == 1)
@@ -1889,6 +1900,11 @@
 }
 
 std::string Intrinsic::generate() {
+  // Avoid duplicated code for big and small endians
+  if (isBigEndianSafe()) {
+generateImpl(false, "", "");
+return OS.str();
+  }
   // Little endian intrinsics are simple and don't require any argument
   // swapping.
   OS << "#ifdef __LITTLE_ENDIAN__\n";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

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

@shafik Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692



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

[PATCH] D65526: [Clangd] Initial prototype version of ExtractFunction

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 216805.
SureYeaah marked 4 inline comments as done.
SureYeaah added a comment.

Addressed more review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65526

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -599,6 +599,106 @@
 R"cpp(const char * x = "test")cpp");
 }
 
+TWEAK_TEST(ExtractFunction);
+TEST_F(ExtractFunctionTest, FunctionTest) {
+  Header = R"cpp(
+#define F(BODY) void FFF() { BODY }
+  )cpp";
+  Context = Function;
+
+  // Root statements should have common parent.
+  EXPECT_EQ(apply("for(;;) [[1+2; 1+2;]]"), "unavailable");
+  // Expressions aren't extracted.
+  EXPECT_EQ(apply("int x = 0; [[x++;]]"), "unavailable");
+  // We don't support extraction from lambdas.
+  EXPECT_EQ(apply("auto lam = [](){ [[int x;]] }; "), "unavailable");
+  // FIXME: Add tests for macros after selectionTree works properly for macros.
+  // FIXME: This should be extractable
+  EXPECT_EQ(apply("F ([[int x = 0;]])"), "unavailable");
+
+  // Ensure that end of Zone and Beginning of PostZone being adjacent doesn't
+  // lead to break being included in the extraction zone.
+  EXPECT_THAT(apply("for(;;) { [[{}]]break; }"), HasSubstr("extracted"));
+  // FIXME: This should be unavailable since partially selected but
+  // selectionTree doesn't always work correctly for VarDecls.
+  EXPECT_THAT(apply("int [[x = 0]];"), HasSubstr("extracted"));
+  // FIXME: ExtractFunction should be unavailable inside loop construct
+  // initalizer/condition.
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
+  // Don't extract because needs hoisting.
+  EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
+  // Don't extract return
+  EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
+  // Don't extract break and continue.
+  // FIXME: We should be able to extract this since it's non broken.
+  EXPECT_THAT(apply(" [[for(;;) break;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" for(;;) [[continue;]] "), StartsWith("fail"));
+}
+
+TEST_F(ExtractFunctionTest, FileTest) {
+  // Check all parameters are in order
+  std::string ParameterCheckInput = R"cpp(
+struct FOO {
+  int x;
+};
+void f(int a) {
+  int b; int *ptr =  FOO foo;
+  [[a += foo.x + b;
+  *ptr++;]]
+})cpp";
+  std::string ParameterCheckOutput = R"cpp(
+struct FOO {
+  int x;
+};
+void extracted(int , int , int * , struct FOO ) {
+a += foo.x + b;
+  *ptr++;
+}
+void f(int a) {
+  int b; int *ptr =  FOO foo;
+  extracted(a, b, ptr, foo);
+})cpp";
+  EXPECT_EQ(apply(ParameterCheckInput), ParameterCheckOutput);
+
+  // Check const qualifier
+  std::string ConstCheckInput = R"cpp(
+void f(const int c) {
+  [[while(c) {}]]
+})cpp";
+  std::string ConstCheckOutput = R"cpp(
+void extracted(const int ) {
+while(c) {}
+}
+void f(const int c) {
+  extracted(c);
+})cpp";
+  EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
+
+  // Don't extract when we need to make a function as a parameter.
+  EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
+
+  // We don't extract from methods for now since they may involve multi-file
+  // edits
+  std::string MethodFailInput = R"cpp(
+class T {
+  void f() {
+[[int x;]]
+  }
+};
+  )cpp";
+  EXPECT_EQ(apply(MethodFailInput), "unavailable");
+
+  // We don't extract from templated functions for now as templates are hard
+  // to deal with.
+  std::string TemplateFailInput = R"cpp(
+template
+void f() {
+  [[int x;]]
+}
+  )cpp";
+  EXPECT_EQ(apply(TemplateFailInput), "unavailable");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -0,0 +1,606 @@
+//===--- ExtractFunction.cpp -*- C++-*-===//
+//
+// 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
+//
+//===--===//
+//
+// Extracts statements to a new function and replaces the statements with a
+// call to the new function.
+// Before:
+//   void f(int a) {
+// [[if(a < 5)
+//   a = 5;]]
+//   }
+// After:
+//   void extracted(int ) {
+// if(a < 5)
+//   a = 5;
+//   }
+//   void 

r369749 - [Docs][OpenCL] Several corrections to C++ for OpenCL

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Fri Aug 23 04:43:49 2019
New Revision: 369749

URL: http://llvm.org/viewvc/llvm-project?rev=369749=rev
Log:
[Docs][OpenCL] Several corrections to C++ for OpenCL

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


Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=369749=369748=369749=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Fri Aug 23 04:43:49 2019
@@ -1640,8 +1640,8 @@ OpenCL Features
 C++ for OpenCL
 --
 
-This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
-features can be used in OpenCL kernel code. All functionality from OpenCL C
+This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
+regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
 is inherited. This section describes minor differences to OpenCL C and any
 limitations related to C++ support as well as interactions between OpenCL and
 C++ features that are not documented elsewhere.
@@ -1652,6 +1652,7 @@ Restrictions to C++17
 The following features are not supported:
 
 - Virtual functions
+- Exceptions
 - ``dynamic_cast`` operator
 - Non-placement ``new``/``delete`` operators
 - Standard C++ libraries. Currently there is no solution for alternative C++
@@ -1667,20 +1668,24 @@ Address space behavior
 Address spaces are part of the type qualifiers; many rules are just inherited
 from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
 extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally yet, Clang extends existing concept
+behavior in C++ is not documented formally, Clang extends the existing concept
 from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using sets and overlapping from
-Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
-implicit conversions are allowed from named to ``__generic`` but not vice versa
-(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
-rules are built on top of this behavior.
+conversion but the compatibility is determined using notation of sets and
+overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
+s3.1.3). For OpenCL it means that implicit conversions are allowed from
+a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
+v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
+address space does not overlap with any other and therefore no valid conversion
+between ``__constant`` and other address spaces exists. Most of the rules
+follow this logic.
 
 **Casts**
 
-C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
-permit implicit conversion to ``__generic``. However converting from named
-address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
-that conversions between ``__constant`` and any other is still disallowed.
+C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
+permit conversion to ``__generic`` implicitly. However converting from
+``__generic`` to named address spaces can only be done using 
``addrspace_cast``.
+Note that conversions between ``__constant`` and any other address space
+are disallowed.
 
 .. _opencl_cpp_addrsp_deduction:
 
@@ -1693,7 +1698,7 @@ Address spaces are not deduced for:
 - non-pointer/non-reference class members except for static data members that 
are
   deduced to ``__global`` address space.
 - non-pointer/non-reference alias declarations.
-- ``decltype`` expression.
+- ``decltype`` expressions.
 
 .. code-block:: c++
 
@@ -1722,7 +1727,7 @@ TODO: Add example for type alias and dec
 
 **References**
 
-References types can be qualified with an address space.
+Reference types can be qualified with an address space.
 
 .. code-block:: c++
 
@@ -1737,29 +1742,29 @@ rules from address space pointer convers
 **Default address space**
 
 All non-static member functions take an implicit object parameter ``this`` that
-is a pointer type. By default this pointer parameter is in ``__generic`` 
address
-space. All concrete objects passed as an argument to ``this`` parameter will be
-converted to ``__generic`` address space first if the conversion is valid.
-Therefore programs using objects in ``__constant`` address space won't be 
compiled
-unless address space is explicitly specified using address space qualifiers on
-member functions
+is a pointer type. By default this pointer parameter is in the ``__generic``
+address space. All concrete objects passed as an argument to ``this`` parameter
+will be converted to the 

[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369749: [Docs][OpenCL] Several corrections to C++ for OpenCL 
(authored by stulova, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64418?vs=215922=216808#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64418

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/docs/UsersManual.rst

Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1640,8 +1640,8 @@
 C++ for OpenCL
 --
 
-This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
-features can be used in OpenCL kernel code. All functionality from OpenCL C
+This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
+regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
 is inherited. This section describes minor differences to OpenCL C and any
 limitations related to C++ support as well as interactions between OpenCL and
 C++ features that are not documented elsewhere.
@@ -1652,6 +1652,7 @@
 The following features are not supported:
 
 - Virtual functions
+- Exceptions
 - ``dynamic_cast`` operator
 - Non-placement ``new``/``delete`` operators
 - Standard C++ libraries. Currently there is no solution for alternative C++
@@ -1667,20 +1668,24 @@
 Address spaces are part of the type qualifiers; many rules are just inherited
 from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
 extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally yet, Clang extends existing concept
+behavior in C++ is not documented formally, Clang extends the existing concept
 from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using sets and overlapping from
-Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
-implicit conversions are allowed from named to ``__generic`` but not vice versa
-(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
-rules are built on top of this behavior.
+conversion but the compatibility is determined using notation of sets and
+overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
+s3.1.3). For OpenCL it means that implicit conversions are allowed from
+a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
+v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
+address space does not overlap with any other and therefore no valid conversion
+between ``__constant`` and other address spaces exists. Most of the rules
+follow this logic.
 
 **Casts**
 
-C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
-permit implicit conversion to ``__generic``. However converting from named
-address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
-that conversions between ``__constant`` and any other is still disallowed.
+C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
+permit conversion to ``__generic`` implicitly. However converting from
+``__generic`` to named address spaces can only be done using ``addrspace_cast``.
+Note that conversions between ``__constant`` and any other address space
+are disallowed.
 
 .. _opencl_cpp_addrsp_deduction:
 
@@ -1693,7 +1698,7 @@
 - non-pointer/non-reference class members except for static data members that are
   deduced to ``__global`` address space.
 - non-pointer/non-reference alias declarations.
-- ``decltype`` expression.
+- ``decltype`` expressions.
 
 .. code-block:: c++
 
@@ -1722,7 +1727,7 @@
 
 **References**
 
-References types can be qualified with an address space.
+Reference types can be qualified with an address space.
 
 .. code-block:: c++
 
@@ -1737,29 +1742,29 @@
 **Default address space**
 
 All non-static member functions take an implicit object parameter ``this`` that
-is a pointer type. By default this pointer parameter is in ``__generic`` address
-space. All concrete objects passed as an argument to ``this`` parameter will be
-converted to ``__generic`` address space first if the conversion is valid.
-Therefore programs using objects in ``__constant`` address space won't be compiled
-unless address space is explicitly specified using address space qualifiers on
-member functions
+is a pointer type. By default this pointer parameter is in the ``__generic``
+address space. All concrete objects passed as an argument to ``this`` parameter
+will be converted to the ``__generic`` address space first if such conversion is
+valid. Therefore programs using objects in the ``__constant`` address space will
+not be compiled unless the address space is explicitly 

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ok.

I like this patch because it eliminates the need for checking the redeclaration 
chains.

Seems like it handles cycles and the simple `f(A,B)` vs `f(A,A)` cases properly 
too. (Not talking about caching now, probably we must remove the 
`NonEquivalentDecls` cache.)
I've added two new test cases, could you please add them to the patch (maybe 
with modifications if you find something)?

  TEST_F(StructuralEquivalenceCacheTest, Cycle) {
auto Decls =
makeTuDecls(
R"(
  class C;
  class A { C *c; };
  class B {
int i;
  };
  void x(A *);
  void y(A *);
  class C {
friend void x(A *);
friend void y(A *);
  };
)",
R"(
  class C;
  class A { C *c; };
  class B {
int i;
  };
  void x(A *);
  void y(A *);
  class C {
friend void x(A *);
friend void y(A *);
  };
)", Lang_CXX);
  
TranslationUnitDecl *TU1 = get<0>(Decls);
TranslationUnitDecl *TU2 = get<1>(Decls);
auto *C1 = LastDeclMatcher().match(
TU1, cxxRecordDecl(hasName("C"), unless(isImplicit(;
auto *C2 = LastDeclMatcher().match(
TU2, cxxRecordDecl(hasName("C"), unless(isImplicit(;
  
llvm::DenseSet> NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
C1->getASTContext(), C2->getASTContext(), NonEquivalentDecls,
StructuralEquivalenceKind::Default, false, false);
bool Eq = Ctx.IsEquivalent(C1, C2);
  
EXPECT_TRUE(Eq);
  }
  
  TEST_F(StructuralEquivalenceCacheTest, SimpleNonEq) {
auto Decls =
makeTuDecls(
R"(
  class A {};
  class B {};
  void x(A, A);
)",
R"(
  class A {};
  class B {};
  void x(A, B);
)", Lang_CXX);
  
TranslationUnitDecl *TU1 = get<0>(Decls);
TranslationUnitDecl *TU2 = get<1>(Decls);
auto *x1 =
FirstDeclMatcher().match(TU1, functionDecl(hasName("x")));
auto *x2 =
FirstDeclMatcher().match(TU2, functionDecl(hasName("x")));
  
llvm::DenseSet> NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
x1->getASTContext(), x2->getASTContext(), NonEquivalentDecls,
StructuralEquivalenceKind::Default, false, false);
bool Eq = Ctx.IsEquivalent(x1, x2);
  
EXPECT_FALSE(Eq);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66538



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

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from the else after return nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66186



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

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D62571#1642096 , @domdom wrote:

> Thanks @aaron.ballman!
>
> I will need someone to commit this for me :)


I'm happy to commit for you, but I get merge conflicts when trying to apply 
your patch. Can you rebase on trunk?


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

https://reviews.llvm.org/D62571



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+  assert(It.second.Edits && "TextEdits hasn't been generated?");
+  if (auto Draft = DraftMgr.getDraft(It.first())) {
+llvm::StringRef Contents = *Draft;

This callback is called asynchronously and the version inside `DraftMgr` may be 
newer than the one we used to calculate the original offsets inside 
replacements at this point.

We should not rely on `DraftMgr`, doing conversions on the source buffers 
inside `SourceManager` is probably the most reliable option we have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D65433: [clangd] DefineInline action availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -8,6 +8,7 @@
 
 #include "Annotations.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "TweakTesting.h"
 #include "refactor/Tweak.h"
@@ -15,6 +16,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
@@ -22,6 +24,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 
 using llvm::Failed;
 using llvm::Succeeded;
@@ -47,20 +51,28 @@
   .str();
 }
 
-void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available) {
+struct TransformInputFile {
+  std::string InitialText;
+  std::string ModifiedText;
+};
+
+void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available,
+const llvm::StringMap ) {
   Annotations Code(Input);
   ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
   << "no points of interest specified";
   TestTU TU;
   TU.Filename = "foo.cpp";
   TU.Code = Code.code();
+  TU.AdditionalFiles = std::move(AdditionalFiles);
 
   ParsedAST AST = TU.build();
 
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+const Tweak::Selection Sel = Tweak::Selection(AST, Begin, End);
+auto T = prepareTweak(ID, Sel);
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
   << "code is " << markRange(Code.code(), Selection);
@@ -75,13 +87,18 @@
 }
 
 /// Checks action is available at every point and range marked in \p Input.
-void checkAvailable(StringRef ID, llvm::StringRef Input) {
-  return checkAvailable(ID, Input, /*Available=*/true);
+void checkAvailable(StringRef ID, llvm::StringRef Input,
+const llvm::StringMap  = {}) {
+  return checkAvailable(ID, Input, /*Available=*/true,
+std::move(AdditionalFiles));
 }
 
 /// Same as checkAvailable, but checks the action is not available.
-void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
-  return checkAvailable(ID, Input, /*Available=*/false);
+void checkNotAvailable(
+StringRef ID, llvm::StringRef Input,
+const llvm::StringMap  = {}) {
+  return checkAvailable(ID, Input, /*Available=*/false,
+std::move(AdditionalFiles));
 }
 
 llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
@@ -603,6 +620,111 @@
 R"cpp(const char * x = "test")cpp");
 }
 
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+  class Bar {
+void baz();
+  };
+
+  // Should it also trigger on NestedNameSpecifier? i.e "Bar::"
+  [[void [[Bar::[[b^a^z() [[{
+return;
+  }
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+return;
+  }
+  )cpp");
+
+  EXPECT_UNAVAILABLE(R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+[[(void)(5+3);
+return;]]
+  }]]
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, NoDecl) {
+  checkNotAvailable(TweakID, R"cpp(
+  #include "a.h"
+  void bar() {
+return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+return;
+  })cpp",
+{{"a.h", "void bar();"}});
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo(int test);
+
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  checkAvailable(TweakID,
+ R"cpp(
+#include "a.h"
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp",
+ {{"a.h", "void bar(); void foo(int test);"}});
+
+  // Internal symbol usage.
+  checkNotAvailable(TweakID,
+R"cpp(
+  #include "a.h"
+  void bar();
+  void fo^o(int baz) {
+int x = 10;
+bar();
+  

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

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

In D66186#1642607 , @aaron.ballman 
wrote:

> LGTM aside from the else after return nit.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66186



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+  assert(It.second.Edits && "TextEdits hasn't been generated?");
+  if (auto Draft = DraftMgr.getDraft(It.first())) {
+llvm::StringRef Contents = *Draft;

ilya-biryukov wrote:
> This callback is called asynchronously and the version inside `DraftMgr` may 
> be newer than the one we used to calculate the original offsets inside 
> replacements at this point.
> 
> We should not rely on `DraftMgr`, doing conversions on the source buffers 
> inside `SourceManager` is probably the most reliable option we have.
I think you've misunderstood what this code is doing (it needs comments!)

The SourceManager contains the code that we've calculated the edits against, by 
definition. So checking against it does nothing.

When we send the edits to the client, it's going to apply them to the file if 
it's not open, but to the dirty buffer if it is open.
If the dirty buffer has different contents than what we saw (and calculated 
edits against) then the edits will be wrong - that's what this code guards 
against.

So looking at the latest available content in the draftmgr (at the end of the 
request, not the start!) is the right thing here, I think.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:646
+llvm::StringRef Contents = *Draft;
+if (llvm::SHA1::hash(llvm::makeArrayRef(Contents.bytes_begin(),
+Contents.size())) !=

We're comparing here content in the source manager (which was read as bytes 
from disk, and converted to utf8) to that from LSP (which was sent as unicode 
text by the editor, and converted to utf8).

Are editors going to preserve the actual line endings from disk when sending 
LSP, or are they going to use the "array-of-lines" from their editor's native 
model? (e.g. join(lines, '\n'))

If the latter, we'd need to normalize line endings, possibly strip trailing 
line endings, etc.
I ask because clangd-vim definitely has an "array-of-lines" model :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+  assert(It.second.Edits && "TextEdits hasn't been generated?");
+  if (auto Draft = DraftMgr.getDraft(It.first())) {
+llvm::StringRef Contents = *Draft;

sammccall wrote:
> ilya-biryukov wrote:
> > This callback is called asynchronously and the version inside `DraftMgr` 
> > may be newer than the one we used to calculate the original offsets inside 
> > replacements at this point.
> > 
> > We should not rely on `DraftMgr`, doing conversions on the source buffers 
> > inside `SourceManager` is probably the most reliable option we have.
> I think you've misunderstood what this code is doing (it needs comments!)
> 
> The SourceManager contains the code that we've calculated the edits against, 
> by definition. So checking against it does nothing.
> 
> When we send the edits to the client, it's going to apply them to the file if 
> it's not open, but to the dirty buffer if it is open.
> If the dirty buffer has different contents than what we saw (and calculated 
> edits against) then the edits will be wrong - that's what this code guards 
> against.
> 
> So looking at the latest available content in the draftmgr (at the end of the 
> request, not the start!) is the right thing here, I think.
> This callback is called asynchronously and the version inside DraftMgr may be 
> newer than the one we used to calculate the original offsets inside 
> replacements at this point.

That's exactly what we are checking though?

We want to bail out if user has a "different" version of the source code in 
their editor by the time we generated edits. Since editors will most likely try 
to apply the edits onto the version in editor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.
Herald added a project: clang.
kadircet added a parent revision: D65433: [clangd] DefineInline action 
availability checks.

Initial version of DefineInline action that will fully qualify every
name inside function body. It has some problems while qualyfying dependent
template arguments, see FIXME in the unittests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66647

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -75,10 +76,12 @@
 auto T = prepareTweak(ID, Sel);
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
-  << "code is " << markRange(Code.code(), Selection);
+  << "code is " << markRange(Code.code(), Selection)
+  << Sel.ASTSelection;
 else
   EXPECT_THAT_EXPECTED(T, Failed())
-  << "code is " << markRange(Code.code(), Selection);
+  << "code is " << markRange(Code.code(), Selection)
+  << Sel.ASTSelection;
   };
   for (auto P : Code.points())
 CheckOver(Range{P, P});
@@ -101,7 +104,9 @@
 std::move(AdditionalFiles));
 }
 
-llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
+llvm::Expected
+apply(StringRef ID, llvm::StringRef Input,
+  const llvm::StringMap  = {}) {
   Annotations Code(Input);
   Range SelectionRng;
   if (Code.points().size() != 0) {
@@ -114,6 +119,9 @@
   TestTU TU;
   TU.Filename = "foo.cpp";
   TU.Code = Code.code();
+  for (auto  : AdditionalFiles) {
+TU.AdditionalFiles[InpFile.first()] = InpFile.second.InitialText;
+  }
 
   ParsedAST AST = TU.build();
   unsigned Begin = cantFail(positionToOffset(Code.code(), SelectionRng.start));
@@ -126,26 +134,44 @@
   return (*T)->apply(S);
 }
 
-llvm::Expected applyEdit(StringRef ID, llvm::StringRef Input) {
-  auto Effect = apply(ID, Input);
+llvm::Expected>
+applyEdit(StringRef ID, llvm::StringRef Input,
+  const llvm::StringMap  = {}) {
+  auto Effect = apply(ID, Input, AdditionalFiles);
   if (!Effect)
 return Effect.takeError();
   if (!Effect->ApplyEdits)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No replacements");
-  auto Edits = *Effect->ApplyEdits;
-  if (Edits.size() > 1)
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Multi-file edits");
-  Annotations Code(Input);
-  return applyAllReplacements(Code.code(), Edits.begin()->second.Reps);
+  llvm::StringMap Output;
+  for (const auto  : *Effect->ApplyEdits) {
+llvm::StringRef FilePath = It.first();
+FilePath.consume_front(testRoot());
+FilePath.consume_front("/");
+auto InpIt = AdditionalFiles.find(FilePath);
+if (InpIt == AdditionalFiles.end())
+  FilePath = "";
+Annotations Code(InpIt == AdditionalFiles.end()
+ ? Input
+ : llvm::StringRef(InpIt->second.InitialText));
+auto Replaced = applyAllReplacements(Code.code(), It.second.Reps);
+if (!Replaced)
+  return Replaced.takeError();
+Output[FilePath] = *Replaced;
+  }
+  return Output;
 }
 
 void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
-std::string Output) {
-  auto Result = applyEdit(ID, Input);
+std::string Output,
+llvm::StringMap AdditionalFiles = {}) {
+  auto Result = applyEdit(ID, Input, AdditionalFiles);
   ASSERT_TRUE(bool(Result)) << llvm::toString(Result.takeError()) << Input;
-  EXPECT_EQ(Output, std::string(*Result)) << Input;
+  EXPECT_EQ(Output, Result->lookup("")) << Input;
+  for (const auto  : AdditionalFiles) {
+llvm::errs() << "Looking for: " << File.first() << '\n';
+EXPECT_EQ(File.second.ModifiedText, Result->lookup(File.first())) << Input;
+  }
 }
 
 TWEAK_TEST(SwapIfBranches);
@@ -154,7 +180,8 @@
   EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
 "if (true) {continue;} else {return 100;}");
   EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
-"if () {continue;} else {return 100;}") << "broken condition";
+"if () {continue;} else {return 100;}")
+  << "broken condition";
   EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
   

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:41
+/// Represents a set of edits generated for a single file.
+struct Edit {
+  /// SHA1 hash of the file contents for the edits generated below. Clients

Hmm, it is also attractive to make this something like

```
struct Edit {
  string CodeBefore;
  tooling::Replacements Reps;

  string codeAfter();
  std::vector asTextEdits();
  bool canApplyTo(StringRef CodeBefore); // does line-ending normalization
}
```
if we were worried about abuse, we could make CodeBefore private



Comment at: clang-tools-extra/clangd/SourceCode.h:43
+  /// SHA1 hash of the file contents for the edits generated below. Clients
+  /// should verify contents they see are the same as this one before applying
+  /// edits.

, if possible.
(Sometimes we're just sending these over the wire)



Comment at: clang-tools-extra/clangd/SourceCode.h:46
+  std::array ContentDigests;
+  tooling::Replacements Reps;
+  llvm::Optional> Edits;

document that these are equivalent.
Why is Edits optional?



Comment at: clang-tools-extra/clangd/SourceCode.h:46
+  std::array ContentDigests;
+  tooling::Replacements Reps;
+  llvm::Optional> Edits;

sammccall wrote:
> document that these are equivalent.
> Why is Edits optional?
nit: reps -> replacements
ContentDigests -> ContentDigest



Comment at: clang-tools-extra/clangd/SourceCode.h:49
+
+  static Edit generateEdit(llvm::StringRef Code, tooling::Replacements Reps) {
+Edit E;

make this a constructor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+  assert(It.second.Edits && "TextEdits hasn't been generated?");
+  if (auto Draft = DraftMgr.getDraft(It.first())) {
+llvm::StringRef Contents = *Draft;

kadircet wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > This callback is called asynchronously and the version inside `DraftMgr` 
> > > may be newer than the one we used to calculate the original offsets 
> > > inside replacements at this point.
> > > 
> > > We should not rely on `DraftMgr`, doing conversions on the source buffers 
> > > inside `SourceManager` is probably the most reliable option we have.
> > I think you've misunderstood what this code is doing (it needs comments!)
> > 
> > The SourceManager contains the code that we've calculated the edits 
> > against, by definition. So checking against it does nothing.
> > 
> > When we send the edits to the client, it's going to apply them to the file 
> > if it's not open, but to the dirty buffer if it is open.
> > If the dirty buffer has different contents than what we saw (and calculated 
> > edits against) then the edits will be wrong - that's what this code guards 
> > against.
> > 
> > So looking at the latest available content in the draftmgr (at the end of 
> > the request, not the start!) is the right thing here, I think.
> > This callback is called asynchronously and the version inside DraftMgr may 
> > be newer than the one we used to calculate the original offsets inside 
> > replacements at this point.
> 
> That's exactly what we are checking though?
> 
> We want to bail out if user has a "different" version of the source code in 
> their editor by the time we generated edits. Since editors will most likely 
> try to apply the edits onto the version in editor.
You are right, I'm sorry. I did not read into the code, just assumed it was 
doing something it doesn't.
Again, very sorry for the disturbance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, lebedev.ri.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

Since statements, expressions and types are allocated with the 
`BumpPtrAllocator` from `ASTContext` their destructor is not executed. Two 
classes are currently exempted from the check : `InitListExpr` due to its 
`ASTVector` and `ConstantArrayType` due to its `APInt`.

No functional changes.


Repository:
  rC Clang

https://reviews.llvm.org/D66646

Files:
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -299,6 +299,18 @@
 #CLASS "Type should not be polymorphic!");
 #include "clang/AST/TypeNodes.def"
 
+// Check that no type class has a non-trival destructor. Types are
+// allocated with the BumpPtrAllocator from ASTContext and therefore
+// their destructor is not executed.
+//
+// FIXME: ConstantArrayType is not trivially destructible because of its
+// APInt member. It should be replaced in favor of ASTContext allocation.
+#define TYPE(CLASS, BASE)  
\
+  static_assert(std::is_trivially_destructible::value ||  
\
+std::is_same::value,   
\
+#CLASS "Type should be trivially destructible!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -83,6 +83,16 @@
 #CLASS " should not be polymorphic!");
 #include "clang/AST/StmtNodes.inc"
 
+// Check that no statement / expression class has a non-trival destructor.
+// Statements and expressions are allocated with the BumpPtrAllocator from
+// ASTContext and therefore their destructor is not executed.
+#define STMT(CLASS, PARENT)
\
+  static_assert(std::is_trivially_destructible::value,  
\
+#CLASS " should be trivially destructible!");
+// FIXME: InitListExpr is not trivially destructible due to its ASTVector.
+#define INITLISTEXPR(CLASS, PARENT)
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -299,6 +299,18 @@
 #CLASS "Type should not be polymorphic!");
 #include "clang/AST/TypeNodes.def"
 
+// Check that no type class has a non-trival destructor. Types are
+// allocated with the BumpPtrAllocator from ASTContext and therefore
+// their destructor is not executed.
+//
+// FIXME: ConstantArrayType is not trivially destructible because of its
+// APInt member. It should be replaced in favor of ASTContext allocation.
+#define TYPE(CLASS, BASE)  \
+  static_assert(std::is_trivially_destructible::value ||  \
+std::is_same::value,   \
+#CLASS "Type should be trivially destructible!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -83,6 +83,16 @@
 #CLASS " should not be polymorphic!");
 #include "clang/AST/StmtNodes.inc"
 
+// Check that no statement / expression class has a non-trival destructor.
+// Statements and expressions are allocated with the BumpPtrAllocator from
+// ASTContext and therefore their destructor is not executed.
+#define STMT(CLASS, PARENT)\
+  static_assert(std::is_trivially_destructible::value,  \
+#CLASS " should be trivially destructible!");
+// FIXME: InitListExpr is not trivially destructible due to its ASTVector.
+#define INITLISTEXPR(CLASS, PARENT)
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66590: [clangd] Fix toHalfOpenFileRange where start/end endpoints are in different files due to #include

[Shaurya Gupta via Phabricator via cfe-commits]

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

LGTM




Comment at: clang-tools-extra/clangd/SourceCode.cpp:284
+  return SM.getComposedLoc(IncludingFile, Offset);
+if (Buf[Offset] == '\n' || Offset == 0) // no hash, what's going on?
+  return SourceLocation();

nit: use this as a while condition?

```
while(Offset-- && Buf[Offset] != '\n')
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66590



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

[PATCH] D66219: [clangd] Added a colorizer to the vscode extension.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks, looks better now. Some more comments on the test code.




Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:3
 import * as vscodelc from 'vscode-languageclient';
-
+import * as SM from './semantic-highlighting';
 /**

I'd name it `semanticHighlighting`



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:128
+  // DecorationTypes for the current theme that are used when highlighting.
+  private decorationTypes: vscode.TextEditorDecorationType[];
+  // The clangd TextMate scope lookup table.

nit: mention the index is the scopeIndex?



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:135
+  // Update the themeRuleMatcher that is used when highlighting. Also triggers 
a
+  // recolorization for all current highlighters. Safe to call multiple times.
+  public initialize(themeRuleMatcher: ThemeRuleMatcher) {

nit: the comment is stale now.  I believe this function is only called whenever 
we reload a theme or at the first launch of the extension.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:137
+  public initialize(themeRuleMatcher: ThemeRuleMatcher) {
+this.decorationTypes = this.scopeLookupTable.map((scopes) => {
+  const options: vscode.DecorationRenderOptions = {

if this.decorationsTypes is not empty (when this API is called multiple times), 
I think we need to call the `dispose` method to release the resource they hold.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:139
+  const options: vscode.DecorationRenderOptions = {
+color : themeRuleMatcher.getBestThemeRule(scopes[0]).foreground,
+// If the rangeBehavior is set to Open in any direction the

just want to confirm: if we fail to find a matched theme rule, we return an 
empty decoration. I think vscode just doesn't do anything on an empty color?



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:157
+  // TextEditor(s).
+  public highlight(fileUri: string, tokens: SemanticHighlightingLine[]) {
+if (!this.files.has(fileUri)) {

nit: tokens => HighlightingLines.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:190
+  protected applyHighlights(fileUri: string) {
+if (!this.decorationTypes)
+  return;

nit: add a comment, we don't apply highlights when the class is not 
ready/initialized.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts:80
+  }
+  getDecorationRanges(fileUri: string) {
+return super.getDecorationRanges(fileUri);

could you add a comment you override this so that it can be access from the 
test?



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts:93
+// Groups decorations into the scopes used.
+let line = [
+  {character : 1, length : 2, scopeIndex : 1},

nit: please use a more descriptive name, e.g. `HightlightsInLine`.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts:94
+let line = [
+  {character : 1, length : 2, scopeIndex : 1},
+  {character : 5, length : 2, scopeIndex : 1},

I believe the test code is correct, but the code here/below is complex and hard 
to follow. I think we need make the code more readable/understandable.

some suggestions:

- the line number is implicitly indicated by the index of the array, I think we 
can add one more field `line` to the structure (call HighlightingTokens).
- and creates some helper functions (with proper names) that take the 
`HighlightingTokens` as parameter and generate the data you need e.g. 
`SemanticHighlightingLine[]`, `vscode.Range`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66219



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

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

SGTM, but i wonder if this should be done one level up, in `BumpPtrAllocator` 
itself?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66646



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

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D66646#1642708 , @lebedev.ri wrote:

> SGTM, but i wonder if this should be done one level up, in `BumpPtrAllocator` 
> itself?


I don't understand. `BumpPtrAllocator` is only used to allocate raw memory and 
doesn't know anything about how it is going to be used.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66646



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

[PATCH] D65526: [Clangd] First version of ExtractFunction

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:214
+bool IsConst;
+Parameter(std::string Name, std::string Type, bool IsConst)
+: Name(Name), Type(Type), IsConst(IsConst) {}

sammccall wrote:
> I'd suggest capturing the type as a QualType instead of a string + const + 
> ref flag
> 
> When types may need to be re-spelled, we'll need that extra information - the 
> context needed to re-spell is available at render() time, not addParameter() 
> time.
Const will also depend on CapturedSourceInfo. So I'm passing QualType instead 
of the name as string.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:360
+
+CapturedSourceInfo::DeclInformation &
+CapturedSourceInfo::getDeclInformationFor(const Decl *D) {

kadircet wrote:
> why not move this and 4 following functions into astvisitor?
Because we would need to pass CapturedSourceInfo and ExtractionZone to the 
Visitor then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65526



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

[PATCH] D65526: [Clangd] First version of ExtractFunction

[Shaurya Gupta via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:360
+
+CapturedSourceInfo::DeclInformation &
+CapturedSourceInfo::getDeclInformationFor(const Decl *D) {

SureYeaah wrote:
> kadircet wrote:
> > why not move this and 4 following functions into astvisitor?
> Because we would need to pass CapturedSourceInfo and ExtractionZone to the 
> Visitor then?
Ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65526



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