[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

2020-04-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:2070
+ diag::err_pp_including_mainfile_in_preamble);
+return {ImportAction::None};
+  }

Instead of returning here I think we should set the Action to `Skip`. So that 
relevant callbacks (`InclusionDirective` and `FileSkipped`) is invoked. (maybe 
have a test for that too?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78366



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

[PATCH] D77621: ADT: SmallVector size & capacity use word-size integers when elements are small.

2020-04-18 Thread Nikita Popov via Phabricator via cfe-commits 
nikic reopened this revision.
nikic added a comment.
This revision is now accepted and ready to land.

I have reverted this change, because it causes a 1% compile-time 

 and memory usage 

 regression. The memory usage regression is probably fine given what this 
change does, but the compile-time regression is not. (For context, this pretty 
much undoes the wins that the recent removal of waymarking gave us.)

Some notes:

- Can you please split out the fix to grow() into a separate revision? It does 
not seem related to the main change, and reduces surface area.
- I don't think the automatic switch of the size/capacity field has been 
justified well. We have plenty of SmallVectors in LLVM that are, indeed, small. 
There is no way an MCRelaxableFragment will ever end up storing a single 
instruction that is 4G large.
- Similarly, I'm not really convinced about handling this in SmallVector at 
all. The original change here just used an `std::vector` in the one place where 
this has become an issue. That seems like a pretty good solution until there is 
evidence that this is really a more widespread problem.

But in any case, my primary concern here is the compile-time regression, and 
it's not immediately clear which part of the change it comes from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D78052: add_new_check.py: Update of the template to add an autofix section

2020-04-18 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru added a comment.

@alexfh ping ? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78052



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

[PATCH] D77983: clang-tidy doc: add a note for every checker with an autofix

2020-04-18 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru added a comment.

@alexfh ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77983



___
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

2020-04-18 Thread Mark Nauwelaerts via Phabricator via cfe-commits 
mnauw added a comment.

I do not have commit access, so it would be helpful that you land this (and the 
other) ;-)


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] D78404: [clang] Implement P0692R1 from C++20 (access checking on specializations)

2020-04-18 Thread Sergej Jaskiewicz via Phabricator via cfe-commits 
broadwaylamb added a comment.

In D78404#1990192 , @rsmith wrote:

> ... please also test ...
>
>   template class TemplateClass3 
> varTemplate3{};
>
>
> ... which we should diagnose, because that's a primary variable template 
> definition, not a partial / explicit specialization or explicit instantiation.


Interestingly, the latest GCC doesn't diagnose here 
, but we do. Do we need to remain compatible with 
GCC in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78404



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

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

2020-04-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 258513.
tambre added a comment.

Add cacheline size per the technical reference manual


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77940

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -974,9 +974,15 @@
   AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
   AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "carmel", "armv8.2-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_RDM,
+  "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 37;
+static constexpr unsigned NumAArch64CPUArchs = 38;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
@@ -1125,6 +1131,8 @@
AArch64::ArchKind::INVALID, "sve"));
   EXPECT_FALSE(testAArch64Extension("a64fx",
AArch64::ArchKind::INVALID, "sve2"));
+  EXPECT_TRUE(
+  testAArch64Extension("carmel", AArch64::ArchKind::INVALID, "fp16"));
 
   EXPECT_FALSE(testAArch64Extension(
   "generic", AArch64::ArchKind::ARMV8A, "ras"));
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -262,6 +262,21 @@
 )";
 
   EXPECT_EQ(sys::detail::getHostCPUNameForARM(A64FXProcCpuInfo), "a64fx");
+
+  // Verify Nvidia Carmel.
+  const std::string CarmelProcCpuInfo = R"(
+processor   : 0
+model name  : ARMv8 Processor rev 0 (v8l)
+BogoMIPS: 62.50
+Features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm dcpop
+CPU implementer : 0x4e
+CPU architecture: 8
+CPU variant : 0x0
+CPU part: 0x004
+CPU revision: 0
+)";
+
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM(CarmelProcCpuInfo), "carmel");
 }
 
 #if defined(__APPLE__) || defined(_AIX)
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -2,6 +2,7 @@
 
 
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=generic 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=carmel 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a35 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a34 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a53 2>&1 | FileCheck %s
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -44,6 +44,7 @@
 AppleA11,
 AppleA12,
 AppleA13,
+Carmel,
 CortexA35,
 CortexA53,
 CortexA55,
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -68,6 +68,9 @@
   switch (ARMProcFamily) {
   case Others:
 break;
+  case Carmel:
+CacheLineSize = 64;
+break;
   case CortexA35:
 break;
   case CortexA53:
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -597,6 +597,14 @@
   FeatureComplxNum
   ]>;
 
+def ProcCarmel : SubtargetFeature<"carmel", "ARMProcFamily", "Carmel",
+  "Nvidia Carmel processors", [
+   HasV8_2aOps,
+   FeatureNEON,
+   FeatureCrypto,
+   FeatureFullFP16
+   ]>;
+
 // Note that cyclone does not fuse AES instructions, but newer apple chips do
 // perform the fusion and cyclone is used by default when targetting apple OSes.
 def ProcAppleA7 : SubtargetFeature<"apple-a7", "ARMProcFamily", "AppleA7

[PATCH] D78252: [AArch64] FMLA/FMLS patterns improvement.

2020-04-18 Thread Dave Green via Phabricator via cfe-commits 
dmgreen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:8055
 multiclass SIMDFPIndexedTiedPatterns {
+  let Predicates = [HasNEON, HasFullFP16] in {
+  // 1 variant for the .8h version: DUPLANE from 128-bit

ilinpv wrote:
> dmgreen wrote:
> > Should we have equal patterns to those below for f32 as well? So using DUP, 
> > D vector (4xf16) and possibly from a vector_extract too.
> I'm worried about performance impact of change fmadd/sub -> fmla/ls in last 
> pattern case.
What performance impact are you worried about?



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:8077
+
+  def : Pat<(f16 (OpNode (f16 FPR16:$Rd), (f16 FPR16:$Rn),
+ (vector_extract (v8f16 V128:$Rm), 
VectorIndexS:$idx))),

Do you mean the v4f16 variant of this pattern?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78252



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

[PATCH] D78252: [AArch64] FMLA/FMLS patterns improvement.

2020-04-18 Thread Pavel Iliin via Phabricator via cfe-commits 
ilinpv marked an inline comment as not done.
ilinpv added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:8055
 multiclass SIMDFPIndexedTiedPatterns {
+  let Predicates = [HasNEON, HasFullFP16] in {
+  // 1 variant for the .8h version: DUPLANE from 128-bit

dmgreen wrote:
> ilinpv wrote:
> > dmgreen wrote:
> > > Should we have equal patterns to those below for f32 as well? So using 
> > > DUP, D vector (4xf16) and possibly from a vector_extract too.
> > I'm worried about performance impact of change fmadd/sub -> fmla/ls in last 
> > pattern case.
> What performance impact are you worried about?
I mean, can fmla/ls take more cycles that fmadd/sub, is it any performance 
improvement of such replacement?



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:8077
+
+  def : Pat<(f16 (OpNode (f16 FPR16:$Rd), (f16 FPR16:$Rn),
+ (vector_extract (v8f16 V128:$Rm), 
VectorIndexS:$idx))),

dmgreen wrote:
> Do you mean the v4f16 variant of this pattern?
This pattern exactly replaces fmadd/sub to fmla/ls, so it is questionable 
weather or not this pattern is useful.
v4f16 vector_extract variant has no any test cases at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78252



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

[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

2020-04-18 Thread Sam McCall via Phabricator via cfe-commits 
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:2070
+ diag::err_pp_including_mainfile_in_preamble);
+return {ImportAction::None};
+  }

kadircet wrote:
> Instead of returning here I think we should set the Action to `Skip`. So that 
> relevant callbacks (`InclusionDirective` and `FileSkipped`) is invoked. 
> (maybe have a test for that too?)
FileSkipped says: `Callback invoked whenever a source file is skipped as the 
result of header guard optimization.` that's not exactly what's happening here 
(if it were, Action is set to Skip above and we never enter this if statement).

As for calling InclusionDirective - it looks like some error paths call it and 
some error paths don't (returning before vs after InclusionDirective(). 
"Erasing" the illegal include seem valid, as would be skipping it. I'm not sure 
I want to change the behavior in this patch (and maybe introduce a new bug 
while fixing this one).

Is there some concrete reason we need this callback?

(As mentioned, I've got no idea how to reasonably test this in a fine-grained 
way without boiling the ocean, it was never previously tested...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78366



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

[PATCH] D78429: [clangd] Trace ASTCache accesses

2020-04-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
javed.absar, ilya-biryukov.
Herald added a project: clang.

Expose AST cache access statistics. Instead of doing this we can also
expose EventTracer::instant. Currently it is only accessible through trace::log,
which only provides plaintext information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78429

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


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -143,10 +143,14 @@
   /// 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::Span Span("ASTCache");
 std::unique_lock Lock(Mut);
 auto Existing = findByKey(K);
-if (Existing == LRU.end())
+if (Existing == LRU.end()) {
+  SPAN_ATTACH(Span, "Hit", false);
   return None;
+}
+SPAN_ATTACH(Span, "Hit", true);
 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/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -143,10 +143,14 @@
   /// 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::Span Span("ASTCache");
 std::unique_lock Lock(Mut);
 auto Existing = findByKey(K);
-if (Existing == LRU.end())
+if (Existing == LRU.end()) {
+  SPAN_ATTACH(Span, "Hit", false);
   return None;
+}
+SPAN_ATTACH(Span, "Hit", true);
 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] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

2020-04-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!




Comment at: clang/lib/Lex/PPDirectives.cpp:2070
+ diag::err_pp_including_mainfile_in_preamble);
+return {ImportAction::None};
+  }

sammccall wrote:
> kadircet wrote:
> > Instead of returning here I think we should set the Action to `Skip`. So 
> > that relevant callbacks (`InclusionDirective` and `FileSkipped`) is 
> > invoked. (maybe have a test for that too?)
> FileSkipped says: `Callback invoked whenever a source file is skipped as the 
> result of header guard optimization.` that's not exactly what's happening 
> here (if it were, Action is set to Skip above and we never enter this if 
> statement).
> 
> As for calling InclusionDirective - it looks like some error paths call it 
> and some error paths don't (returning before vs after InclusionDirective(). 
> "Erasing" the illegal include seem valid, as would be skipping it. I'm not 
> sure I want to change the behavior in this patch (and maybe introduce a new 
> bug while fixing this one).
> 
> Is there some concrete reason we need this callback?
> 
> (As mentioned, I've got no idea how to reasonably test this in a fine-grained 
> way without boiling the ocean, it was never previously tested...)
That's how most of the tools collect includes, there are some clang-tidy 
checkers and also clangd features depend on receiving that callback. In the 
absence of that the include will be totally invisible to clangd. I suppose its 
OK to not support goto/documetlink for mainfile itself, it might cause troubles 
in feature if we plan to make use of a preamble built for one file in other 
files, but we can worry about that later on.

So it is ok if you want to keep current behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78366



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

[PATCH] D76957: HIP: Merge builtin library handling

2020-04-18 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:364
+bool FastRelaxedMath = false;
+bool CorrectSqrt = false;
+

By default this was on. We should keep the old behavior.


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

https://reviews.llvm.org/D76957



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

[PATCH] D78404: [clang] Implement P0692R1 from C++20 (access checking on specializations)

2020-04-18 Thread Sergej Jaskiewicz via Phabricator via cfe-commits 
broadwaylamb updated this revision to Diff 258555.
broadwaylamb added a comment.

- Add more tests
- Allow class template member explicit specializations
- Inherit TypeAliasDecl from DeclContext (this is needed so that we could 
perform access checks when parsing 'using' declaration templates)


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

https://reviews.llvm.org/D78404

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DeclNodes.td
  clang/lib/AST/DeclBase.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/temp/temp.decls/temp.class.spec/p10.cpp
  clang/test/CXX/temp/temp.spec/p6.cpp
  clang/test/CXX/temp/temp.spec/temp.explicit/p11.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -966,7 +966,7 @@
 
   Access checking on specializations
   https://wg21.link/p0692r1";>P0692R1
-  Partial
+  Clang 11
 
 
   Default constructible and assignable stateless lambdas
Index: clang/test/CXX/temp/temp.spec/temp.explicit/p11.cpp
===
--- clang/test/CXX/temp/temp.spec/temp.explicit/p11.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
-
-class X {
-  template  class Y {};
-};
-
-class A {
-  class B {};
-  class C {};
-};
-
-// C++0x [temp.explicit] 14.7.2/11:
-//   The usual access checking rules do not apply to names used to specify
-//   explicit instantiations.
-template class X::Y;
-
-// As an extension, this rule is applied to explicit specializations as well.
-template <> class X::Y {};
Index: clang/test/CXX/temp/temp.spec/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.spec/p6.cpp
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+class X {
+  template  class Y {};
+};
+
+class A {
+  class B {};
+  class C {};
+
+  void func();
+  static void staticFunc();
+
+  // See https://llvm.org/PR37424
+  void funcOverloaded();
+  void funcOverloaded(int);
+  static void staticFuncOverloaded();
+  static void staticFuncOverloaded(int);
+
+  int field;
+};
+
+// C++20 [temp.spec] 17.8/6:
+//   The usual access checking rules do not apply to names in a declaration of
+//   an explicit instantiation or explicit specialization, with the exception
+//   of names appearing in a function body, default argument, base-clause,
+//   member-specification, enumerator-list, or static data member or variable
+//   template initializer.
+template class X::Y;
+
+template  class D {};
+template class D<&A::func>;
+template class D<&A::funcOverloaded>;
+
+template  class E { };
+template class E<&A::staticFunc>;
+template class E<&A::staticFuncOverloaded>;
+
+template  class G {};
+template class G<&A::field>;
+
+template <> class X::Y {};
+
+namespace member_spec {
+
+  template 
+  struct X {
+struct A {};
+void f();
+enum E : int;
+static int var;
+  };
+
+  class Y {
+using Z = int;
+  };
+
+  template <>
+  struct X::A {};
+
+  template <>
+  void X::f() {}
+
+  template <>
+  enum X::E : int {};
+
+  template <>
+  int X::var = 76;
+
+}
Index: clang/test/CXX/temp/temp.decls/temp.class.spec/p10.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.decls/temp.class.spec/p10.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// C++20 [temp.class.spec] 17.6.5/10:
+//   The usual access checking rules do not apply to non-dependent names used
+//   to specify template arguments of the simple-template-id of the partial
+//   specialization.
+
+class TestClass {
+  // expected-note@+1 4 {{declared private here}}
+  void func();
+
+  // expected-note@+1 4 {{declared private here}}
+  void funcOverloaded();
+
+  void funcOverloaded(int);
+
+  // expected-note@+1 2 {{declared private here}}
+  static void staticFunc();
+
+  // expected-note@+1 2 {{declared private here}}
+  static void staticFuncOverloaded();
+
+  static void staticFuncOverloaded(int);
+
+  // expected-note@+1 {{declared private here}}
+  class Nested {};
+
+  // expected-note@+1 {{declared private here}}
+  int field;
+};
+
+template  class TemplateClass {};
+template <> class TemplateClass<&TestClass::func> {};
+template <> class TemplateClass<&TestClass::funcOverloaded> {};
+
+// expected-error@+1 {{'func' is a private member of 'TestClass'}}
+using alias1_1 = TemplateClass<&TestClass::func>;
+
+// expected-error@+1 {{'funcOverloaded' is a private member of 'TestClass'}}
+using alias1_2 = TemplateClass<&TestClass::funcOverloaded>;
+
+template  class TemplateClass2 { };
+template <> class Temp

[PATCH] D78429: [clangd] Trace ASTCache accesses

2020-04-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet updated this revision to Diff 258559.
kadircet added a comment.

Extend tracer api to enable exporting metrics.


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,36 @@
 #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.
+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 T;
+
+  Metric(llvm::StringLiteral Name) : Name(Name) {}
+};
+
 /// 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 +69,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
@@ -69,6 +94,9 @@
 /// Records a single instant event, associated with the current thread.
 void log(const llvm::Twine &Name);
 
+/// Reports a metric to tracer. No-op if there's no active tracer.
+void record(const Metric &M);
+
 /// Records an event whose duration is the lifetime of the Span object.
 /// This lifetime is extended when the span's context is reused.
 ///
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);
 });
   }
@@ -208,6 +208,12 @@
   T->instant("Log", llvm::json::Object{{"Message", Message.str()}});
 }
 
+void record(const Metric &M) {
+  if (!T)
+return;
+  T->record(M);
+}
+
 // Returned context owns Args.
 static Context makeSpanContext(llvm::Twine Name, llvm::json::Object *Args) {
   if (!T)
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -143,10 +143,18 @@
   /// 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.T = trace::Metric::Increment;
+M.Value = 1;
+// Record metric after unlocking the mutex.
+auto _ = llvm::make_scope_exit([&M] { trace::record(M); });
 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] D78404: [clang] Implement P0692R1 from C++20 (access checking on specializations)

2020-04-18 Thread Sergej Jaskiewicz via Phabricator via cfe-commits 
broadwaylamb marked 3 inline comments as done.
broadwaylamb added inline comments.



Comment at: clang/include/clang/AST/Decl.h:3198
 /// alias-declaration.
-class TypeAliasDecl : public TypedefNameDecl {
+class TypeAliasDecl : public TypedefNameDecl, public DeclContext {
   /// The template for which this is the pattern, if any.

I'm not sure about inheriting `TypeAliasDecl` from `DeclContext`, but (see 
below)



Comment at: clang/lib/Parse/ParseTemplate.cpp:211
+Decl *Decl = usingDeclPtr.get().getSingleDecl();
+ParsingDeclRAII.complete(Decl);
+return Decl;

…but otherwise I couldn't make it print access level diagnostics for a 
particular kind of `using` declaration template (see in the next inline 
comment).

Why? Because of [this 
line](https://github.com/llvm/llvm-project/blob/8e0c9e21bf5f3e7a427b07e3eaf3bc80d2c74cb6/clang/lib/Sema/SemaAccess.cpp#L1479)
 — we need to be able to cast the `TypeAliasDecl` to `DeclContext` in order for 
delayed access check to be actually performed.



Comment at: clang/test/CXX/temp/temp.decls/temp.class.spec/p10.cpp:58
+template 
+using alias3_1 = TemplateClass3;
+

I'm talking about declarations like this.

Previously, we didn't reject it (which I believe was incorrect), and now we do.


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

https://reviews.llvm.org/D78404



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

[PATCH] D78404: [clang] Implement P0692R1 from C++20 (access checking on specializations)

2020-04-18 Thread Sergej Jaskiewicz via Phabricator via cfe-commits 
broadwaylamb marked an inline comment as done.
broadwaylamb added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:5672
+  //   initializer.
+  SuppressAccessChecks diagsFromTag(*this);
+

This is for things like

```
template<> void X::f() {}
```

not to be rejected (here `Z` is a private member of class `Y`)

I wasn't sure how to suppress it only when we're parsing template parameter 
list, so we suppress it unconditionally here. All the tests pass though, but 
I'd appreciate any hints.

Note that testing that `D.getContext() == 
DeclaratorContext::TemplateParamContext` doesn't work — when we get here, we're 
actually in a `FileContext`. 


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

https://reviews.llvm.org/D78404



___
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

2020-04-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Depends on D78429 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78436

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -396,6 +397,10 @@
   if (Err)
 return CB(std::move(Err));
 }
+trace::Metric M("/clangd/rename");
+M.T = trace::Metric::Sample;
+M.Value = Edits->size();
+trace::record(M);
 return CB(std::move(*Edits));
   };
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
@@ -438,10 +443,15 @@
 auto Filter = [&](const Tweak &T) {
   return TweakFilter(T) && !PreparedTweaks.count(T.id());
 };
+trace::Metric M("/clangd/shown_codeaction");
+M.T = trace::Metric::Increment;
+M.Value = 1;
 for (const auto &Sel : *Selections) {
   for (auto &T : prepareTweaks(*Sel, Filter)) {
 Res.push_back({T->id(), T->title(), T->intent()});
 PreparedTweaks.insert(T->id());
+M.Labels = {T->id()};
+trace::record(M);
   }
 }
 
@@ -462,6 +472,11 @@
 auto Selections = tweakSelection(Sel, *InpAST);
 if (!Selections)
   return CB(Selections.takeError());
+trace::Metric M("/clangd/executed_tweaks");
+M.T = trace::Metric::Increment;
+M.Value = 1;
+M.Labels = {TweakID, /*Error=*/""};
+auto _ = llvm::make_scope_exit([&M] { trace::record(M); });
 llvm::Optional> Effect;
 // Try each selection, take the first one that prepare()s.
 // If they all fail, Effect will hold get the last error.
@@ -483,8 +498,11 @@
 if (llvm::Error Err = reformatEdit(E, Style))
   elog("Failed to format {0}: {1}", It.first(), std::move(Err));
   }
+  return CB(std::move(*Effect));
 }
-return CB(std::move(*Effect));
+auto Err = Effect->takeError();
+M.Labels.back() = llvm::to_string(Err);
+return CB(std::move(Err));
   };
   WorkScheduler.runWithAST("ApplyTweak", File, std::move(Action));
 }


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -396,6 +397,10 @@
   if (Err)
 return CB(std::move(Err));
 }
+trace::Metric M("/clangd/rename");
+M.T = trace::Metric::Sample;
+M.Value = Edits->size();
+trace::record(M);
 return CB(std::move(*Edits));
   };
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
@@ -438,10 +443,15 @@
 auto Filter = [&](const Tweak &T) {
   return TweakFilter(T) && !PreparedTweaks.count(T.id());
 };
+trace::Metric M("/clangd/shown_codeaction");
+M.T = trace::Metric::Increment;
+M.Value = 1;
 for (const auto &Sel : *Selections) {
   for (auto &T : prepareTweaks(*Sel, Filter)) {
 Res.push_back({T->id(), T->title(), T->intent()});
 PreparedTweaks.insert(T->id());
+M.Labels = {T->id()};
+trace::record(M);
   }
 }
 
@@ -462,6 +472,11 @@
 auto Selections = tweakSelection(Sel, *InpAST);
 if (!Selections)
   return CB(Selections.takeError());
+trace::Metric M("/clangd/executed_tweaks");
+M.T = trace::Metric::Increment;
+M.Value = 1;
+M.Labels = {TweakID, /*Error=*/""};
+auto _ = llvm::make_scope_exit([&M] { trace::record(M); });
 llvm::Optional> Effect;
 // Try each selection, take the first one that prepare()s.
 // If they all fail, Effect will hold get the last error.
@@ -483,8 +498,11 @@
 if (llvm::Error Err = reformatEdit(E, Style))
   elog("Failed to format {0}: {1}", It.first(), std::move(Err));
   }
+  return CB(std::move(*Effect));
 }
-return CB(std::move(*Effect));
+auto Err = Effect->takeError();
+M.Labels.back() = llvm::to_string(Err);
+return CB(std::move(Err));
   };
   WorkScheduler.runWithAST("ApplyTweak", File, std::move(Action));
 }
___
cfe-commits mailing lis

[PATCH] D76957: HIP: Merge builtin library handling

2020-04-18 Thread Matt Arsenault via Phabricator via cfe-commits 
arsenm updated this revision to Diff 258562.
arsenm added a comment.

Switch default back for correct sqrt. Also add more checks for all of the 
linked libs, and fix duplicating wave64 logic


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

https://reviews.llvm.org/D76957

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip
  clang/test/Driver/rocm-device-libs.cl

Index: clang/test/Driver/rocm-device-libs.cl
===
--- clang/test/Driver/rocm-device-libs.cl
+++ clang/test/Driver/rocm-device-libs.cl
@@ -121,6 +121,21 @@
 
 
 
+// Test --hip-device-lib-path format
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --hip-device-lib-path=%S/Inputs/rocm-device-libs/amdgcn/bitcode \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+// Test environment variable HIP_DEVICE_LIB_PATH
+// RUN: env HIP_DEVICE_LIB_PATH=%S/Inputs/rocm-device-libs/amdgcn/bitcode %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+
+
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc"
Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -7,16 +7,16 @@
 
 // Test subtarget with flushing on by default.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:  --cuda-gpu-arch=gfx803 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
 
 // Test subtarget with flushing off by ddefault.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx900 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:  --cuda-gpu-arch=gfx900 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -25,7 +25,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -34,7 +34,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -43,7 +43,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -52,7 +52,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -61,7 +61,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -69,7 +69,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -77,7 +77,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fno-cuda-flush-denorm

[PATCH] D78404: [clang] Implement P0692R1 from C++20 (access checking on specializations)

2020-04-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

In D78404#1990461 , @broadwaylamb 
wrote:

> In D78404#1990192 , @rsmith wrote:
>
> > ... please also test ...
> >
> >   template class TemplateClass3 
> > varTemplate3{};
> >
> >
> > ... which we should diagnose, because that's a primary variable template 
> > definition, not a partial / explicit specialization or explicit 
> > instantiation.
>
>
> Interestingly, the latest GCC doesn't diagnose here 
> , but we do. Do we need to remain compatible 
> with GCC in this case?


GCC doesn't even diagnose

  class TestClass { struct X {}; };
  template TestClass::X varTemplate3_1b{};
  void use() { varTemplate3_1b = {}; }

... which is a flagrant access violation. I don't think we should be compatible 
with that, it just seems like a regular GCC bug rather than any kind of 
intentional extension.




Comment at: clang/include/clang/AST/Decl.h:3198
 /// alias-declaration.
-class TypeAliasDecl : public TypedefNameDecl {
+class TypeAliasDecl : public TypedefNameDecl, public DeclContext {
   /// The template for which this is the pattern, if any.

broadwaylamb wrote:
> I'm not sure about inheriting `TypeAliasDecl` from `DeclContext`, but (see 
> below)
Yeah, this doesn't seem appropriate. Hopefully we can find another way.



Comment at: clang/lib/Parse/ParseDecl.cpp:5672
+  //   initializer.
+  SuppressAccessChecks diagsFromTag(*this);
+

broadwaylamb wrote:
> This is for things like
> 
> ```
> template<> void X::f() {}
> ```
> 
> not to be rejected (here `Z` is a private member of class `Y`)
> 
> I wasn't sure how to suppress it only when we're parsing template parameter 
> list, so we suppress it unconditionally here. All the tests pass though, but 
> I'd appreciate any hints.
> 
> Note that testing that `D.getContext() == 
> DeclaratorContext::TemplateParamContext` doesn't work — when we get here, 
> we're actually in a `FileContext`. 
The cases that this is going to incorrectly accept will be a bit weird. For 
example:

```
struct A { void f(); };
class B { using T = A; };
void B::T::f() {}
```

We should be able to feed through information on whether we're parsing after 
`template` or `template<>` here (in which case we should suppress access) -- it 
seems reasonable to track that on the `Declarator`. One thing that seems 
unclear is whether we should suppress access here:

```
template struct A;
class X { struct Y {}; };
template<> struct A { void f(); };
void A::f() {} // suppress access here or not?
```

To get that right, we'd need to temporarily suppress access here and do a 
one-token lookahead past the scope specifier before diagnosing -- this case 
should still report an access error:

```
template struct A;
class X { struct Y {}; };
template<> struct A { int f(); };
int A::*f() {} // do not suppress access here, this is a regular 
non-template function
```

> Note that testing that `D.getContext() == 
> DeclaratorContext::TemplateParamContext` doesn't work

Right, that context indicates that we're parsing a template parameter.





Comment at: clang/test/CXX/temp/temp.decls/temp.class.spec/p10.cpp:58
+template 
+using alias3_1 = TemplateClass3;
+

broadwaylamb wrote:
> I'm talking about declarations like this.
> 
> Previously, we didn't reject it (which I believe was incorrect), and now we 
> do.
Do you know where we're suppressing the diagnostic in the first place? After we 
parse the *template-head*, we may be able to just look at the next token (to 
see if it's `using`) to determine if we should defer access checks. Making 
`TypeAliasDecl` be a `DeclContext` seems like a very heavy-handed way of 
handling this.


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

https://reviews.llvm.org/D78404



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

[PATCH] D76957: HIP: Merge builtin library handling

2020-04-18 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


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

https://reviews.llvm.org/D76957



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

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

2020-04-18 Thread Ronald Wampler via Phabricator via cfe-commits 
rdwampler created this revision.
rdwampler added a reviewer: rsmith.
rdwampler added a project: clang.
Herald added a subscriber: cfe-commits.

These warnings are grouped under '-Wclass-conversion' to be compatiable with 
GCC 9.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78442

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (71):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8537,11 +8537,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,6 +60,7 @@
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
+def ClassConversion: DiagGroup<"class-conversion">;
 def DeprecatedEnumCompareConditional :
   DiagGroup<"deprecated-enum-compare-conditional">;
 def EnumCompareConditional : DiagGroup<"enum-compare-conditional",


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (71):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8537,11 +8537,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that m

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

2020-04-18 Thread Ronald Wampler via Phabricator via cfe-commits 
rdwampler created this revision.
rdwampler added reviewers: saar.raz, aaron.ballman, doug.gregor, rsmith.
rdwampler added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

This allows for suppressing warnings about the conversion function never being 
called if it overrides a virtual function in a base class.


Repository:
  rG LLVM Github Monorepo

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,27 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class DerivedB;
+
+class BaseA {
+public:
+  virtual operator BaseA&() = 0; //OK. Virtual function 
+  virtual operator DerivedB&() = 0; //OK. Virtual function
+  virtual operator const void() = 0; //OK. Virtual function
+};
+
+class DerivedB : public BaseA {
+#if __cplusplus >= 201103L
+operator BaseA&() override; //OK. Overrides virtual function in BaseA
+operator DerivedB&() override; //OK. Overrides virtual function BaseA
+operator const void() override; //OK. Overrides virtual function BaseA
+#else
+virtual operator BaseA&(); //OK. Overrides virtual function in BaseA
+virtual operator DerivedB&(); //OK. Overrides virtual function BaseA
+virtual operator const void(); //OK. Overrides virtual function BaseA
+#endif
+};
+
 // 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->isVirtual())
+/*Suppress diagnostics if the function is virtual*/;
   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,27 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class DerivedB;
+
+class BaseA {
+public:
+  virtual operator BaseA&() = 0; //OK. Virtual function 
+  virtual operator DerivedB&() = 0; //OK. Virtual function
+  virtual operator const void() = 0; //OK. Virtual function
+};
+
+class DerivedB : public BaseA {
+#if __cplusplus >= 201103L
+operator BaseA&() override; //OK. Overrides virtual function in BaseA
+operator DerivedB&() override; //OK. Overrides virtual function BaseA
+operator const void() override; //OK. Overrides virtual function BaseA
+#else
+virtual operator BaseA&(); //OK. Overrides virtual function in BaseA
+virtual operator Derive

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

2020-04-18 Thread Dylan McKay via Phabricator via cfe-commits 
dylanmckay requested changes to this revision.
dylanmckay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Basic/Targets/AVR.h:21
 
+#include "llvm/Support/AVRTargetParser.h"
+

Order this `#include` alphabetically with the ones above it



Comment at: llvm/include/llvm/Support/AVRTargetParser.h:9
+//
+// This file implements a target parser to recognise AVR CPUs
+//

Add another sentence or two to this documentation, as this will be a new 
public-facing API that all frontends can use.



Comment at: llvm/lib/Support/AVRTargetParser.cpp:20
+// This list should be kept up-to-date with AVRDevices.td in LLVM.
+static constexpr AVR::MCUInfo AVRMcus[] = {
+{"avr1", "at90s1200", "__AVR_AT90S1200__"},

Add a comment like `FIXME: At some point, this list should removed and the 
logic directly driven from the definitions in `AVRDevices.td`.


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