[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Karasev Nikita via Phabricator via cfe-commits]

f00kat marked 4 inline comments as done.
f00kat added a comment.

Ping? :)




Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

NoQ wrote:
> NoQ wrote:
> > The sequence of `FieldRegion`s and `ElementRegion`s on top of a base region 
> > may be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.
> > 
> > I'd rather unwrap those regions one-by-one in a loop and look at the 
> > alignment of each layer.
> Alternatively, just decompose the whole region into base region and offset 
> and see if base region has the necessary alignment and the offset is 
> divisible by the necessary alignment.
> The sequence of FieldRegions and ElementRegions on top of a base region may 
> be arbitrary: var.a[0].b[1][2].c.d[3] etc.

But i think(hope) I already do this and even have tests for this cases. For 
example
```void test22() {
  struct alignas(alignof(short)) Z {
char p;
char c[10];
  };

  struct Y {
char p;
Z b[10];
  };

  struct X {
Y a[10];
  } Xi; // expected-note {{'Xi' initialized here}}

  // ok. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset Z.p) 
+ 3(index)
  ::new (&Xi.a[0].b[0].c[3]) long;
}```

Cases with multidimensional arrays will also be handled correctly because 
method 'TheElementRegion->getAsArrayOffset()' calculates the offset for 
multidimensional arrays
```void testXX() {
struct Y {
char p;
char b[10][10];
};

struct X {
Y a[10];
} Xi;

::new (&Xi.a[0].b[0][0]) long;
}```

I can explain the code below for ElementRegion if needed.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

f00kat wrote:
> NoQ wrote:
> > NoQ wrote:
> > > The sequence of `FieldRegion`s and `ElementRegion`s on top of a base 
> > > region may be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.
> > > 
> > > I'd rather unwrap those regions one-by-one in a loop and look at the 
> > > alignment of each layer.
> > Alternatively, just decompose the whole region into base region and offset 
> > and see if base region has the necessary alignment and the offset is 
> > divisible by the necessary alignment.
> > The sequence of FieldRegions and ElementRegions on top of a base region may 
> > be arbitrary: var.a[0].b[1][2].c.d[3] etc.
> 
> But i think(hope) I already do this and even have tests for this cases. For 
> example
> ```void test22() {
>   struct alignas(alignof(short)) Z {
> char p;
> char c[10];
>   };
> 
>   struct Y {
> char p;
> Z b[10];
>   };
> 
>   struct X {
> Y a[10];
>   } Xi; // expected-note {{'Xi' initialized here}}
> 
>   // ok. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset 
> Z.p) + 3(index)
>   ::new (&Xi.a[0].b[0].c[3]) long;
> }```
> 
> Cases with multidimensional arrays will also be handled correctly because 
> method 'TheElementRegion->getAsArrayOffset()' calculates the offset for 
> multidimensional arrays
> ```void testXX() {
>   struct Y {
>   char p;
>   char b[10][10];
>   };
> 
>   struct X {
>   Y a[10];
>   } Xi;
> 
>   ::new (&Xi.a[0].b[0][0]) long;
> }```
> 
> I can explain the code below for ElementRegion if needed.
?



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:25
 public:
   void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
 

NoQ wrote:
> Before i forget: Ideally @martong should have subscribed to [[ 
> https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CheckerDocumentation.html#a7fdb3b5ff726f4c5e782cef0d59c01ad
>  | `checkNewAllocator` ]] because it fires before the construct-expression 
> whereas this callback fires after construct-expression which is too late as 
> the UB we're trying to catch has occured much earlier.
Ops, I forgot about it. Will be fixed soon.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:25
 public:
   void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
 

f00kat wrote:
> NoQ wrote:
> > Before i forget: Ideally @martong should have subscribed to [[ 
> > https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CheckerDocumentation.html#a7fdb3b5ff726f4c5e782cef0d59c01ad
> >  | `checkNewAllocator` ]] because it fires before the construct-expression 
> > whereas this callback fires after construct-expression which is too late as 
> > the UB we're trying to catch has occured much earlier.
> Ops, I forgot about it. Will be fixed soon.
When I use checkNewAllocator instead of check::PreStmt an error 
occures in method PathDiagnosticBuilder::generate.
I

[PATCH] D78429: [clangd] Metric tracking through Tracer

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Introduce latency tracking through context.
- Add metrics for code actions and rename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78429

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/Trace.cpp
  clang-tools-extra/clangd/Trace.h

Index: clang-tools-extra/clangd/Trace.h
===
--- clang-tools-extra/clangd/Trace.h
+++ clang-tools-extra/clangd/Trace.h
@@ -18,14 +18,44 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace trace {
 
+/// Represents measurements of clangd events, e.g. operation latency. Reports
+/// itself to active tracer on destruction.
+struct Metric {
+  enum Type {
+/// Occurence of an event, e.g. cache access.
+Increment,
+/// Aspect of an event, e.g. number of symbols in an index lookup.
+Sample,
+  };
+  /// Uniquely identifies the metric.
+  const llvm::StringLiteral Name;
+  /// Divides a metric into meaningful parts. e.g. hit/miss to represent result
+  /// of a cache access.
+  std::vector Labels;
+  double Value = 0;
+  Type Ty;
+
+  Metric(llvm::StringLiteral Name) : Name(Name) {}
+  Metric(const Metric &) = delete;
+  Metric &operator=(const Metric &) = delete;
+  ~Metric();
+};
+
+/// Creates a context that will report its duration as a metric on destruction.
+Context latencyTrackingContext(llvm::StringRef OpName);
+
 /// A consumer of trace events. The events are produced by Spans and trace::log.
 /// Implementations of this interface must be thread-safe.
 class EventTracer {
@@ -47,6 +77,9 @@
 
   /// Called for instant events.
   virtual void instant(llvm::StringRef Name, llvm::json::Object &&Args) = 0;
+
+  /// Called whenever an event exports a measurement.
+  virtual void record(const Metric &Metric) {}
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
Index: clang-tools-extra/clangd/Trace.cpp
===
--- clang-tools-extra/clangd/Trace.cpp
+++ clang-tools-extra/clangd/Trace.cpp
@@ -10,11 +10,14 @@
 #include "Context.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/FormatProviders.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Threading.h"
 #include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -149,10 +152,10 @@
   void rawEvent(llvm::StringRef Phase,
 const llvm::json::Object &Event) /*REQUIRES(Mu)*/ {
 // PID 0 represents the clangd process.
-Out.object([&]{
+Out.object([&] {
   Out.attribute("pid", 0);
   Out.attribute("ph", Phase);
-  for (const auto& KV : Event)
+  for (const auto &KV : Event)
 Out.attribute(KV.first, KV.second);
 });
   }
@@ -230,6 +233,25 @@
 T->endSpan();
 }
 
+Metric::~Metric() {
+  if (!T)
+return;
+  T->record(*this);
+}
+
+Context latencyTrackingContext(llvm::StringRef OpName) {
+  using Clock = std::chrono::high_resolution_clock;
+  return Context::current().derive(
+  llvm::make_scope_exit([StartTime = Clock::now(), OpName = OpName.str()] {
+Metric M("/clangd/latencies");
+M.Value = std::chrono::duration_cast(
+  Clock::now() - StartTime)
+  .count();
+M.Labels = {OpName};
+M.Ty = Metric::Sample;
+  }));
+}
+
 } // namespace trace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -143,10 +143,17 @@
   /// the cache anymore. If nullptr was cached for \p K, this function will
   /// return a null unique_ptr wrapped into an optional.
   llvm::Optional> take(Key K) {
+trace::Metric M("/clangd/ast_cache");
+M.Ty = trace::Metric::Increment;
+M.Value = 1;
+// Record metric after unlocking the mutex.
 std::unique_lock Lock(Mut);
 auto Existing = findByKey(K);
-if (Existing == LRU.end())
+if (Existing == LRU.end()) {
+  M.Labels = {"miss"};
   return None;
+}
+M.Labels = {"hit"};
 std::unique_ptr V = std::move(Existing->second);
 LRU.erase(Existing);
 // GCC 4.8 fails to compile `return V;`, as it tries to call the copy
Index: clang-tools-extra/clangd/ClangdServer.cpp
==

[PATCH] D78429: [clangd] Metric tracking through Tracer

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Report metrics on destruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78429

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/Trace.cpp
  clang-tools-extra/clangd/Trace.h

Index: clang-tools-extra/clangd/Trace.h
===
--- clang-tools-extra/clangd/Trace.h
+++ clang-tools-extra/clangd/Trace.h
@@ -18,14 +18,40 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace trace {
 
+/// Represents measurements of clangd events, e.g. operation latency. Reports
+/// itself to active tracer on destruction.
+struct Metric {
+  enum Type {
+/// Occurence of an event, e.g. cache access.
+Increment,
+/// Aspect of an event, e.g. number of symbols in an index lookup.
+Sample,
+  };
+  /// Uniquely identifies the metric.
+  const llvm::StringLiteral Name;
+  /// Divides a metric into meaningful parts. e.g. hit/miss to represent result
+  /// of a cache access.
+  std::vector Labels;
+  double Value = 0;
+  Type Ty;
+
+  Metric(llvm::StringLiteral Name) : Name(Name) {}
+  Metric(const Metric &) = delete;
+  Metric &operator=(const Metric &) = delete;
+  ~Metric();
+};
+
 /// A consumer of trace events. The events are produced by Spans and trace::log.
 /// Implementations of this interface must be thread-safe.
 class EventTracer {
@@ -47,6 +73,9 @@
 
   /// Called for instant events.
   virtual void instant(llvm::StringRef Name, llvm::json::Object &&Args) = 0;
+
+  /// Called whenever an event exports a measurement.
+  virtual void record(const Metric &Metric) {}
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
Index: clang-tools-extra/clangd/Trace.cpp
===
--- clang-tools-extra/clangd/Trace.cpp
+++ clang-tools-extra/clangd/Trace.cpp
@@ -149,10 +149,10 @@
   void rawEvent(llvm::StringRef Phase,
 const llvm::json::Object &Event) /*REQUIRES(Mu)*/ {
 // PID 0 represents the clangd process.
-Out.object([&]{
+Out.object([&] {
   Out.attribute("pid", 0);
   Out.attribute("ph", Phase);
-  for (const auto& KV : Event)
+  for (const auto &KV : Event)
 Out.attribute(KV.first, KV.second);
 });
   }
@@ -230,6 +230,12 @@
 T->endSpan();
 }
 
+Metric::~Metric() {
+  if (!T)
+return;
+  T->record(*this);
+}
+
 } // namespace trace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -143,10 +143,17 @@
   /// the cache anymore. If nullptr was cached for \p K, this function will
   /// return a null unique_ptr wrapped into an optional.
   llvm::Optional> take(Key K) {
+trace::Metric M("/clangd/ast_cache");
+M.Ty = trace::Metric::Increment;
+M.Value = 1;
+// Record metric after unlocking the mutex.
 std::unique_lock Lock(Mut);
 auto Existing = findByKey(K);
-if (Existing == LRU.end())
+if (Existing == LRU.end()) {
+  M.Labels = {"miss"};
   return None;
+}
+M.Labels = {"hit"};
 std::unique_ptr V = std::move(Existing->second);
 LRU.erase(Existing);
 // GCC 4.8 fails to compile `return V;`, as it tries to call the copy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78436: [clangd] Record metrics for code action and rename usage

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet abandoned this revision.
kadircet added a comment.

Merged into D78429 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78436



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

[PATCH] D78429: [clangd] Metric tracking through Tracer

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 258595.
kadircet added a comment.
Herald added a subscriber: ormris.

- Add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78429

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/Trace.cpp
  clang-tools-extra/clangd/Trace.h
  clang-tools-extra/clangd/unittests/TraceTests.cpp

Index: clang-tools-extra/clangd/unittests/TraceTests.cpp
===
--- clang-tools-extra/clangd/unittests/TraceTests.cpp
+++ clang-tools-extra/clangd/unittests/TraceTests.cpp
@@ -6,10 +6,12 @@
 //
 //===--===//
 
+#include "Context.h"
 #include "Trace.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/YAMLParser.h"
@@ -122,6 +124,49 @@
   ASSERT_EQ(++Prop, Root->end());
 }
 
+class MetricsTracerTest : public ::testing::Test {
+public:
+  MetricsTracerTest() : Tracer(Metrics), Session(Tracer) {}
+
+protected:
+  class MetricsTracer : public trace::EventTracer {
+  public:
+MetricsTracer(std::vector &Metrics)
+: Metrics(Metrics) {}
+Context beginSpan(llvm::StringRef, llvm::json::Object *) override {
+  return Context::current().clone();
+}
+void record(const trace::Metric &Metric) override {
+  Metrics.push_back(Metric.Name);
+}
+void instant(llvm::StringRef, llvm::json::Object &&) override {}
+
+  private:
+std::vector &Metrics;
+  };
+  std::vector Metrics;
+  MetricsTracer Tracer;
+  trace::Session Session;
+};
+
+TEST_F(MetricsTracerTest, SendsOnDestruct) {
+  const llvm::StringLiteral MetricName = "/test/metric";
+  {
+trace::Metric M{MetricName};
+EXPECT_THAT(Metrics, testing::IsEmpty());
+  }
+  EXPECT_THAT(Metrics, testing::ElementsAre(MetricName));
+}
+
+TEST_F(MetricsTracerTest, LatencyTest) {
+  const llvm::StringLiteral MetricName = "/clangd/latencies";
+  {
+WithContext Ctx(trace::latencyTrackingContext("op_name"));
+EXPECT_THAT(Metrics, testing::IsEmpty());
+  }
+  EXPECT_THAT(Metrics, testing::ElementsAre(MetricName));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Trace.h
===
--- clang-tools-extra/clangd/Trace.h
+++ clang-tools-extra/clangd/Trace.h
@@ -18,14 +18,44 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace trace {
 
+/// Represents measurements of clangd events, e.g. operation latency. Reports
+/// itself to active tracer on destruction.
+struct Metric {
+  enum Type {
+/// Occurence of an event, e.g. cache access.
+Increment,
+/// Aspect of an event, e.g. number of symbols in an index lookup.
+Sample,
+  };
+  /// Uniquely identifies the metric.
+  const llvm::StringLiteral Name;
+  /// Divides a metric into meaningful parts. e.g. hit/miss to represent result
+  /// of a cache access.
+  std::vector Labels;
+  double Value = 0;
+  Type Ty;
+
+  Metric(llvm::StringLiteral Name) : Name(Name) {}
+  Metric(const Metric &) = delete;
+  Metric &operator=(const Metric &) = delete;
+  ~Metric();
+};
+
+/// Creates a context that will report its duration as a metric on destruction.
+Context latencyTrackingContext(llvm::StringRef OpName);
+
 /// A consumer of trace events. The events are produced by Spans and trace::log.
 /// Implementations of this interface must be thread-safe.
 class EventTracer {
@@ -47,6 +77,9 @@
 
   /// Called for instant events.
   virtual void instant(llvm::StringRef Name, llvm::json::Object &&Args) = 0;
+
+  /// Called whenever an event exports a measurement.
+  virtual void record(const Metric &Metric) {}
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
Index: clang-tools-extra/clangd/Trace.cpp
===
--- clang-tools-extra/clangd/Trace.cpp
+++ clang-tools-extra/clangd/Trace.cpp
@@ -10,11 +10,14 @@
 #include "Context.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/FormatProviders.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Threading.h"
 #include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -149,10 +152,10 @@
   void rawEvent(llvm::StringRef Phase,
 const llvm::json::Object &Event) /*REQUIRES(Mu)*/ 

[PATCH] D78454: [clangd] Highlight related control flow.

[Sam McCall via Phabricator via cfe-commits]

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

This means e.g. highlighting "return" will show other returns/throws
from the same function, highlighting a case will show all the
return/breaks etc.

This is a bit of an abuse of textDocument/highlight, but seems useful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78454

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -116,6 +116,117 @@
   }
 }
 
+TEST(HighlightsTest, ControlFlow) {
+  const char *Tests[] = {
+  R"cpp(
+// Highlight same-function returns.
+int fib(unsigned n) {
+  if (n <= 1) [[ret^urn]] 1;
+  [[return]] fib(n - 1) + fib(n - 2);
+
+  // Returns from other functions not highlighted.
+  auto Lambda = [] { return; };
+  class LocalClass { void x() { return; } };
+}
+  )cpp",
+
+  R"cpp(
+// Highlight loop control flow
+int magic() {
+  int counter = 0;
+  [[^for]] (char c : "fruit loops!") {
+if (c == ' ') [[continue]];
+counter += c;
+if (c == '!') [[break]];
+if (c == '?') [[return]] -1;
+  }
+  return counter;
+}
+  )cpp",
+
+  R"cpp(
+// Highlight loop and same-loop control flow
+void nonsense() {
+  [[while]] (true) {
+if (false) [[bre^ak]];
+switch (1) break;
+[[continue]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight switch for break (but not other breaks).
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[default]]:
+[[^break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight case and exits for switch-break (but not other cases).
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[default]]:
+[[return]];
+[[^break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight exits and switch for case
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[d^efault]]:
+[[return]];
+[[break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight nothing for switch.
+void describe(unsigned n) {
+  s^witch(n) {
+  case 0:
+break;
+  default:
+return;
+break;
+  }
+}
+  )cpp",
+
+  R"cpp(
+// FIXME: match exception type against catch blocks
+int catchy() {
+  try { // wrong: highlight try with matching catch
+try {   // correct: has no matching catch
+  [[thr^ow]] "oh no!";
+} catch (int) { }   // correct: catch doesn't match type
+[[return]] -1;  // correct: exits the matching catch
+  } catch (const char*) { } // wrong: highlight matching catch
+  [[return]] 42;// wrong: throw doesn't exit function
+}
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
+<< Test;
+  }
+}
+
 MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
   llvm::Optional Def = DefOrNone;
   if (Name != arg.Name) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -22,12 +22,17 @@
 #include "index/Relation.h"
 #include "index/SymbolLocation.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LLVM.h"
@@ -43,6 +48,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -614,35 +

[clang-tools-extra] 8c68de2 - [clangd] Extend YAML Serialization

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-19T14:32:31+02:00
New Revision: 8c68de2d63000d2d66f2109665a892e673f93107

URL: 
https://github.com/llvm/llvm-project/commit/8c68de2d63000d2d66f2109665a892e673f93107
DIFF: 
https://github.com/llvm/llvm-project/commit/8c68de2d63000d2d66f2109665a892e673f93107.diff

LOG: [clangd] Extend YAML Serialization

Reviewers: sammccall

Reviewed By: sammccall

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

Tags: #clang-tools-extra, #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/index/YAMLSerialization.cpp
clang-tools-extra/clangd/unittests/SerializationTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp 
b/clang-tools-extra/clangd/index/YAMLSerialization.cpp
index 8895d7a97011..79965ceb1634 100644
--- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -41,6 +41,8 @@ struct VariantEntry {
   llvm::Optional Symbol;
   llvm::Optional Refs;
   llvm::Optional Relation;
+  llvm::Optional Source;
+  llvm::Optional Cmd;
 };
 // A class helps YAML to serialize the 32-bit encoded position (Line&Column),
 // as YAMLIO can't directly map bitfields.
@@ -49,10 +51,16 @@ struct YPosition {
   uint32_t Column;
 };
 
+// avoid ODR violation of specialization for non-owned CompileCommand
+struct CompileCommandYAML : clang::tooling::CompileCommand {};
+
 } // namespace
 namespace llvm {
 namespace yaml {
 
+using clang::clangd::FileDigest;
+using clang::clangd::IncludeGraph;
+using clang::clangd::IncludeGraphNode;
 using clang::clangd::Ref;
 using clang::clangd::RefKind;
 using clang::clangd::Relation;
@@ -65,6 +73,7 @@ using clang::index::SymbolInfo;
 using clang::index::SymbolKind;
 using clang::index::SymbolLanguage;
 using clang::index::SymbolRole;
+using clang::tooling::CompileCommand;
 
 // Helper to (de)serialize the SymbolID. We serialize it as a hex string.
 struct NormalizedSymbolID {
@@ -308,6 +317,59 @@ template <> struct MappingTraits {
   }
 };
 
+struct NormalizedSourceFlag {
+  NormalizedSourceFlag(IO &) {}
+  NormalizedSourceFlag(IO &, IncludeGraphNode::SourceFlag O) {
+Flag = static_cast(O);
+  }
+
+  IncludeGraphNode::SourceFlag denormalize(IO &) {
+return static_cast(Flag);
+  }
+
+  uint8_t Flag = 0;
+};
+
+struct NormalizedFileDigest {
+  NormalizedFileDigest(IO &) {}
+  NormalizedFileDigest(IO &, const FileDigest &Digest) {
+HexString = llvm::toHex(Digest);
+  }
+
+  FileDigest denormalize(IO &I) {
+FileDigest Digest;
+if (HexString.size() == Digest.size() * 2 &&
+llvm::all_of(HexString, llvm::isHexDigit)) {
+  memcpy(Digest.data(), llvm::fromHex(HexString).data(), Digest.size());
+} else {
+  I.setError(std::string("Bad hex file digest: ") + HexString);
+}
+return Digest;
+  }
+
+  std::string HexString;
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, IncludeGraphNode &Node) {
+IO.mapRequired("URI", Node.URI);
+MappingNormalization
+NSourceFlag(IO, Node.Flags);
+IO.mapRequired("Flags", NSourceFlag->Flag);
+MappingNormalization NDigest(IO,
+   
Node.Digest);
+IO.mapRequired("Digest", NDigest->HexString);
+IO.mapRequired("DirectIncludes", Node.DirectIncludes);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, CompileCommandYAML &Cmd) {
+IO.mapRequired("Directory", Cmd.Directory);
+IO.mapRequired("CommandLine", Cmd.CommandLine);
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO &IO, VariantEntry &Variant) {
 if (IO.mapTag("!Symbol", Variant.Symbol.hasValue())) {
@@ -322,6 +384,15 @@ template <> struct MappingTraits {
   if (!IO.outputting())
 Variant.Relation.emplace();
   MappingTraits::mapping(IO, *Variant.Relation);
+} else if (IO.mapTag("!Source", Variant.Source.hasValue())) {
+  if (!IO.outputting())
+Variant.Source.emplace();
+  MappingTraits::mapping(IO, *Variant.Source);
+} else if (IO.mapTag("!Cmd", Variant.Cmd.hasValue())) {
+  if (!IO.outputting())
+Variant.Cmd.emplace();
+  MappingTraits::mapping(
+  IO, static_cast(*Variant.Cmd));
 }
   }
 };
@@ -351,6 +422,18 @@ void writeYAML(const IndexFileOut &O, llvm::raw_ostream 
&OS) {
   Entry.Relation = R;
   Yout << Entry;
 }
+  if (O.Sources) {
+for (const auto &Source : *O.Sources) {
+  VariantEntry Entry;
+  Entry.Source = Source.getValue();
+  Yout << Entry;
+}
+  }
+  if (O.Cmd) {
+VariantEntry Entry;
+Entry.Cmd = *O.Cmd;
+Yout << Entry;
+  }
 }
 
 llvm::Expected readYAML(llvm::StringRef Data) {
@@ -361,6 +444,8 @@ llvm::Expected readYAML(llvm::StringRef Data) {
   Arena

[clang-tools-extra] 098e40e - [clangd] Add index export to dexp

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-19T14:34:46+02:00
New Revision: 098e40eac524fd7bcad72d37378d25f4305de512

URL: 
https://github.com/llvm/llvm-project/commit/098e40eac524fd7bcad72d37378d25f4305de512
DIFF: 
https://github.com/llvm/llvm-project/commit/098e40eac524fd7bcad72d37378d25f4305de512.diff

LOG: [clangd] Add index export to dexp

Summary: Add a command to dexp that exports index data in chosen format (e.g. 
YAML).

Reviewers: sammccall

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

Tags: #clang-tools-extra, #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp 
b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
index ae49f9437211..015246d1a82f 100644
--- a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -258,6 +258,56 @@ class Refs : public Command {
   }
 };
 
+class Export : public Command {
+  llvm::cl::opt Format{
+  "format",
+  llvm::cl::desc("Format of index export"),
+  llvm::cl::values(
+  clEnumValN(IndexFileFormat::YAML, "yaml",
+ "human-readable YAML format"),
+  clEnumValN(IndexFileFormat::RIFF, "binary", "binary RIFF format")),
+  llvm::cl::init(IndexFileFormat::YAML),
+  };
+  llvm::cl::opt OutputFile{
+  "output-file",
+  llvm::cl::Positional,
+  llvm::cl::Required,
+  llvm::cl::desc("Output file for export"),
+  };
+
+public:
+  void run() {
+using namespace clang::clangd;
+// Read input file (as specified in global option)
+auto Buffer = llvm::MemoryBuffer::getFile(IndexPath);
+if (!Buffer) {
+  llvm::errs() << llvm::formatv("Can't open {0}", IndexPath) << "\n";
+  return;
+}
+
+// Auto-detects input format when parsing
+auto IndexIn = clang::clangd::readIndexFile(Buffer->get()->getBuffer());
+if (!IndexIn) {
+  llvm::errs() << llvm::toString(IndexIn.takeError()) << "\n";
+  return;
+}
+
+// Prepare output file
+std::error_code EC;
+llvm::raw_fd_ostream OutputStream(OutputFile, EC);
+if (EC) {
+  llvm::errs() << llvm::formatv("Can't open {0} for writing", OutputFile)
+   << "\n";
+  return;
+}
+
+// Export
+clang::clangd::IndexFileOut IndexOut(IndexIn.get());
+IndexOut.Format = Format;
+OutputStream << IndexOut;
+  }
+};
+
 struct {
   const char *Name;
   const char *Description;
@@ -268,6 +318,7 @@ struct {
  std::make_unique},
 {"refs", "Find references by ID or qualified name",
  std::make_unique},
+{"export", "Export index", std::make_unique},
 };
 
 std::unique_ptr openIndex(llvm::StringRef Index) {



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

[PATCH] D77938: [clangd] Extend YAML Serialization

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c68de2d6300: [clangd] Extend YAML Serialization (authored 
by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77938

Files:
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp

Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -16,6 +16,7 @@
 
 using ::testing::_;
 using ::testing::AllOf;
+using ::testing::ElementsAre;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedElementsAreArray;
@@ -91,6 +92,20 @@
 Object:
   ID:  6512AEC512EA3A2D
 ...
+--- !Cmd
+Directory:   'testdir'
+CommandLine:
+  - 'cmd1'
+  - 'cmd2'
+...
+--- !Source
+URI: 'file:///path/source1.cpp'
+Flags:   1
+Digest:  EED8F5EAF25C453C
+DirectIncludes:
+  - 'file:///path/inc1.h'
+  - 'file:///path/inc2.h'
+...
 )";
 
 MATCHER_P(ID, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); }
@@ -152,6 +167,21 @@
   EXPECT_THAT(
   *ParsedYAML->Relations,
   UnorderedElementsAre(Relation{Base, RelationKind::BaseOf, Derived}));
+
+  ASSERT_TRUE(bool(ParsedYAML->Cmd));
+  auto &Cmd = *ParsedYAML->Cmd;
+  ASSERT_EQ(Cmd.Directory, "testdir");
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre("cmd1", "cmd2"));
+
+  ASSERT_TRUE(bool(ParsedYAML->Sources));
+  const auto *URI = "file:///path/source1.cpp";
+  ASSERT_TRUE(ParsedYAML->Sources->count(URI));
+  auto IGNDeserialized = ParsedYAML->Sources->lookup(URI);
+  EXPECT_EQ(llvm::toHex(IGNDeserialized.Digest), "EED8F5EAF25C453C");
+  EXPECT_THAT(IGNDeserialized.DirectIncludes,
+  ElementsAre("file:///path/inc1.h", "file:///path/inc2.h"));
+  EXPECT_EQ(IGNDeserialized.URI, URI);
+  EXPECT_EQ(IGNDeserialized.Flags, IncludeGraphNode::SourceFlag(1));
 }
 
 std::vector YAMLFromSymbols(const SymbolSlab &Slab) {
Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -41,6 +41,8 @@
   llvm::Optional Symbol;
   llvm::Optional Refs;
   llvm::Optional Relation;
+  llvm::Optional Source;
+  llvm::Optional Cmd;
 };
 // A class helps YAML to serialize the 32-bit encoded position (Line&Column),
 // as YAMLIO can't directly map bitfields.
@@ -49,10 +51,16 @@
   uint32_t Column;
 };
 
+// avoid ODR violation of specialization for non-owned CompileCommand
+struct CompileCommandYAML : clang::tooling::CompileCommand {};
+
 } // namespace
 namespace llvm {
 namespace yaml {
 
+using clang::clangd::FileDigest;
+using clang::clangd::IncludeGraph;
+using clang::clangd::IncludeGraphNode;
 using clang::clangd::Ref;
 using clang::clangd::RefKind;
 using clang::clangd::Relation;
@@ -65,6 +73,7 @@
 using clang::index::SymbolKind;
 using clang::index::SymbolLanguage;
 using clang::index::SymbolRole;
+using clang::tooling::CompileCommand;
 
 // Helper to (de)serialize the SymbolID. We serialize it as a hex string.
 struct NormalizedSymbolID {
@@ -308,6 +317,59 @@
   }
 };
 
+struct NormalizedSourceFlag {
+  NormalizedSourceFlag(IO &) {}
+  NormalizedSourceFlag(IO &, IncludeGraphNode::SourceFlag O) {
+Flag = static_cast(O);
+  }
+
+  IncludeGraphNode::SourceFlag denormalize(IO &) {
+return static_cast(Flag);
+  }
+
+  uint8_t Flag = 0;
+};
+
+struct NormalizedFileDigest {
+  NormalizedFileDigest(IO &) {}
+  NormalizedFileDigest(IO &, const FileDigest &Digest) {
+HexString = llvm::toHex(Digest);
+  }
+
+  FileDigest denormalize(IO &I) {
+FileDigest Digest;
+if (HexString.size() == Digest.size() * 2 &&
+llvm::all_of(HexString, llvm::isHexDigit)) {
+  memcpy(Digest.data(), llvm::fromHex(HexString).data(), Digest.size());
+} else {
+  I.setError(std::string("Bad hex file digest: ") + HexString);
+}
+return Digest;
+  }
+
+  std::string HexString;
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, IncludeGraphNode &Node) {
+IO.mapRequired("URI", Node.URI);
+MappingNormalization
+NSourceFlag(IO, Node.Flags);
+IO.mapRequired("Flags", NSourceFlag->Flag);
+MappingNormalization NDigest(IO,
+   Node.Digest);
+IO.mapRequired("Digest", NDigest->HexString);
+IO.mapRequired("DirectIncludes", Node.DirectIncludes);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, CompileCommandYAML &Cmd) {
+IO.mapRequired("Directory", Cmd.Directory);
+IO.mapRequired("CommandLine", Cmd.CommandLine);
+  }
+};
+
 template <> struc

[PATCH] D77385: [clangd] Add index export to dexp

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG098e40eac524: [clangd] Add index export to dexp (authored by 
sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77385

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -258,6 +258,56 @@
   }
 };
 
+class Export : public Command {
+  llvm::cl::opt Format{
+  "format",
+  llvm::cl::desc("Format of index export"),
+  llvm::cl::values(
+  clEnumValN(IndexFileFormat::YAML, "yaml",
+ "human-readable YAML format"),
+  clEnumValN(IndexFileFormat::RIFF, "binary", "binary RIFF format")),
+  llvm::cl::init(IndexFileFormat::YAML),
+  };
+  llvm::cl::opt OutputFile{
+  "output-file",
+  llvm::cl::Positional,
+  llvm::cl::Required,
+  llvm::cl::desc("Output file for export"),
+  };
+
+public:
+  void run() {
+using namespace clang::clangd;
+// Read input file (as specified in global option)
+auto Buffer = llvm::MemoryBuffer::getFile(IndexPath);
+if (!Buffer) {
+  llvm::errs() << llvm::formatv("Can't open {0}", IndexPath) << "\n";
+  return;
+}
+
+// Auto-detects input format when parsing
+auto IndexIn = clang::clangd::readIndexFile(Buffer->get()->getBuffer());
+if (!IndexIn) {
+  llvm::errs() << llvm::toString(IndexIn.takeError()) << "\n";
+  return;
+}
+
+// Prepare output file
+std::error_code EC;
+llvm::raw_fd_ostream OutputStream(OutputFile, EC);
+if (EC) {
+  llvm::errs() << llvm::formatv("Can't open {0} for writing", OutputFile)
+   << "\n";
+  return;
+}
+
+// Export
+clang::clangd::IndexFileOut IndexOut(IndexIn.get());
+IndexOut.Format = Format;
+OutputStream << IndexOut;
+  }
+};
+
 struct {
   const char *Name;
   const char *Description;
@@ -268,6 +318,7 @@
  std::make_unique},
 {"refs", "Find references by ID or qualified name",
  std::make_unique},
+{"export", "Export index", std::make_unique},
 };
 
 std::unique_ptr openIndex(llvm::StringRef Index) {


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -258,6 +258,56 @@
   }
 };
 
+class Export : public Command {
+  llvm::cl::opt Format{
+  "format",
+  llvm::cl::desc("Format of index export"),
+  llvm::cl::values(
+  clEnumValN(IndexFileFormat::YAML, "yaml",
+ "human-readable YAML format"),
+  clEnumValN(IndexFileFormat::RIFF, "binary", "binary RIFF format")),
+  llvm::cl::init(IndexFileFormat::YAML),
+  };
+  llvm::cl::opt OutputFile{
+  "output-file",
+  llvm::cl::Positional,
+  llvm::cl::Required,
+  llvm::cl::desc("Output file for export"),
+  };
+
+public:
+  void run() {
+using namespace clang::clangd;
+// Read input file (as specified in global option)
+auto Buffer = llvm::MemoryBuffer::getFile(IndexPath);
+if (!Buffer) {
+  llvm::errs() << llvm::formatv("Can't open {0}", IndexPath) << "\n";
+  return;
+}
+
+// Auto-detects input format when parsing
+auto IndexIn = clang::clangd::readIndexFile(Buffer->get()->getBuffer());
+if (!IndexIn) {
+  llvm::errs() << llvm::toString(IndexIn.takeError()) << "\n";
+  return;
+}
+
+// Prepare output file
+std::error_code EC;
+llvm::raw_fd_ostream OutputStream(OutputFile, EC);
+if (EC) {
+  llvm::errs() << llvm::formatv("Can't open {0} for writing", OutputFile)
+   << "\n";
+  return;
+}
+
+// Export
+clang::clangd::IndexFileOut IndexOut(IndexIn.get());
+IndexOut.Format = Format;
+OutputStream << IndexOut;
+  }
+};
+
 struct {
   const char *Name;
   const char *Description;
@@ -268,6 +318,7 @@
  std::make_unique},
 {"refs", "Find references by ID or qualified name",
  std::make_unique},
+{"export", "Export index", std::make_unique},
 };
 
 std::unique_ptr openIndex(llvm::StringRef Index) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks pretty good! Haven't reviewed the tests or removal of consistent 
preamble support yet. (Mostly note-to-self)




Comment at: clang-tools-extra/clangd/Preamble.cpp:115
+do {
+  PP.Lex(Tok);
+} while (Tok.isNot(tok::eof) &&

Given your getDecomposedTok before, you might want to assert Tok is in the main 
file inside the loop



Comment at: clang-tools-extra/clangd/Preamble.cpp:126
+std::vector
+getPreambleIncludes(llvm::StringRef Contents,
+llvm::IntrusiveRefCntPtr VFS,

I'd call this scanPreambleIncludes or something just slightly less generic - 
again here we don't want it to be  accidentally reused for something that needs 
to be accurate.



Comment at: clang-tools-extra/clangd/Preamble.cpp:148
+  PreambleOnlyAction Action;
+  if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]))
+return {};

Something needs to check that there's one input etc.
(This could maybe be moved into buildCompilerInvocation I guess, but currently 
the caller does it)



Comment at: clang-tools-extra/clangd/Preamble.cpp:244
 std::vector Diags = PreambleDiagnostics.take();
+auto AllIncludes = getPreambleIncludes(Inputs.Contents,
+   
StatCache->getConsumingFS(Inputs.FS),

I'd stick to LexedIncludes or ApproxmimateIncludes or BaselineIncludes - it 
excludes stuff under ifdefs that *can* be resolved, and we don't want it to be 
used for other purposes...



Comment at: clang-tools-extra/clangd/Preamble.cpp:278
+  // This shouldn't coincide with any real file name.
+  PP.PatchFileName = llvm::formatv("{0}_preamble_patch.h", FileName);
+

I'm slightly nervous about incorporating the filename itself, not sure why but 
it feels unneccesary.
WDYT about "dir/__preamble_patch__.h"?



Comment at: clang-tools-extra/clangd/Preamble.cpp:281
+  // We are only interested in newly added includes.
+  llvm::StringSet<> ExistingIncludes;
+  for (const auto &Inc : Preamble.LexedIncludes)

Why not a DenseSet>?
(The copies probably don't matter, but I think it'd be a bit clearer and more 
typesafe)



Comment at: clang-tools-extra/clangd/Preamble.cpp:313
+  PPOpts.addRemappedFile(PatchFileName, PatchBuffer.release());
+  PPOpts.Includes.push_back(PatchFileName);
+}

worth a comment indicating exactly where this is going to be inserted into the 
parsing stream.



Comment at: clang-tools-extra/clangd/Preamble.h:72
   CanonicalIncludes CanonIncludes;
+  /// Contains all of the includes spelled in the preamble section, even the
+  /// ones in disabled regions.

Maybe "Used as a baseline for creating a PreamblePatch"?
Since this is a bit unusual.



Comment at: clang-tools-extra/clangd/Preamble.h:74
+  /// ones in disabled regions.
+  std::vector LexedIncludes;
 };

Since computing these inclusions seems to be cheap (you've disabled all the 
IO), would it be clearer and more flexible to store the preamble text as a 
string and compute both the baseline & new includes at the same time?
I'm thinking if other preamble constructs (macros?) are added, needing to put 
more data structures in PreambleData, where if you store the text only those 
can be private details.

(In fact the preamble text is already stored in PrecompiledPreamble, you could 
just expose it there)





Comment at: clang-tools-extra/clangd/Preamble.h:98
+/// Stores information required to use an existing preamble with different file
+/// contents.
+class PreamblePatch {

This is probably a good place for a high-level overview of the concept, and 
maybe an example.

In particular, *something* should mention that this is approximate, and what we 
use it for/don't use it for.



Comment at: clang-tools-extra/clangd/Preamble.h:101
+public:
+  PreamblePatch() = default;
+  /// Builds a patch that contains new PP directives introduced to the preamble

// With an empty patch, the preamble is used verbatim.



Comment at: clang-tools-extra/clangd/Preamble.h:107
+  static PreamblePatch create(llvm::StringRef FileName, const ParseInputs &PI,
+  const PreambleData &Preamble);
+  /// Inserts an artifical include to the \p CI that contains new directives

nit: it'd be good to give these "version"s names. e.g. PI -> Modified, Preamble 
-> Baseline. And maybe mention these in the class description?



Comment at: clang-tools-extra/clangd/Preamble.h:108
+  const PreambleData &Preamble);
+  /// Inserts an artifical include to the \p CI that contains new directives
+  /// calculated with create.

This exp

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


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

https://reviews.llvm.org/D73846



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

[PATCH] D78323: [clang] Fix invalid comparator in tablegen

[Matt Kulukundis via Phabricator via cfe-commits]

fowles added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78323



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 258605.
llunak retitled this revision from "PerformPendingInstatiations() already in 
the PCH" to "Add option to instantiate templates already in the PCH".
llunak edited the summary of this revision.
llunak added a comment.

Changed to use -fpch-instantiate-templates to control the feature.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/codegen.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid updated this revision to Diff 258608.
zbrid added a comment.

- Remove dfsan feature based on review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390

Files:
  clang/docs/DataFlowSanitizer.rst
  libcxx/CMakeLists.txt
  libcxx/utils/libcxx/test/config.py
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/CMake.rst


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()
Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()
Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, ``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/Handle

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: mikhail.ramalho, NoQ, Szelethus, baloghadamsoftware, 
xazax.hun.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a project: clang.

`FalsePositiveRefutationBRVisitor` had a bug(*) where the constraints were not 
properly collected thus crosschecked with Z3.
This patch fixes that bug by eliminating the `FalsePositiveRefutationBRVisitor` 
completely.

As turned out we don't even need a `BugReporterVisitor` for doing the 
crosscheck.
We should simply use the constraints of the `ErrorNode` since that has all the 
necessary information.

Bug(*):
The visitor collected all the constraints on a `BugPath`.
Since it is a visitor, it visited the node before the `ErrorNode`, and so on 
until the `RootNode`.
At the end visited the ErrorNode explicitly, but that visit had no effect.
Since the constraints were collected into a map, mapping each symbol to its 
`RangeSet`.
If the map already had a mapping with the symbol, then it was skipped.

Consider the following case:
We already had a constraint on a symbol, but az the end in the ErrorNode we 
have a stricter constraint on that.
This visitor would not utilize that stricter constraint in the crosschecking.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78457

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2809,74 +2809,6 @@
   return nullptr;
 }
 
-//===--===//
-// Implementation of FalsePositiveRefutationBRVisitor.
-//===--===//
-
-FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor()
-: Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {}
-
-void FalsePositiveRefutationBRVisitor::finalizeVisitor(
-BugReporterContext &BRC, const ExplodedNode *EndPathNode,
-PathSensitiveBugReport &BR) {
-  // Collect new constraints
-  VisitNode(EndPathNode, BRC, BR);
-
-  // Create a refutation manager
-  llvm::SMTSolverRef RefutationSolver = llvm::CreateZ3Solver();
-  ASTContext &Ctx = BRC.getASTContext();
-
-  // Add constraints to the solver
-  for (const auto &I : Constraints) {
-const SymbolRef Sym = I.first;
-auto RangeIt = I.second.begin();
-
-llvm::SMTExprRef Constraints = SMTConv::getRangeExpr(
-RefutationSolver, Ctx, Sym, RangeIt->From(), RangeIt->To(),
-/*InRange=*/true);
-while ((++RangeIt) != I.second.end()) {
-  Constraints = RefutationSolver->mkOr(
-  Constraints, SMTConv::getRangeExpr(RefutationSolver, Ctx, Sym,
- RangeIt->From(), RangeIt->To(),
- /*InRange=*/true));
-}
-
-RefutationSolver->addConstraint(Constraints);
-  }
-
-  // And check for satisfiability
-  Optional isSat = RefutationSolver->check();
-  if (!isSat.hasValue())
-return;
-
-  if (!isSat.getValue())
-BR.markInvalid("Infeasible constraints", EndPathNode->getLocationContext());
-}
-
-PathDiagnosticPieceRef FalsePositiveRefutationBRVisitor::VisitNode(
-const ExplodedNode *N, BugReporterContext &, PathSensitiveBugReport &) {
-  // Collect new constraints
-  const ConstraintRangeTy &NewCs = N->getState()->get();
-  ConstraintRangeTy::Factory &CF =
-  N->getState()->get_context();
-
-  // Add constraints if we don't have them yet
-  for (auto const &C : NewCs) {
-const SymbolRef &Sym = C.first;
-if (!Constraints.contains(Sym)) {
-  Constraints = CF.add(Constraints, Sym, C.second);
-}
-  }
-
-  return nullptr;
-}
-
-void FalsePositiveRefutationBRVisitor::Profile(
-llvm::FoldingSetNodeID &ID) const {
-  static int Tag = 0;
-  ID.AddPointer(&Tag);
-}
-
 //===--===//
 // Implementation of TagVisitor.
 //===--===//
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -38,6 +38,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/Static

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[Kristóf Umann via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:152-158
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
   assert(Desc && Desc->StreamArgNo != ArgNone &&
  "Try to get a non-existing stream argument.");
   return Call.getArgSVal(Desc->StreamArgNo);
 }

If we add methods to `FnDescription`, we might as well add this as well, but I 
don't insist.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:222
 {StreamState::FEof, StreamState::FError, StreamState::NoError}}},
-  {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+  // Note: ftell sets errno only.
+  {{"ftell", 1},

C'99 standard [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf | 
§7.19.9.4.3]], about the `ftell function`:

> If successful, the `ftell` function returns the current value of the file 
> position indicator for the stream. On failure, the `ftell` function returns 
> `-1L` and stores an implementation-defined positive value in `errno`.

So we need an evalCall for this.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:485
+State = bindInt(0, State, C, CE);
+State = State->set(StreamSym, StreamState::getOpened(Desc));
+C.addTransition(State);

Isn't the state change redundant? We have a `preCall` to this function and we 
assert this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374



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

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:222
 {StreamState::FEof, StreamState::FError, StreamState::NoError}}},
-  {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+  // Note: ftell sets errno only.
+  {{"ftell", 1},

Szelethus wrote:
> C'99 standard [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf | 
> §7.19.9.4.3]], about the `ftell function`:
> 
> > If successful, the `ftell` function returns the current value of the file 
> > position indicator for the stream. On failure, the `ftell` function returns 
> > `-1L` and stores an implementation-defined positive value in `errno`.
> 
> So we need an evalCall for this.
I mean, this function can only fail if the stream itself is an erroneous or 
closed state, right? So we can associate the -1 return value with the stream 
being in that, and the other with the stream being opened.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

[Marcus Johnson via Phabricator via cfe-commits]

MarcusJohnson91 updated this revision to Diff 258612.
MarcusJohnson91 edited the summary of this revision.

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

https://reviews.llvm.org/D75791

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2509,10 +2509,43 @@
Style);
   verifyFormat("extern \"C\"\n"
"{\n"
-   "  int foo();\n"
+   "int foo();\n"
"}",
Style);
 }
+  
+TEST_F(FormatTest, IndentExternBlockStyle) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentWidth = 2;
+  
+  Style.IndentExternBlock = FormatStyle::IEBS_Indent;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
+   "  int foo1();\n"
+   "}", Style);
+  
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
+   "int foo2();\n"
+   "}", Style);
+  
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\"\n{\n"
+   "  int foo3();\n"
+   "}", Style);
+  
+  Style.BraceWrapping.AfterExternBlock = false;
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
+   "int foo4();\n"
+   "}", Style);
+}
 
 TEST_F(FormatTest, FormatsInlineASM) {
   verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));");
@@ -13262,6 +13295,13 @@
   CHECK_PARSE("AllowShortIfStatementsOnASingleLine: true",
   AllowShortIfStatementsOnASingleLine,
   FormatStyle::SIS_WithoutElse);
+  
+  Style.IndentExternBlock = FormatStyle.IEBS_Indent;
+  CHECK_PARSE("IndentExternBlock: AfterExternBlock", IndentExternBlock, FormatStyle::IEBS_AfterExternBlock);
+  CHECK_PARSE("IndentExternBlock: Indent", IndentExternBlock, FormatStyle::IEBS_Indent);
+  CHECK_PARSE("IndentExternBlock: NoIndent", IndentExternBlock, FormatStyle::IEBS_NoIndent);
+  CHECK_PARSE("IndentExternBlock: true", IndentExternBlock, FormatStyle::IEBS_Indent);
+  CHECK_PARSE("IndentExternBlock: false", IndentExternBlock, FormatStyle::IEBS_NoIndent);
 
   // FIXME: This is required because parsing a configuration simply overwrites
   // the first N elements of the list instead of resetting it.
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1112,11 +1112,11 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterExternBlock) {
+if (Style.IndentExternBlock == FormatStyle::IEBS_AfterExternBlock && Style.BraceWrapping.AfterExternBlock) {
   addUnwrappedLine();
-  parseBlock(/*MustBeDeclaration=*/true);
+  parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/Style.BraceWrapping.AfterExternBlock == true ? true : false);
 } else {
-  parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
+  parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/Style.IndentExternBlock == FormatStyle::IEBS_Indent ? true : false);
 }
 addUnwrappedLine();
 return;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -202,6 +202,18 @@
 IO.enumCase(Value, "Always", FormatStyle::BWACS_Always);
   }
 };
+  
+template <>
+struct ScalarEnumerationTraits {
+  static void
+  enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) {
+IO.enumCase(Value, "AfterExternBlock", FormatStyle::IEBS_AfterExternBlock);
+IO.enumCase(Value, "Indent", FormatStyle::IEBS_Indent);
+IO.enumCase(Value, "NoIndent", FormatStyle::IEBS_NoIndent);
+IO.enumCase(Value, "true", FormatStyle::IEBS_Indent);
+IO.enumCase(Value, "false", FormatStyle::IEBS_NoIndent);
+  }
+};
 
 template <>
 struct ScalarEnumerationTraits {
@@ -494,6 +506,7 @@
 IO.mapOptional("IndentWidth", Style.IndentWidth);
 IO.mapOptional("IndentWrappedFunctionNames",
Style.IndentWrappedFunctionNames);
+IO.mapOptional("IndentExternBlock", Style.IndentExternBlock);
 IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
 IO.mapOptional("Jav

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

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

xazax.hun added a comment.

> As turned out we don't even need a BugReporterVisitor for doing the 
> crosscheck.
>  We should simply use the constraints of the ErrorNode since that has all the 
> necessary information.

This should not be true. But we should definitely have a test case that proves 
this. The main idea is that unused symbols can be garbage collected. The 
problem is that the ErrorNode does not have any information about the symbols 
that were garbage collected earlier. This is why we need the visitor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 258613.
rdwampler added a comment.

changed to check it overrides a virtual function in a base class and not just 
virtual.


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

https://reviews.llvm.org/D78444

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class BaseA { };
+class DerivedA;
+
+class BaseB {
+virtual operator BaseA&() = 0;
+virtual operator DerivedA&() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+virtual operator BaseA&(); // OK. Overrides BaseB::operatorBaseA&()
+virtual operator DerivedA&(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+virtual operator DerivedB&(); // expected-warning{{conversion function 
converting 'DerivedB' to itself will never be used}}
+virtual operator BaseA&(); // expected-warning{{conversion function 
converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if(Conversion->size_overridden_methods() != 0)
+/* Suppress diagnostics for overriding virtual function in a base class. 
*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10713,9 +10713,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10743,6 +10740,9 @@
   if (Method->isStatic())
 checkThisInStaticMemberFunctionType(Method);
 }
+
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
 
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class BaseA { };
+class DerivedA;
+
+class BaseB {
+virtual operator BaseA&() = 0;
+virtual operator DerivedA&() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+virtual operator BaseA&(); // OK. Overrides BaseB::operatorBaseA&()
+virtual operator DerivedA&(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+virtual operator DerivedB&(); // expected-warning{{conversion function converting 'DerivedB' to itself will never be used}}
+virtual operator BaseA&(); // expected-warning{{conversion function converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

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

Looks OK as a workaround. Do you know why we consider these to be in the main 
file? If we could fix that in the source manager, that'd seem preferable.


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

https://reviews.llvm.org/D73846



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

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D78457#1991288 , @xazax.hun wrote:

> > As turned out we don't even need a BugReporterVisitor for doing the 
> > crosscheck.
> >  We should simply use the constraints of the ErrorNode since that has all 
> > the necessary information.
>
> This should not be true. But we should definitely have a test case that 
> proves this. The main idea is that unused symbols can be garbage collected. 
> The problem is that the ErrorNode does not have any information about the 
> symbols that were garbage collected earlier. This is why we need the visitor.


You might be right. Could you give a short example to a garbage-collected 
symbol?
Either way, we need to fix the bug which I stated earlier.

I tried to create a unit-test for the bug, but I miserably failed.
To call the `visitor::finalizeVisitor` you would need almost the entire world.  
(A `ExprEngine`, a `CompilerInstance`, etc.)
I also considered a `lit-test`, though I really doubt that is the right tool 
for testing this.
Since we are constantly improving the checkers, we should not bake in a 
false-positive bugreport which is then invalidated by the Z3 visitor...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D77938: [clangd] Extend YAML Serialization

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Done, thanks for the contributions! 
Feel free to apply for commit access at any point if you plan to work more on 
clangd or other LLVM stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77938



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

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 258627.
aaronpuchert added a comment.

Remove loop in TreeTransform::TransformLambdaExpr and make sure 
Sema::SubstParmVarDecl handles the situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76038

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/vartemplate-lambda.cpp
  clang/test/SemaTemplate/instantiate-local-class.cpp

Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -486,3 +486,14 @@
   }
   void g() { f(); }
 }
+
+namespace PR45000 {
+  template 
+  void f(int x = [](T x = nullptr) -> int { return x; }());
+  // expected-error@-1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'}}
+  // expected-note@-2 {{in instantiation of default function argument expression for 'operator()'}}
+  // expected-note@-3 {{passing argument to parameter 'x' here}}
+
+  void g() { f(); }
+  // expected-note@-1 {{in instantiation of default function argument expression for 'f'}}
+}
Index: clang/test/SemaCXX/vartemplate-lambda.cpp
===
--- clang/test/SemaCXX/vartemplate-lambda.cpp
+++ clang/test/SemaCXX/vartemplate-lambda.cpp
@@ -4,7 +4,12 @@
 template  void foo0() { fn0(); }
 
 template auto fn1 = [](auto a) { return a + T(1); };
-template auto v1 = [](int a = T(1)) { return a; }();
+template auto v1 = [](int a = T()) { return a; }();
+// expected-error@-1{{cannot initialize a parameter of type 'int' with an rvalue of type 'int *'}}
+// expected-error@-2{{no matching function for call}}
+// expected-note@-3{{passing argument to parameter 'a' here}}
+// expected-note@-4{{candidate function not viable}}
+// expected-note@-5{{conversion candidate of type 'int (*)(int)'}}
 
 struct S {
   template
@@ -16,6 +21,7 @@
   X a = 0x61;
   fn1(a);
   (void)v1;
+  (void)v1; // expected-note{{in instantiation of variable template specialization 'v1' requested here}}
   (void)S::t; // expected-note{{in instantiation of static data member 'S::t' requested here}}
   return 0;
 }
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -12159,19 +12159,6 @@
 
   LSI->CallOperator = NewCallOperator;
 
-  for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
-   I != NumParams; ++I) {
-auto *P = NewCallOperator->getParamDecl(I);
-if (P->hasUninstantiatedDefaultArg()) {
-  EnterExpressionEvaluationContext Eval(
-  getSema(),
-  Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, P);
-  ExprResult R = getDerived().TransformExpr(
-  E->getCallOperator()->getParamDecl(I)->getDefaultArg());
-  P->setDefaultArg(R.get());
-}
-  }
-
   getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
   getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4359,7 +4359,7 @@
 EPI.ExceptionSpec.Type != EST_None &&
 EPI.ExceptionSpec.Type != EST_DynamicNone &&
 EPI.ExceptionSpec.Type != EST_BasicNoexcept &&
-!Tmpl->isLexicallyWithinFunctionOrMethod()) {
+!Tmpl->isInLocalScope()) {
   FunctionDecl *ExceptionSpecTemplate = Tmpl;
   if (EPI.ExceptionSpec.Type == EST_Uninstantiated)
 ExceptionSpecTemplate = EPI.ExceptionSpec.SourceTemplate;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2380,7 +2380,7 @@
 UnparsedDefaultArgInstantiations[OldParm].push_back(NewParm);
   } else if (Expr *Arg = OldParm->getDefaultArg()) {
 FunctionDecl *OwningFunc = cast(OldParm->getDeclContext());
-if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
+if (OwningFunc->isInLocalScope()) {
   // Instantiate default arguments for methods of local classes (DR1484)
   // and non-defining declarations.
   Sema::ContextRAII SavedContext(*this, OwningFunc);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -332,13 +332,16 @@
   }
 }
 
-bool Decl::isLexicallyWithinFunctionOrMethod() const {
+bo

[PATCH] D77221: [AVR] Rework MCU family detection to support more AVR MCUs

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

Great! I was considering writing something like this but this patch is much 
better than what I had in mind.

Note that this might conflict with D78117 .


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

https://reviews.llvm.org/D77221



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

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added subscribers: ahatanak, sepavloff.
aaronpuchert added a comment.

Adding @ahatanak and @sepavloff since I'm effectively reverting D23096 
 now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76038



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

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.

This seems to be in line with our current behavior, so I hope it's right:

  struct S { S(int); };
  
  template
  auto l = [](T x = T()) { return x; };
  
  template
  struct Fun {
  T operator()(T x = T()) const { return x; }
  };
  
  void f() {
  l();   // Ok.
  l(S(0)); // Error at "T()": no matching constructor for initialization 
of 'S'
  Fun f;
  f(S(0));// Ok.
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76038



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

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 258628.
aaronpuchert edited the summary of this revision.
aaronpuchert added a comment.

Slightly adapt test results: there is an additional error now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76038

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/vartemplate-lambda.cpp
  clang/test/SemaTemplate/instantiate-local-class.cpp

Index: clang/test/SemaTemplate/instantiate-local-class.cpp
===
--- clang/test/SemaTemplate/instantiate-local-class.cpp
+++ clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -486,3 +486,16 @@
   }
   void g() { f(); }
 }
+
+namespace PR45000 {
+  template 
+  void f(int x = [](T x = nullptr) -> int { return x; }());
+  // expected-error@-1 {{cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'}}
+  // expected-note@-2 {{passing argument to parameter 'x' here}}
+  // expected-error@-3 {{no matching function for call}}
+  // expected-note@-4 {{candidate function not viable: requires single argument 'x', but no arguments were provided}}
+  // expected-note@-5 {{conversion candidate of type 'auto (*)(int) -> int'}}
+
+  void g() { f(); }
+  // expected-note@-1 {{in instantiation of default function argument expression for 'f' required here}}
+}
Index: clang/test/SemaCXX/vartemplate-lambda.cpp
===
--- clang/test/SemaCXX/vartemplate-lambda.cpp
+++ clang/test/SemaCXX/vartemplate-lambda.cpp
@@ -4,7 +4,12 @@
 template  void foo0() { fn0(); }
 
 template auto fn1 = [](auto a) { return a + T(1); };
-template auto v1 = [](int a = T(1)) { return a; }();
+template auto v1 = [](int a = T()) { return a; }();
+// expected-error@-1{{cannot initialize a parameter of type 'int' with an rvalue of type 'int *'}}
+// expected-error@-2{{no matching function for call}}
+// expected-note@-3{{passing argument to parameter 'a' here}}
+// expected-note@-4{{candidate function not viable}}
+// expected-note@-5{{conversion candidate of type 'int (*)(int)'}}
 
 struct S {
   template
@@ -16,6 +21,7 @@
   X a = 0x61;
   fn1(a);
   (void)v1;
+  (void)v1; // expected-note{{in instantiation of variable template specialization 'v1' requested here}}
   (void)S::t; // expected-note{{in instantiation of static data member 'S::t' requested here}}
   return 0;
 }
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -12159,19 +12159,6 @@
 
   LSI->CallOperator = NewCallOperator;
 
-  for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
-   I != NumParams; ++I) {
-auto *P = NewCallOperator->getParamDecl(I);
-if (P->hasUninstantiatedDefaultArg()) {
-  EnterExpressionEvaluationContext Eval(
-  getSema(),
-  Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, P);
-  ExprResult R = getDerived().TransformExpr(
-  E->getCallOperator()->getParamDecl(I)->getDefaultArg());
-  P->setDefaultArg(R.get());
-}
-  }
-
   getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
   getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4359,7 +4359,7 @@
 EPI.ExceptionSpec.Type != EST_None &&
 EPI.ExceptionSpec.Type != EST_DynamicNone &&
 EPI.ExceptionSpec.Type != EST_BasicNoexcept &&
-!Tmpl->isLexicallyWithinFunctionOrMethod()) {
+!Tmpl->isInLocalScope()) {
   FunctionDecl *ExceptionSpecTemplate = Tmpl;
   if (EPI.ExceptionSpec.Type == EST_Uninstantiated)
 ExceptionSpecTemplate = EPI.ExceptionSpec.SourceTemplate;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2380,7 +2380,7 @@
 UnparsedDefaultArgInstantiations[OldParm].push_back(NewParm);
   } else if (Expr *Arg = OldParm->getDefaultArg()) {
 FunctionDecl *OwningFunc = cast(OldParm->getDeclContext());
-if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
+if (OwningFunc->isInLocalScope()) {
   // Instantiate default arguments for methods of local classes (DR1484)
   // and non-defining declarations.
   Sema::ContextRAII SavedContext(*this, OwningFunc);
Index: clang/lib/AST/DeclBase.cpp
==

[PATCH] D78429: [clangd] Metric tracking through Tracer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for tackling this! Mostly API stuff, obviously :-)




Comment at: clang-tools-extra/clangd/Trace.h:35
+/// itself to active tracer on destruction.
+struct Metric {
+  enum Type {

Conceptually there's a difference between a metric (the thing being measured, 
such as "latency distribution by method") and the measurement (such as "code 
completion took 90ms this time").
The attributes such as name, type, permitted labels are associated with the 
metric - these are schema. Whereas the measurement is a tuple `(metric, actual 
label, value)` - it doesn't make sense to have measurements for the same metric 
with different types. And the metric is immutable while the measurement is 
transient.

For the API (to emit data) it seems natural to model this split using constexpr 
objects for the metrics, and methods for the measurement:
```
constexpr Metric IncomingLatencyMS("/clangd/latency", Distribution);
...
void handleCodeCompletion() {
  ...
  IncomingLatencyMS.record(90);
}
```
(IOW I think our favorite metrics framework got this right)


For the SPI (to consume data), things are a bit muddier. For I think we want to 
avoid having a protocol to explicitly register metrics, so there's an 
attraction to just reporting the flat `(name, type, label, value)` tuple. I 
also quite like the idea of reusing the type above and reporting this as 
`(const Metric&, label, value)` which makes extending Metric more natural.



Comment at: clang-tools-extra/clangd/Trace.h:36
+struct Metric {
+  enum Type {
+/// Occurence of an event, e.g. cache access.

I think we're missing an important type: a value directly provided by the 
measurement, such as memory usage.



Comment at: clang-tools-extra/clangd/Trace.h:36
+struct Metric {
+  enum Type {
+/// Occurence of an event, e.g. cache access.

sammccall wrote:
> I think we're missing an important type: a value directly provided by the 
> measurement, such as memory usage.
This enum is about defining the relationship between the metric and the 
measurements, but also about the (related) nature of the metric itself.

The current names are measurement-centric, which is good for the former but not 
so much for the latter, and a bit confusing if we set this enum when 
initializing the *metric*. I'd consider having this be metric-centric and 
documenting the measurements instead. e.g.

```
enum MetricType {
  /// A number whose value is meaningful, and may vary over time.
  /// Each measurement replaces the current value.
  Value,

  /// An aggregate number whose rate of change over time is meaningful.
  /// Each measurement is an increment for the counter.
  Counter,

  /// A distribution of values with a meaningful mean and count.
  /// Each measured value is a sample for the distribution.
  /// The distribution is assumed not to vary, samples are aggregated over time.
  Distribution,
};
```



Comment at: clang-tools-extra/clangd/Trace.h:42
+  };
+  /// Uniquely identifies the metric.
+  const llvm::StringLiteral Name;

An example to show the naming scheme?
I'd suggest just "method_latency" or so for most things - we can add "foo/bar" 
or "foo.bar" for hierarchy if needed, but I don't think a leading slash or a 
"clangd" in the path add anything. 



Comment at: clang-tools-extra/clangd/Trace.h:46
+  /// of a cache access.
+  std::vector Labels;
+  double Value = 0;

(Not clear whether this is the allowed set of labels for a metric, bag of tags 
for a measurement, tuple of coordinates for a measurement...)

What are the design goals around labels? Can we make them as simple as possible 
(or remove them) for now?

Take the example of cache hit/miss.
There are at least two ways to model this without labels:
 - `cache.hit` and `cache.miss` counters
 - `cache_hit` distribution: measure 1 for hit, 0 for miss. Mean is the hit 
rate, count is the number of attempts.

The latter seems to capture the structure pretty well, but doesn't generalize 
(what if there are three categories?). The former seems almost equivalent to 
labels though. 

Benefits of labels:
 - part of the measurement, therefore can be dynamic (like filenames)
 - explicit that it's meaningful to aggregate across labels (naming conventions 
are OK for this too for one dimension)
 - generalizes sensibly to multiple dimensions (schema/docs become important)
 - quoting/escaping is annoying for labels with funny characters (e.g. file 
paths)
 - can bridge to other systems without knowledge of metric structure/semantics 
(this seems like a bad idea)

The first benefit seems pretty strong actually. I would suggest only supporting 
a single label though - it's a way simpler interface, we avoid complicated 
metrics whose schema might need documentation and enforcement,  and we avoid 
any hint of memory allocation in the common case (j

[PATCH] D78100: [AST] Suppress the spammy "attempt to use a deleted fucntion" diagnostic.

[Sam McCall via Phabricator via cfe-commits]

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

LG but please fix the sourcerange




Comment at: clang/lib/Sema/SemaDecl.cpp:12268
 /// of sanity.
 void Sema::ActOnInitializerError(Decl *D) {
   // Our main concern here is re-establishing invariants like "a

rsmith wrote:
> hokein wrote:
> > sammccall wrote:
> > > rsmith wrote:
> > > > Should we attach a `RecoveryExpr` initializer in this case too?
> > > Now we're really slipping into a different set of use-cases for 
> > > RecoveryExpr... 
> > > I assume we're talking about a RecoveryExpr that has no children, at 
> > > least in the short term, as parsing failures don't return the likely 
> > > subexpressions found. So it's a pure error marker in the form of an AST 
> > > node. Quite a lot of ExprError()s could become these...
> > > 
> > > Actually there's another virtue here: even without subexpressions, the 
> > > RecoveryExpr can capture the token range. So VarDecl::getSourceRange() 
> > > will include the malformed initializer, so tools can at least attribute 
> > > these tokens to the right part of the AST.
> > yeah, I'm not sure how much benefit we can get from the recovery-expr in 
> > this case, if the initializer is failed to parse, we don't have any ast 
> > nodes.
> What I'm thinking is that we should have some kind of marker in the AST to 
> track that an initializer was provided for the variable but was malformed. 
> Right now, we use the "invalid" bit for that, which means that downstream 
> code that refers to the variable will have poor recovery. If we attach an 
> initialization expression marked with the "contains errors" bit instead, then 
> we can keep the `VarDecl` valid, and improve the recovery not necessarily for 
> this node but for things downstream of it.
> 
> (I guess ultimately it seems reasonable to me to use the same AST 
> representation for "no initializer was provided and implicit default init 
> failed", "an initializer was provided but we couldn't parse / semantically 
> analyze it", and "an initializer was provided but was incompatible with the 
> variable's type" -- except that in the third case we can track the expression 
> that we formed for the initializer.)
> 
> I don't think there's any urgency to this, and having both AST models for a 
> while (in different cases) doesn't seem like it's going to cause much pain.
Yeah, I think this makes sense. @hokein: even without child AST nodes, this 
will improve SelectionTree accuracy in clangd: go to definition on some part of 
an invalid initializer currently goes to the vardecl, and after adding 
RecoveryExpr here it'll go nowhere instead which is progress.

But the expr needs to have at least the source range, so this isn't a trivial 
change --> another patch?



Comment at: clang/lib/Sema/SemaDecl.cpp:12565
+  auto RecoveryExpr =
+  CreateRecoveryExpr(Var->getLocation(), Var->getEndLoc(), {});
+  if (RecoveryExpr.get())

This seems like it's going to claim some actual tokens, when the thing it 
represents doesn't cover any tokens.

I think both start/end source locations should be invalid.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15007
+  // If initializing the variable failed, don't also diagnose problems with
+  // the desctructor, they're likely related.
+  if (VD->getInit() && VD->getInit()->containsErrors())

desctructor -> destructor
(sorry, you copied my typo)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78100



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

[PATCH] D77456: [clangd] Parse `foo` in documentation comments and render as code.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D77456#1961374 , @kadircet wrote:

> LGTM, thanks!
>
> Regarding spaces between code and text chunks, are you suggesting we should 
> print:
>
>   Tests primality of`p`
>


No, I'm complaining about the space before the period in

  Tests primality of `p` .

and the plaintext rendering too

  Tests primality of p .



> So having a raw paragraph chunk(in addition to plaintext and inline code). 
> might be a middle ground here. Plaintext renderers will keep showing the 
> documentation as it is written in the source code, including backticks, 
> whereas markdown renderers would display a more rich text. WDYT?

Two main objections to the idea of "raw":

- we're going to emit arbitrary, potentially malformed markdown into the 
markdown stream, which can have arbitrary effects/glitches. I'd rather the 
emitter always emits valid markup and thus can't lose track of the context.
- this assumes the input is markdown. I want `\c foo` to also render as 
code-font `foo` in markdown and as backtick-foo in plaintext. So what do we do 
there, generate markdown as a string and then emit it as a raw chunk? What a 
mess.

I really do think what we want is a chunk with semantics "emphasized code" that 
renders as a code span in markdown and as backtick-delimited text in plaintext. 
Thus the proposal to put an emphasis bit on the code chunk. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77456



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

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1509
+
+MCSymbol *Name = getSymbol(&F);
+if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {

Add a comment that this gives us the function descriptor symbol.



Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2146
   }
 }
 

Replicate the
```
llvm_unreachable("Unknown linkage type!");
```
here before falling off the end of a function that does not return `void`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932



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

[PATCH] D78441: Delete NaCl supportSee https://developer.chrome.com/native-client/migration

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I think someone from the current NaCl team should OK this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441



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

LLVM buildmaster will be updated and restarted soon

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted in about half hour.

Thanks

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

[PATCH] D78473: [i386] Fix bug that get __m128/__m256/__m512 with wrong alignment for variadic functions.Currently clang aligns to 16 bytes when passing __m128/__m256/__m512 vector type.However, when

[LiuChen via Phabricator via cfe-commits]

LiuChen3 created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
LiuChen3 abandoned this revision.

...alignment, including
struct, union and vector types. For struct/union, there is no probem because it 
will align
to 4 bytes when passing them. For __m128/__m256/__m512 vector type, it will get 
wrong result.

This patch will get va_arg according the rules below:

1. When the target doesn't support avx and avx512: get __m128/__m256/__m512 
from 16 bytes aligned stack.
2. When the target supports avx: get __m256/__m512 from 32 bytes aligned stack.
3. When the target supports avx512: get __m512 from 64 bytes aligned stack.

Notice: The current behavior of clang is inconsistent with i386 abi. The 
i386-abi says as below:

1. If parameters of type __m256 are required to be passed on the stack, the 
stack pointer must be aligned on a 0 mod 32 byte boundary at the time of the 
call.
2. If parameters of type __m512 are required to be passed on the stack, the 
stack pointer must be aligned on a 0 mod 64 byte boundary at the time of the 
call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78473

Files:
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/x86_32-align-linux-avx2.c
  clang/test/CodeGen/x86_32-align-linux-avx512f.c
  clang/test/CodeGen/x86_32-align-linux.c

Index: clang/test/CodeGen/x86_32-align-linux.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_32-align-linux.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -w -fblocks -ffreestanding -triple i386-pc-linux-gnu -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+#include 
+
+// CHECK-LABEL: define void @testm128
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 15
+// CHECK-NEXT:  %2 = and i32 %1, -16
+// CHECK-NEXT:  %argp.cur.aligned = inttoptr i32 %2 to i8*
+void testm128(int argCount, ...) {
+  __m128 res;
+  __builtin_va_list args;
+  __builtin_va_start(args, argCount);
+  res = __builtin_va_arg(args, __m128);
+  __builtin_va_end(args);
+}
+
+// CHECK-LABEL: define void @testm256
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 15
+// CHECK-NEXT:  %2 = and i32 %1, -16
+// CHECK-NEXT:  %argp.cur.aligned = inttoptr i32 %2 to i8*
+void testm256(int argCount, ...) {
+  __m256 res;
+  __builtin_va_list args;
+  __builtin_va_start(args, argCount);
+  res = __builtin_va_arg(args, __m256);
+  __builtin_va_end(args);
+}
+
+// CHECK-LABEL: define void @testm512
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 15
+// CHECK-NEXT:  %2 = and i32 %1, -16
+// CHECK-NEXT:  %argp.cur.aligned = inttoptr i32 %2 to i8*
+void testm512(int argCount, ...) {
+  __m512 res;
+  __builtin_va_list args;
+  __builtin_va_start(args, argCount);
+  res = __builtin_va_arg(args, __m512);
+  __builtin_va_end(args);
+}
Index: clang/test/CodeGen/x86_32-align-linux-avx512f.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_32-align-linux-avx512f.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -w -fblocks -ffreestanding -triple i386-pc-linux-gnu -target-feature +avx512f -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+#include 
+
+// CHECK-LABEL: define void @testm512
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 63
+// CHECK-NEXT:  %2 = and i32 %1, -64
+// CHECK-NEXT:  %argp.cur.aligned = inttoptr i32 %2 to i8*
+void testm512(int argCount, ...) {
+  __m512 res;
+  __builtin_va_list args;
+  __builtin_va_start(args, argCount);
+  res = __builtin_va_arg(args, __m512);
+  __builtin_va_end(args);
+}
Index: clang/test/CodeGen/x86_32-align-linux-avx2.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_32-align-linux-avx2.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -w -fblocks -ffreestanding -triple i386-pc-linux-gnu -target-feature +avx -emit-llvm -o %t %s
+// RUN: FileCheck < %t %s
+
+#include 
+
+// CHECK-LABEL: define void @testm128
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 15
+// CHECK-NEXT:  %2 = and i32 %1, -16
+// CHECK-NEXT:  %argp.cur.aligned = inttoptr i32 %2 to i8*
+void testm128(int argCount, ...) {
+  __m128 res;
+  __builtin_va_list args;
+  __builtin_va_start(args, argCount);
+  res = __builtin_va_arg(args, __m128);
+  __builtin_va_end(args);
+}
+
+// CHECK-LABEL: define void @testm256
+// CHECK-LABEL: %argp.cur = load i8*, i8** %args, align 4
+// CHECK-NEXT:  %0 = ptrtoint i8* %argp.cur to i32
+// CHECK-NEXT:  %1 = add i32 %0, 31
+// CHECK-NEXT:  

[clang-tools-extra] 3be73df - [clangd][test] Make sed git bash compliant

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-04-20T08:27:15+02:00
New Revision: 3be73dfde7bae80db4064cc7f7bf55d613d2deae

URL: 
https://github.com/llvm/llvm-project/commit/3be73dfde7bae80db4064cc7f7bf55d613d2deae
DIFF: 
https://github.com/llvm/llvm-project/commit/3be73dfde7bae80db4064cc7f7bf55d613d2deae.diff

LOG: [clangd][test] Make sed git bash compliant

Added: 


Modified: 
clang-tools-extra/clangd/test/compile-commands-path-in-initialize.test
clang-tools-extra/clangd/test/system-include-extractor.test

Removed: 




diff  --git 
a/clang-tools-extra/clangd/test/compile-commands-path-in-initialize.test 
b/clang-tools-extra/clangd/test/compile-commands-path-in-initialize.test
index 034299df2965..8c7ea26ca94d 100644
--- a/clang-tools-extra/clangd/test/compile-commands-path-in-initialize.test
+++ b/clang-tools-extra/clangd/test/compile-commands-path-in-initialize.test
@@ -10,7 +10,7 @@
 
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
-# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+# RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
 
 # RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
 

diff  --git a/clang-tools-extra/clangd/test/system-include-extractor.test 
b/clang-tools-extra/clangd/test/system-include-extractor.test
index 74d395869ad0..03cb54069d6e 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -31,7 +31,7 @@
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
-# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+# RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
 
 # Bless the mock driver we've just created so that clangd can execute it.
 # RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test | FileCheck 
-strict-whitespace %t.test



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