[PATCH] D94287: [clang-format] Fix include sorting bug

2021-01-08 Thread Kent Sommer via Phabricator via cfe-commits
kentsommer abandoned this revision.
kentsommer added a comment.

In D94287#2486099 , @curdeius wrote:

> Seems to be a duplicate of D94206 .

Right you are! Thanks for catching that! I'll abandon this one in favor of the 
one that has already been reviewed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94287

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


[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

2021-01-08 Thread Richard Barton via Phabricator via cfe-commits
richard.barton.arm added a comment.

Could we tweak the wording to clarify that it is Clang-specific?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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


[PATCH] D94289: [clangd] Add go-to-def metric.

2021-01-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang.

to track the number of different "special" go-to-def request.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94289

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


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -335,6 +335,8 @@
   // Keep track of SymbolID -> index mapping, to fill in index data later.
   llvm::DenseMap ResultIndex;
 
+  static constexpr trace::Metric LocateASTReferentMetric(
+  "locate_ast_referent", trace::Metric::Counter, "case");
   auto AddResultDecl = [&](const NamedDecl *D) {
 D = getPreferredDecl(D);
 auto Loc =
@@ -368,8 +370,10 @@
   // saved in the AST.
   if (CMD->isPure()) {
 if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
- TouchedIdentifier->location())
+ TouchedIdentifier->location()) {
   VirtualMethods.insert(getSymbolID(CMD));
+  LocateASTReferentMetric.record(1, "go-to-overrides");
+}
   }
   // Special case: void foo() ^override: jump to the overridden method.
   const InheritableAttr *Attr = D->getAttr();
@@ -378,6 +382,7 @@
   if (Attr && TouchedIdentifier &&
   SM.getSpellingLoc(Attr->getLocation()) ==
   TouchedIdentifier->location()) {
+LocateASTReferentMetric.record(1, "go-to-overridens");
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -420,6 +425,7 @@
  ID->getName() == TouchedIdentifier->text(SM)))
   AddResultDecl(ID);
 
+LocateASTReferentMetric.record(1, "regular");
 // Otherwise the target declaration is the right one.
 AddResultDecl(D);
   }


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -335,6 +335,8 @@
   // Keep track of SymbolID -> index mapping, to fill in index data later.
   llvm::DenseMap ResultIndex;
 
+  static constexpr trace::Metric LocateASTReferentMetric(
+  "locate_ast_referent", trace::Metric::Counter, "case");
   auto AddResultDecl = [&](const NamedDecl *D) {
 D = getPreferredDecl(D);
 auto Loc =
@@ -368,8 +370,10 @@
   // saved in the AST.
   if (CMD->isPure()) {
 if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
- TouchedIdentifier->location())
+ TouchedIdentifier->location()) {
   VirtualMethods.insert(getSymbolID(CMD));
+  LocateASTReferentMetric.record(1, "go-to-overrides");
+}
   }
   // Special case: void foo() ^override: jump to the overridden method.
   const InheritableAttr *Attr = D->getAttr();
@@ -378,6 +382,7 @@
   if (Attr && TouchedIdentifier &&
   SM.getSpellingLoc(Attr->getLocation()) ==
   TouchedIdentifier->location()) {
+LocateASTReferentMetric.record(1, "go-to-overridens");
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -420,6 +425,7 @@
  ID->getName() == TouchedIdentifier->text(SM)))
   AddResultDecl(ID);
 
+LocateASTReferentMetric.record(1, "regular");
 // Otherwise the target declaration is the right one.
 AddResultDecl(D);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

2021-01-08 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

That sounds good but it's different to what this patch is trying to accomplish, 
isn't it? -ansi already doesn't get passed to the linker.
E.g. with clang 10.0.1 without this patch:

  ~ ❯ clang test.o -o test -ansi -###
  clang version 10.0.1 (Fedora 10.0.1-3.fc32)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: /usr/bin
  clang-10: warning: argument unused during compilation: '-ansi' 
[-Wunused-command-line-argument]
   "/usr/bin/ld" "--hash-style=gnu" "--build-id" "--eh-frame-hdr" "-m" 
"elf_x86_64" "-dynamic-linker" "/lib64/ld-linux-x86-64.so.2" "-o" "test" 
"/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crt1.o" 
"/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crti.o" 
"/usr/bin/../lib/gcc/x86_64-redhat-linux/10/crtbegin.o" 
"-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10" 
"-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64" 
"-L/usr/bin/../lib64" "-L/lib/../lib64" "-L/usr/lib/../lib64" 
"-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../.." "-L/usr/bin/../lib" 
"-L/lib" "-L/usr/lib" "test.o" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"/usr/bin/../lib/gcc/x86_64-redhat-linux/10/crtend.o" 
"/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crtn.o"




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

https://reviews.llvm.org/D93375

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


[PATCH] D94188: [OpenCL] Documentation for experimental C++ libraries support

2021-01-08 Thread Marco Antognini via Phabricator via cfe-commits
mantognini accepted this revision.
mantognini added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D94188

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


[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D94206#2485397 , @rjelonek wrote:

> I do not have commit access.

we are going need to know your name and email address  so we can commit on your 
behalf

  git commit --amend --author="John Doe "

https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator


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

https://reviews.llvm.org/D94206

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 315313.
serge-sans-paille added a comment.

Address some of the review


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }// expected-warning {{'foo__bar' is a reserved identifier}}
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _barbouille() { return 0; } // expected-warning {{'_barbouille' is a reserved identifier}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template  constexpr bool __toucan = true; // expected-warning {{'__toucan' is a reserved identifier}}
+
+template 
+concept _Barbotine = __toucan; // expected-warning {{'_Barbotine' is a reserved identifier}}
+
+template  // expected-warning {{'__' is a reserved identifier}}
+struct BarbeNoire {};
+
+template  // expected-warning {{'__' is a reserved identifier}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{'_Barbidur' is a reserved identifier}}
+
+struct __barbidou {}; // expected-warning {{'__barbidou' is a reserved identifier}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{'__barbouille' is a reserved identifier}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{'__babar' is a reserved identifier}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa { // expected-warning {{'__barbapapa' is a reserved identifier}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;   // expected-warning {{'_Barbalala' is a reserved identifier}}
+};
+
+enum class __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some,   // expected-warning {{'__some' is a reserved identifier}}
+  _Other// expected-warning {{'_Other' is a reserved identifier}}
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{'_Barbamama' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+// we skip this one because it's not at top-level.
+int _barbatruc; // no-warning
+}
+
+long double operator"" _BarbeBleue(long double) // no-warning
+{
+  return 0.;
+}
Index: clang/test/Sema/reserved-identifier.c
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wreserved-identifier -Wno-visibility %s
+
+#define __oof foo__ // expected-warning {{macro name is a reserved identifier}}
+
+int foo__bar() { return 0; }// no-warning
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _foo() { return 0; }// expected-warning {{'_foo' is a reserved identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+  goto __reserved;
+__reserved: // expected-warning {{'__reserved' is a reserved identifier}}
+;
+}
+
+enum __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some, // expected-warning {{'__some' is a reserved identifier}}
+  _Other, // expected-warning {{'_Other' is a reserved identifier}}
+  _other  // no-warning
+};
+
+struct __babar { // expected-warning {{'__babar' is a reserved identifier}}
+};
+
+typedef struct {
+  int _Field; // expected-warning {{'_Field' is a reserved identifier}}
+  int _field; // no-warning
+} _Typedef;   // expected-warning {{'_Typedef' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+struct _reserved { /

[clang] 8e3230f - [clang][cli] Port DiagnosticOpts to new option parsing system

2021-01-08 Thread Jan Svoboda via cfe-commits

Author: Jan Svoboda
Date: 2021-01-08T10:44:22+01:00
New Revision: 8e3230ffa3ad2994c3bbddffc3e53b3bccb2ee41

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

LOG: [clang][cli] Port DiagnosticOpts to new option parsing system

This patch introduces additional infrastructure necessary to accommodate 
DiagnosticOptions.

DiagnosticOptions are unique in that they are parsed by the same function in 
cc1 AND in the Clang driver. The call to the parsing function from the driver 
occurs early on in the compilation process, where no proper DiagnosticEngine 
exists, because the diagnostic options (passed through command line) are not 
known yet.

To preserve the current behavior, we need to be able to selectively parse:
* all options (for -cc1),
* only diagnostic options (for driver).

This patch achieves that in the following way:
* new MacroPrefix field is added to the Option TableGen class,
* new IsDiag TableGen mixin sets MacroPrefix to "DIAG_",
* TableGen backend serializes option records into a macro with the prefix,
* CompilerInvocation parse/generate methods define the 
[DIAG_]OPTION_WITH_MARSHALLING macros to handle diagnostic options separately.

Depends on D93700, D93701 & D93702.

Reviewed By: dexonsmith

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticOptions.h
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticOptions.h 
b/clang/include/clang/Basic/DiagnosticOptions.h
index 7fbe534c5994..17533b38ff5f 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.h
+++ b/clang/include/clang/Basic/DiagnosticOptions.h
@@ -15,7 +15,14 @@
 #include 
 #include 
 
+namespace llvm {
+namespace opt {
+class ArgList;
+} // namespace opt
+} // namespace llvm
+
 namespace clang {
+class DiagnosticsEngine;
 
 /// Specifies which overload candidates to display when overload
 /// resolution fails.
@@ -61,6 +68,11 @@ raw_ostream& operator<<(raw_ostream& Out, 
DiagnosticLevelMask M);
 
 /// Options for controlling the compiler diagnostics engine.
 class DiagnosticOptions : public RefCountedBase{
+  friend bool ParseDiagnosticArgs(DiagnosticOptions &, llvm::opt::ArgList &,
+  clang::DiagnosticsEngine *, bool);
+
+  friend class CompilerInvocation;
+
 public:
   enum TextDiagnosticFormat { Clang, MSVC, Vi };
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 6585fd1ceb01..3cefcc2c6654 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -241,6 +241,8 @@ def mno_mpx : Flag<["-"], "mno-mpx">, 
Group;
 def clang_ignored_gcc_optimization_f_Group : OptionGroup<
   "">, Group, 
Flags<[Ignored]>;
 
+class IsDiag { string MacroPrefix = "DIAG_"; }
+
 // A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
 // Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
 // This is useful if the option is usually disabled.
@@ -755,7 +757,7 @@ def Wp_COMMA : CommaJoined<["-"], "Wp,">,
 def Wundef_prefix_EQ : CommaJoined<["-"], "Wundef-prefix=">, 
Group,
   Flags<[CC1Option, CoreOption, HelpHidden]>, MetaVarName<"">,
   HelpText<"Enable warnings for undefined macros with a prefix in the comma 
separated list ">,
-  MarshallingInfoStringVector<"DiagnosticOpts->UndefPrefixes">;
+  MarshallingInfoStringVector<"UndefPrefixes">, IsDiag;
 def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group, 
Flags<[CC1Option, HelpHidden]>;
 def Wno_write_strings : Flag<["-"], "Wno-write-strings">, Group, 
Flags<[CC1Option, HelpHidden]>;
 def W_Joined : Joined<["-"], "W">, Group, Flags<[CC1Option, 
CoreOption]>,
@@ -1188,7 +1190,9 @@ defm borland_extensions : 
BoolFOption<"borland-extensions",
 def fbuiltin : Flag<["-"], "fbuiltin">, Group, Flags<[CoreOption]>;
 def fbuiltin_module_map : Flag <["-"], "fbuiltin-module-map">, Group,
   Flags<[NoXarchOption]>, HelpText<"Load the clang builtins module map file.">;
-defm caret_diagnostics : OptOutFFlag<"caret-diagnostics", "", "">;
+defm caret_diagnostics : BoolFOption<"caret-diagnostics",
+  "ShowCarets", DefaultsToTrue,
+  ChangedBy, ResetBy>, IsDiag;
 def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">, 
Group,
   Flags<[CC1Option]>, MetaVarName<"">, 
Values<".,latest">,
   HelpText<"Attempt to match the ABI of Clang ">;
@@ -1200,7 +1204,7 @@ def fdiagnostics_color : Flag<["-"], 
"fdiagnostics-color">, Group,
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, 
Group;
 def fansi_escape_codes : F

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

2021-01-08 Thread Jan Svoboda via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e3230ffa3ad: [clang][cli] Port DiagnosticOpts to new option 
parsing system (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84673

Files:
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -66,6 +66,7 @@
   static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
   const Record &R;
   bool ShouldAlwaysEmit;
+  StringRef MacroPrefix;
   StringRef KeyPath;
   StringRef DefaultValue;
   StringRef NormalizedValuesScope;
@@ -100,6 +101,10 @@
 
   MarshallingInfo(const Record &R) : R(R) {}
 
+  std::string getMacroName() const {
+return (MacroPrefix + MarshallingInfo::MacroName).str();
+  }
+
   void emit(raw_ostream &OS) const {
 write_cstring(OS, StringRef(getOptionSpelling(R)));
 OS << ", ";
@@ -163,6 +168,7 @@
   MarshallingInfo Ret(R);
 
   Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
+  Ret.MacroPrefix = R.getValueAsString("MacroPrefix");
   Ret.KeyPath = R.getValueAsString("KeyPath");
   Ret.DefaultValue = R.getValueAsString("DefaultValue");
   Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
@@ -424,13 +430,13 @@
 MarshallingInfos.push_back(createMarshallingInfo(*R));
 
   for (const auto &MI : MarshallingInfos) {
-OS << "#ifdef " << MarshallingInfo::MacroName << "\n";
-OS << MarshallingInfo::MacroName << "(";
+OS << "#ifdef " << MI.getMacroName() << "\n";
+OS << MI.getMacroName() << "(";
 WriteOptRecordFields(OS, MI.R);
 OS << ", ";
 MI.emit(OS);
 OS << ")\n";
-OS << "#endif // " << MarshallingInfo::MacroName << "\n";
+OS << "#endif // " << MI.getMacroName() << "\n";
   }
 
   OS << "\n";
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -97,6 +97,7 @@
   OptionGroup Group = ?;
   Option Alias = ?;
   list AliasArgs = [];
+  code MacroPrefix = "";
   code KeyPath = ?;
   code DefaultValue = ?;
   code ImpliedValue = ?;
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -680,4 +680,19 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-cl-mad-enable")));
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-menable-unsafe-fp-math")));
 }
+
+// Diagnostic option.
+
+TEST_F(CommandLineTest, DiagnosticOptionPresent) {
+  const char *Args[] = {"-verify=xyz"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_EQ(Invocation.getDiagnosticOpts().VerifyPrefixes,
+std::vector({"xyz"}));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-verify=xyz"), 1));
+}
 } // anonymous namespace
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -388,7 +388,6 @@
 DiagnosticsEngine &Diags,
 const InputArgList &Args) {
   LangOptions &LangOpts = *Invocation.getLangOpts();
-  DiagnosticOptions &DiagOpts = Invocation.getDiagnosticOpts();
   CodeGenOptions &CodeGenOpts = Invocation.getCodeGenOpts();
   TargetOptions &TargetOpts = Invocation.getTargetOpts();
   FrontendOptions &FrontendOpts = Invocation.getFrontendOpts();
@@ -402,8 +401,6 @@
   LangOpts.SpeculativeLoadHardening = CodeGenOpts.SpeculativeLoadHardening;
   LangOpts.CurrentModule = LangOpts.ModuleName;
 
-  llvm::sys::Process::UseANSIEscapeCodes(DiagOpts.UseANSIEscapeCodes);
-
   llvm::Triple T(TargetOpts.Triple);
   llvm::Triple::ArchType Arch = T.getArch();
 
@@ -1404,14 +1401,14 @@
   IMPLIED_CHECK, IMPLIED_VALUE,\
   NORMALIZER, MERGER, TABLE_INDEX) \
   if ((FLAGS)&options::CC1Option) {\
-this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  \
+KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE);  \
 if (IMPLIED_CHECK)   

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek added a comment.

"Rafał Jelonek <71409580+rjelo...@users.noreply.github.com>"


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

https://reviews.llvm.org/D94206

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


[clang] 8e3e148 - Revert "[clang][cli] Port DiagnosticOpts to new option parsing system"

2021-01-08 Thread Jan Svoboda via cfe-commits

Author: Jan Svoboda
Date: 2021-01-08T10:53:12+01:00
New Revision: 8e3e148c888e1d9d2b11721112a54a62e33a635a

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

LOG: Revert "[clang][cli] Port DiagnosticOpts to new option parsing system"

This reverts commit 8e3230ff

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticOptions.h
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticOptions.h 
b/clang/include/clang/Basic/DiagnosticOptions.h
index 17533b38ff5f..7fbe534c5994 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.h
+++ b/clang/include/clang/Basic/DiagnosticOptions.h
@@ -15,14 +15,7 @@
 #include 
 #include 
 
-namespace llvm {
-namespace opt {
-class ArgList;
-} // namespace opt
-} // namespace llvm
-
 namespace clang {
-class DiagnosticsEngine;
 
 /// Specifies which overload candidates to display when overload
 /// resolution fails.
@@ -68,11 +61,6 @@ raw_ostream& operator<<(raw_ostream& Out, 
DiagnosticLevelMask M);
 
 /// Options for controlling the compiler diagnostics engine.
 class DiagnosticOptions : public RefCountedBase{
-  friend bool ParseDiagnosticArgs(DiagnosticOptions &, llvm::opt::ArgList &,
-  clang::DiagnosticsEngine *, bool);
-
-  friend class CompilerInvocation;
-
 public:
   enum TextDiagnosticFormat { Clang, MSVC, Vi };
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3cefcc2c6654..6585fd1ceb01 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -241,8 +241,6 @@ def mno_mpx : Flag<["-"], "mno-mpx">, 
Group;
 def clang_ignored_gcc_optimization_f_Group : OptionGroup<
   "">, Group, 
Flags<[Ignored]>;
 
-class IsDiag { string MacroPrefix = "DIAG_"; }
-
 // A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
 // Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
 // This is useful if the option is usually disabled.
@@ -757,7 +755,7 @@ def Wp_COMMA : CommaJoined<["-"], "Wp,">,
 def Wundef_prefix_EQ : CommaJoined<["-"], "Wundef-prefix=">, 
Group,
   Flags<[CC1Option, CoreOption, HelpHidden]>, MetaVarName<"">,
   HelpText<"Enable warnings for undefined macros with a prefix in the comma 
separated list ">,
-  MarshallingInfoStringVector<"UndefPrefixes">, IsDiag;
+  MarshallingInfoStringVector<"DiagnosticOpts->UndefPrefixes">;
 def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group, 
Flags<[CC1Option, HelpHidden]>;
 def Wno_write_strings : Flag<["-"], "Wno-write-strings">, Group, 
Flags<[CC1Option, HelpHidden]>;
 def W_Joined : Joined<["-"], "W">, Group, Flags<[CC1Option, 
CoreOption]>,
@@ -1190,9 +1188,7 @@ defm borland_extensions : 
BoolFOption<"borland-extensions",
 def fbuiltin : Flag<["-"], "fbuiltin">, Group, Flags<[CoreOption]>;
 def fbuiltin_module_map : Flag <["-"], "fbuiltin-module-map">, Group,
   Flags<[NoXarchOption]>, HelpText<"Load the clang builtins module map file.">;
-defm caret_diagnostics : BoolFOption<"caret-diagnostics",
-  "ShowCarets", DefaultsToTrue,
-  ChangedBy, ResetBy>, IsDiag;
+defm caret_diagnostics : OptOutFFlag<"caret-diagnostics", "", "">;
 def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">, 
Group,
   Flags<[CC1Option]>, MetaVarName<"">, 
Values<".,latest">,
   HelpText<"Attempt to match the ABI of Clang ">;
@@ -1204,7 +1200,7 @@ def fdiagnostics_color : Flag<["-"], 
"fdiagnostics-color">, Group,
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, 
Group;
 def fansi_escape_codes : Flag<["-"], "fansi-escape-codes">, Group,
   Flags<[CoreOption, CC1Option]>, HelpText<"Use ANSI escape codes for 
diagnostics">,
-  MarshallingInfoFlag<"UseANSIEscapeCodes">, IsDiag;
+  MarshallingInfoFlag<"DiagnosticOpts->UseANSIEscapeCodes">;
 def fcomment_block_commands : CommaJoined<["-"], "fcomment-block-commands=">, 
Group, Flags<[CC1Option]>,
   HelpText<"Treat each comma separated argument in  as a documentation 
comment block command">,
   MetaVarName<"">, 
MarshallingInfoStringVector<"LangOpts->CommentOpts.BlockCommandNames">;
@@ -1257,16 +1253,11 @@ def fdebug_pass_structure : Flag<["-"], 
"fdebug-pass-structure">, Group
 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
 Group, Flags<[CC1Option]>;
 def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, 
Group;
-def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, 
Group,
-  Flags<[CC1Option]>, HelpText<"Do not include fixit information in 
diagnostics">,
-  MarshallingInfoNegativeFlag<"ShowFixits

[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked an inline comment as done.
serge-sans-paille added inline comments.



Comment at: clang/test/Sema/reserved-identifier.c:9
+int _foo() { return 0; }// expected-warning {{'_foo' is a reserved 
identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier

Quuxplusone wrote:
> aaron.ballman wrote:
> > Can you add a test that we do not warn on an extern declaration? e.g.,
> > ```
> > extern char *_strdup(const char *);
> > ```
> > This is sometimes used (esp in older C code bases) to avoid having to 
> > include an entire header to scrape one declaration out of it, and there are 
> > popular libraries (like the MSVC CRT) that have APIs starting with a single 
> > underscore and lowercase letter.
> > 
> > The idea here is: if it's an extern declaration, the identifier is "owned" 
> > by some other declaration and this is not likely something the user has 
> > control over.
> Should that logic also apply to a forward declaration like `struct _foo;`? 
> Should it apply to `struct _foo { int i; };`? These are also things the user 
> might not have control over. (And they're equally things that the user 
> //could// pull out into a separate .h file surrounded by disabled-warning 
> pragmas, if they really wanted to.)
I'd say 100% yeah to the forward declaration, but I'm unsure about the actual 
definition, because there's no way to distinguish it from a user definition.



Comment at: clang/test/Sema/reserved-identifier.c:24
+  _Other, // expected-warning {{'_Other' is a reserved identifier}}
+  _other  // no-warning
+};

aaron.ballman wrote:
> I'm on the fence about whether this should have no warning or not. 
> Enumeration constants in C (and sometimes in C++, depending on the 
> enumeration) are in the enclosing scope. e.g.,
> ```
> enum foo {
>   _bar
> };
> 
> int i = _bar;
> ```
> So if a user has an enumeration constant named `_bar`, the implementation is 
> not free to add `int _bar(void);` as it will cause compile errors. WDYT?
We could state it that way: would the code still compile if there's a `int 
_bar` symbol defined in a system header. here the answer is no, so reserved.


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

https://reviews.llvm.org/D93095

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


[PATCH] D92749: [clangd] go-to-implementation on a base class jumps to all subclasses.

2021-01-08 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 accepted this revision.
usaxena95 added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92749

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


[PATCH] D94290: [clang][AArch64][SVE] Avoid going through memory for coerced VLST return values

2021-01-08 Thread Joe Ellis via Phabricator via cfe-commits
joechrisellis created this revision.
joechrisellis added reviewers: c-rhodes, bsmith, peterwaller-arm.
Herald added subscribers: NickHung, psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
joechrisellis requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

VLST return values are coerced to VLATs in the function epilog for
consistency with the VLAT ABI. Previously, this coercion was done
through memory. It is preferable to use the
llvm.experimental.vector.insert intrinsic to avoid going through memory
here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94290

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c

Index: clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
@@ -17,7 +17,6 @@
 // CHECK-NEXT:[[PRED_ADDR:%.*]] = alloca , align 2
 // CHECK-NEXT:[[VEC_ADDR:%.*]] = alloca , align 16
 // CHECK-NEXT:[[PG:%.*]] = alloca , align 2
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store  [[PRED:%.*]], * [[PRED_ADDR]], align 2
 // CHECK-NEXT:store  [[VEC:%.*]], * [[VEC_ADDR]], align 16
 // CHECK-NEXT:[[TMP0:%.*]] = load , * [[PRED_ADDR]], align 2
@@ -35,11 +34,9 @@
 // CHECK-NEXT:[[TMP10:%.*]] = call  @llvm.aarch64.sve.add.nxv4i32( [[TMP9]],  [[CASTSCALABLESVE]],  [[TMP8]])
 // CHECK-NEXT:[[CASTFIXEDSVE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[TMP10]], i64 0)
 // CHECK-NEXT:store <16 x i32> [[CASTFIXEDSVE]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP11:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP12:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP11]], i8* align 16 [[TMP12]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP13:%.*]] = load , * [[RETVAL_COERCE]], align 16
-// CHECK-NEXT:ret  [[TMP13]]
+// CHECK-NEXT:[[TMP11:%.*]] = load <16 x i32>, <16 x i32>* [[RETVAL]], align 16
+// CHECK-NEXT:[[CASTSCALABLESVE1:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TMP11]], i64 0)
+// CHECK-NEXT:ret  [[CASTSCALABLESVE1]]
 //
 fixed_int32_t foo(svbool_t pred, svint32_t vec) {
   svbool_t pg = svand_z(pred, global_pred, global_pred);
@@ -50,16 +47,13 @@
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[RETVAL:%.*]] = alloca <16 x i32>, align 16
 // CHECK-NEXT:[[GLOBAL_VEC_PTR:%.*]] = alloca <16 x i32>*, align 8
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store <16 x i32>* @global_vec, <16 x i32>** [[GLOBAL_VEC_PTR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load <16 x i32>*, <16 x i32>** [[GLOBAL_VEC_PTR]], align 8
 // CHECK-NEXT:[[TMP1:%.*]] = load <16 x i32>, <16 x i32>* [[TMP0]], align 16
 // CHECK-NEXT:store <16 x i32> [[TMP1]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP2:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP2]], i8* align 16 [[TMP3]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP4:%.*]] = load , * [[RETVAL_COERCE]], align 16
-// CHECK-NEXT:ret  [[TMP4]]
+// CHECK-NEXT:[[TMP2:%.*]] = load <16 x i32>, <16 x i32>* [[RETVAL]], align 16
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TMP2]], i64 0)
+// CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 fixed_int32_t test_ptr_to_global() {
   fixed_int32_t *global_vec_ptr;
@@ -73,17 +67,14 @@
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[RETVAL:%.*]] = alloca <16 x i32>, align 16
 // CHECK-NEXT:[[ARR_ADDR:%.*]] = alloca <16 x i32>*, align 8
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store <16 x i32>* [[ARR:%.*]], <16 x i32>** [[ARR_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load <16 x i32>*, <16 x i32>** [[ARR_ADDR]], align 8
 // CHECK-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds <16 x i32>, <16 x i32>* [[TMP0]], i64 0
 // CHECK-NEXT:[[TMP1:%.*]] = load <16 x i32>, <16 x i32>* [[ARRAYIDX]], align 16
 // CHECK-NEXT:store <16 x i32> [[TMP1]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP2:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP2]], i8* align 16 [[TMP3]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP4:%.*]] = load , * [[RETVAL_COERCE]], 

[PATCH] D94162: [PowerPC] Add variants of 64-bit vector types for vec_sel.

2021-01-08 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

Kind of bizarre that we missed these, thanks for adding them. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94162

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


[PATCH] D94290: [clang][AArch64][SVE] Avoid going through memory for coerced VLST return values

2021-01-08 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

I've left one minor nit but looks otherwise looks fine to me




Comment at: clang/lib/CodeGen/CGCall.cpp:1273
+if (auto *FixedSrc =
+dyn_cast(Src.getElementType())) {
+  if (ScalableDst->getElementType() == FixedSrc->getElementType()) {

nit: `s/Src.getElementType()/SrcTy`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94290

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


[PATCH] D94290: [clang][AArch64][SVE] Avoid going through memory for coerced VLST return values

2021-01-08 Thread Joe Ellis via Phabricator via cfe-commits
joechrisellis updated this revision to Diff 315339.
joechrisellis added a comment.

Address @c-rhodes's comment.

- Use `SrcTy` instead of `Src.getElementType()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94290

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c

Index: clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
@@ -17,7 +17,6 @@
 // CHECK-NEXT:[[PRED_ADDR:%.*]] = alloca , align 2
 // CHECK-NEXT:[[VEC_ADDR:%.*]] = alloca , align 16
 // CHECK-NEXT:[[PG:%.*]] = alloca , align 2
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store  [[PRED:%.*]], * [[PRED_ADDR]], align 2
 // CHECK-NEXT:store  [[VEC:%.*]], * [[VEC_ADDR]], align 16
 // CHECK-NEXT:[[TMP0:%.*]] = load , * [[PRED_ADDR]], align 2
@@ -35,11 +34,9 @@
 // CHECK-NEXT:[[TMP10:%.*]] = call  @llvm.aarch64.sve.add.nxv4i32( [[TMP9]],  [[CASTSCALABLESVE]],  [[TMP8]])
 // CHECK-NEXT:[[CASTFIXEDSVE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[TMP10]], i64 0)
 // CHECK-NEXT:store <16 x i32> [[CASTFIXEDSVE]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP11:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP12:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP11]], i8* align 16 [[TMP12]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP13:%.*]] = load , * [[RETVAL_COERCE]], align 16
-// CHECK-NEXT:ret  [[TMP13]]
+// CHECK-NEXT:[[TMP11:%.*]] = load <16 x i32>, <16 x i32>* [[RETVAL]], align 16
+// CHECK-NEXT:[[CASTSCALABLESVE1:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TMP11]], i64 0)
+// CHECK-NEXT:ret  [[CASTSCALABLESVE1]]
 //
 fixed_int32_t foo(svbool_t pred, svint32_t vec) {
   svbool_t pg = svand_z(pred, global_pred, global_pred);
@@ -50,16 +47,13 @@
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[RETVAL:%.*]] = alloca <16 x i32>, align 16
 // CHECK-NEXT:[[GLOBAL_VEC_PTR:%.*]] = alloca <16 x i32>*, align 8
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store <16 x i32>* @global_vec, <16 x i32>** [[GLOBAL_VEC_PTR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load <16 x i32>*, <16 x i32>** [[GLOBAL_VEC_PTR]], align 8
 // CHECK-NEXT:[[TMP1:%.*]] = load <16 x i32>, <16 x i32>* [[TMP0]], align 16
 // CHECK-NEXT:store <16 x i32> [[TMP1]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP2:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP2]], i8* align 16 [[TMP3]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP4:%.*]] = load , * [[RETVAL_COERCE]], align 16
-// CHECK-NEXT:ret  [[TMP4]]
+// CHECK-NEXT:[[TMP2:%.*]] = load <16 x i32>, <16 x i32>* [[RETVAL]], align 16
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TMP2]], i64 0)
+// CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 fixed_int32_t test_ptr_to_global() {
   fixed_int32_t *global_vec_ptr;
@@ -73,17 +67,14 @@
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[RETVAL:%.*]] = alloca <16 x i32>, align 16
 // CHECK-NEXT:[[ARR_ADDR:%.*]] = alloca <16 x i32>*, align 8
-// CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
 // CHECK-NEXT:store <16 x i32>* [[ARR:%.*]], <16 x i32>** [[ARR_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load <16 x i32>*, <16 x i32>** [[ARR_ADDR]], align 8
 // CHECK-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds <16 x i32>, <16 x i32>* [[TMP0]], i64 0
 // CHECK-NEXT:[[TMP1:%.*]] = load <16 x i32>, <16 x i32>* [[ARRAYIDX]], align 16
 // CHECK-NEXT:store <16 x i32> [[TMP1]], <16 x i32>* [[RETVAL]], align 16
-// CHECK-NEXT:[[TMP2:%.*]] = bitcast * [[RETVAL_COERCE]] to i8*
-// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i32>* [[RETVAL]] to i8*
-// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[TMP2]], i8* align 16 [[TMP3]], i64 64, i1 false)
-// CHECK-NEXT:[[TMP4:%.*]] = load , * [[RETVAL_COERCE]], align 16
-// CHECK-NEXT:ret  [[TMP4]]
+// CHECK-NEXT:[[TMP2:%.*]] = load <16 x i32>, <16 x i32>* [[RETVAL]], align 16
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TMP2]], i64 0)
+// CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 fixed_int32_t array_arg(fixed_int32_t arr[])

[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
kuhnel requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94293

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -524,6 +524,15 @@
 std::function getMemoryCleanupFunction() { return nullptr; }
 #endif
 
+opt IndexSTL{
+"index-stl",
+cat(Features),
+desc("Background index should always include the STL headers even if \n"
+ "not used at the moment. This will enable code completion and \n"
+ "include fixer."),
+init(true),
+};
+
 #if CLANGD_ENABLE_REMOTE
 opt RemoteIndexAddress{
 "remote-index-address",
@@ -897,6 +906,7 @@
 
   // Shall we allow to customize the file limit?
   Opts.Rename.AllowCrossFile = CrossFileRename;
+  Opts.IndexSTL = IndexSTL;
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -141,6 +141,7 @@
 std::function ContextProvider = nullptr;
 // Whether to collect references to main-file-only symbols.
 bool CollectMainFileRefs = false;
+bool IndexSTL = false;
   };
 
   /// Creates a new background index and starts its threads.
@@ -155,6 +156,8 @@
   // available sometime later.
   void enqueue(const std::vector &ChangedFiles) {
 Queue.push(changedFilesTask(ChangedFiles));
+// TODO(kuhnel): Only index if enqueueing C++ file.
+indexSTLHeaders(ChangedFiles);
   }
 
   /// Boosts priority of indexing related to Path.
@@ -176,6 +179,8 @@
 
   void profile(MemoryTree &MT) const;
 
+  void indexSTLHeaders(const std::vector &ChangedFiles);
+
 private:
   /// Represents the state of a single file when indexing was performed.
   struct ShardVersion {
@@ -220,6 +225,8 @@
   BackgroundQueue Queue;
   AsyncTaskRunner ThreadPool;
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
+  // Whether the STL header files need to be indexed
+  bool NeedToIndexSTLHeaders = true;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -103,7 +104,8 @@
   CommandsChanged(
   CDB.watch([&](const std::vector &ChangedFiles) {
 enqueue(ChangedFiles);
-  })) {
+  })),
+  NeedToIndexSTLHeaders(Opts.IndexSTL) {
   assert(Opts.ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
   for (unsigned I = 0; I < Opts.ThreadPoolSize; ++I) {
@@ -420,5 +422,56 @@
   // We don't want to mix memory used by index and symbols, so call base class.
   MT.child("index").addUsage(SwapIndex::estimateMemoryUsage());
 }
+
+void BackgroundIndex::indexSTLHeaders(
+const std::vector &ChangedFiles) {
+  // Only index if we need to.
+  if (!NeedToIndexSTLHeaders)
+return;
+
+  vlog("Trying to find place for STL header include.");
+  std::string SourceRoot;
+  for (auto ChangedFile : ChangedFiles) {
+if (CDB.getProjectInfo(ChangedFile)) {
+  SourceRoot = CDB.getProjectInfo(ChangedFile)->SourceRoot;
+  break;
+}
+  }
+  // fallback if we can't find a Compilations Database
+  if (SourceRoot.empty()) {
+if (ChangedFiles.empty()) {
+  vlog("Could not find place for STL header include.");
+  return;
+}
+SourceRoot = llvm::sys::path::parent_path(ChangedFiles[0]).str();
+  }
+  // TODO(kuhnel): is the a better place to store this file?
+  // TODO(kuhnel): do we need a file at all, can we just pass a string to the
+  //   indexer?
+  const Path STLHeaderPath = SourceRoot + "/.stl_header.h";
+  vlog("Indexing STL headers from {0}", STLHeaderPath);
+  std::ofstream STLIndexFile;
+  std::set Headers;
+
+// gather all the known STL headers, without duplicates
+#define SYMBOL(Name, NameSpace, Header) Headers.insert(#Header);
+#include "StdSymbolMap.inc"
+#undef SYMBOL
+
+  // TODO(kuhnel): figure out if there are other ways to write to files in llvm.
+  STLIndexFile.open(STLHeaderPath);
+  STLIndexFile << "/* This is a temporary file created by c

[clang] 38d18d9 - [SVE] Add support to vectorize_width loop pragma for scalable vectors

2021-01-08 Thread David Sherwood via cfe-commits

Author: David Sherwood
Date: 2021-01-08T11:37:27Z
New Revision: 38d18d93534d290d045bbbfa86337e70f1139dc2

URL: 
https://github.com/llvm/llvm-project/commit/38d18d93534d290d045bbbfa86337e70f1139dc2
DIFF: 
https://github.com/llvm/llvm-project/commit/38d18d93534d290d045bbbfa86337e70f1139dc2.diff

LOG: [SVE] Add support to vectorize_width loop pragma for scalable vectors

This patch adds support for two new variants of the vectorize_width
pragma:

1. vectorize_width(X[, fixed|scalable]) where an optional second
parameter is passed to the vectorize_width pragma, which indicates if
the user wishes to use fixed width or scalable vectorization. For
example the user can now write something like:

  #pragma clang loop vectorize_width(4, fixed)
or
  #pragma clang loop vectorize_width(4, scalable)

In the absence of a second parameter it is assumed the user wants
fixed width vectorization, in order to maintain compatibility with
existing code.
2. vectorize_width(fixed|scalable) where the width is left unspecified,
but the user hints what type of vectorization they prefer, either
fixed width or scalable.

I have implemented this by making use of the LLVM loop hint attribute:

  llvm.loop.vectorize.scalable.enable

Tests were added to

  clang/test/CodeGenCXX/pragma-loop.cpp

for both the 'fixed' and 'scalable' optional parameter.

See this thread for context: 
http://lists.llvm.org/pipermail/cfe-dev/2020-November/067262.html

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/lib/AST/AttrImpl.cpp
clang/lib/CodeGen/CGLoopInfo.cpp
clang/lib/CodeGen/CGLoopInfo.h
clang/lib/Parse/ParsePragma.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/AST/ast-print-pragmas.cpp
clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
clang/test/CodeGenCXX/pragma-loop.cpp
clang/test/Parser/pragma-loop.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 0c01a2bbc52b..6fa6c55b15fc 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3107,8 +3107,18 @@ manually enable vectorization or interleaving.
 ...
   }
 
-The vector width is specified by ``vectorize_width(_value_)`` and the 
interleave
-count is specified by ``interleave_count(_value_)``, where
+The vector width is specified by
+``vectorize_width(_value_[, fixed|scalable])``, where _value_ is a positive
+integer and the type of vectorization can be specified with an optional
+second parameter. The default for the second parameter is 'fixed' and
+refers to fixed width vectorization, whereas 'scalable' indicates the
+compiler should use scalable vectors instead. Another use of vectorize_width
+is ``vectorize_width(fixed|scalable)`` where the user can hint at the type
+of vectorization to use without specifying the exact width. In both variants
+of the pragma the vectorizer may decide to fall back on fixed width
+vectorization if the target does not support scalable vectors.
+
+The interleave count is specified by ``interleave_count(_value_)``, where
 _value_ is a positive integer. This is useful for specifying the optimal
 width/count of the set of target architectures supported by your application.
 

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index b84e6a14f371..248409946123 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3375,8 +3375,10 @@ def LoopHint : Attr {
"PipelineDisabled", "PipelineInitiationInterval", 
"Distribute",
"VectorizePredicate"]>,
   EnumArgument<"State", "LoopHintState",
-   ["enable", "disable", "numeric", "assume_safety", 
"full"],
-   ["Enable", "Disable", "Numeric", "AssumeSafety", 
"Full"]>,
+   ["enable", "disable", "numeric", "fixed_width",
+"scalable_width", "assume_safety", "full"],
+   ["Enable", "Disable", "Numeric", "FixedWidth",
+"ScalableWidth", "AssumeSafety", "Full"]>,
   ExprArgument<"Value">];
 
   let AdditionalMembers = [{

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 8f78bbfc4e70..0ed80a481e78 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1396,6 +1396,12 @@ def err_pragma_loop_invalid_option : Error<
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
   "vectorize_width, interleave, interleave_count, unroll, unroll_count, "
   "pipeline, pipeline_initiation_interval, vectorize_predicate, or 

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

2021-01-08 Thread David Sherwood via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG38d18d93534d: [SVE] Add support to vectorize_width loop 
pragma for scalable vectors (authored by david-arm).

Changed prior to commit:
  https://reviews.llvm.org/D89031?vs=315121&id=315341#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89031

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/AST/AttrImpl.cpp
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp
  clang/test/Parser/pragma-loop.cpp

Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -60,7 +60,8 @@
 
 template 
 void test_nontype_template_badarg(int *List, int Length) {
-  /* expected-error {{use of undeclared identifier 'Vec'}} */ #pragma clang loop vectorize_width(Vec) interleave_count(I)
+  /* expected-error {{use of undeclared identifier 'Vec'}} */ #pragma clang loop vectorize_width(Vec) interleave_count(I) /*
+ expected-note {{vectorize_width loop hint malformed; use vectorize_width(X, fixed) or vectorize_width(X, scalable) where X is an integer, or vectorize_width('fixed' or 'scalable')}} */
   /* expected-error {{use of undeclared identifier 'Int'}} */ #pragma clang loop vectorize_width(V) interleave_count(Int)
   for (int i = 0; i < Length; i++) {
 List[i] = i;
@@ -74,6 +75,11 @@
   for (int i = 0; i < Length; i++) {
 List[i] = i;
   }
+
+  /* expected-error {{invalid value '-1'; must be positive}} */ #pragma clang loop vectorize_width(Value, fixed)
+  for (int i = 0; i < Length; i++) {
+List[i] = i;
+  }
 }
 
 void test(int *List, int Length) {
@@ -189,12 +195,15 @@
 /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1 +) 1
 /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1) +1
 const int VV = 4;
-/* expected-error {{expected expression}} */ #pragma clang loop vectorize_width(VV +/ 2)
-/* expected-error {{use of undeclared identifier 'undefined'}} */ #pragma clang loop vectorize_width(VV+undefined)
+/* expected-error {{expected expression}} */ #pragma clang loop vectorize_width(VV +/ 2) /*
+   expected-note {{vectorize_width loop hint malformed; use vectorize_width(X, fixed) or vectorize_width(X, scalable) where X is an integer, or vectorize_width('fixed' or 'scalable')}} */
+/* expected-error {{use of undeclared identifier 'undefined'}} */ #pragma clang loop vectorize_width(VV+undefined) /*
+   expected-note {{vectorize_width loop hint malformed; use vectorize_width(X, fixed) or vectorize_width(X, scalable) where X is an integer, or vectorize_width('fixed' or 'scalable')}} */
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize_width(1+(^*/2 * ()
 /* expected-warning {{extra tokens at end of '#pragma clang loop' - ignored}} */ #pragma clang loop vectorize_width(1+(-0[0]))
 
-/* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop vectorize_width(badvalue)
+/* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop vectorize_width(badvalue) /*
+   expected-note {{vectorize_width loop hint malformed; use vectorize_width(X, fixed) or vectorize_width(X, scalable) where X is an integer, or vectorize_width('fixed' or 'scalable')}} */
 /* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop interleave_count(badvalue)
 /* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop unroll_count(badvalue)
   while (i-6 < Length) {
@@ -215,7 +224,7 @@
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop interleave(*)
 /* expected-error {{invalid argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll(=)
 /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute(+)
-/* expected-error {{type name requires a specifier or qualifier}} expected-error {{expected expression}} */ #pragma clang loop vectorize_width(^)
+/* expected-error {{type name requires a specifier or qualifier}} expected-error {{expected expression}} */ #pragma clang loop vectorize_width(^) /* expected-note {{vectorize_width loop hint malformed; use vectorize_width(X, fixed) or vectorize_width(X, scalable) where X is an integer, or vectorize_width('fixed' or 'scalable')}} */
 /* expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop interleave_count(/)
 /* 

[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

@sammccall here the first implementation we talked about yesterday. This works 
well on my testing poject that has a compile_commands.json. It does not work 
without one.

Also I'm not really happy with the design as I haven't really found a good 
place to store the generated header file.

Should we look into implementing a separate Index to only handle the STL?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

(not sure if you were looking for comments yet, but i was just passing by and 
it was a small-ish patch, so couldn't resist :D)




Comment at: clang-tools-extra/clangd/index/Background.cpp:449
+  // TODO(kuhnel): is the a better place to store this file?
+  // TODO(kuhnel): do we need a file at all, can we just pass a string to the
+  //   indexer?

In theory BackgroundIndex only reads headers from the FS, so we can provide an 
in-memory buffer as the main file itself. `prepareCompilerInstance` in 
`Compiler.h` (and used by `BackgroundIndex::index` does that exactly).

It might be better to just have a separate endpoint in BackgroundIndex (as 
`indexSTLHeaders`) that is invoked once at construction time that enqueues 
indexing of this phantom file.

WDYT?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
 
+opt IndexSTL{
+"index-stl",

do we really need to hide this behind a flag ? it sounds like a quite useful 
feature to me with minimal risk of regressions and unlikely to make anyone 
upset. in the end we are not doing anything heuristically and just throwing 
clang on some STL headers, that'll probably be included within the TUs 
eventually.

there's always the risk of crashing while parsing some STL headers, but i don't 
think it is any different than user just including the header manually.

the biggest problem i can see is people using custom STL headers, but hopefully 
compile flags interpolation logic should be able to infer the relevant location 
for those. and in the cases it fails we either index the default STL and 
suggest people some symbols their implementation might lack, or fail to find 
STL at all and print some logs for the missing includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

I like what you did with tests. And thank you for the patch.
Before accepting this formally, I'd like to see the premerge tests pass. How do 
you upload the diff? Do you use arcanist?


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

https://reviews.llvm.org/D94217

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


[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

2021-01-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Frontend/PrecompiledPreamble.h:108
   bool CanReuse(const CompilerInvocation &Invocation,
-const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds 
Bounds,
-llvm::vfs::FileSystem *VFS) const;
+const llvm::MemoryBufferRef &MainFileBuffer,
+PreambleBounds Bounds, llvm::vfs::FileSystem &VFS) const;

dexonsmith wrote:
> jansvoboda11 wrote:
> > dexonsmith wrote:
> > > kadircet wrote:
> > > > why not accept a value directly here? (i.e. drop const and ref)
> > > Ah, yes, I've done this a few times, and it still seems not quite right. 
> > > But the alternative also doesn't feel right when it's not necessary:
> > > ```
> > > #include "llvm/Basic/MemoryBufferRef.h"
> > > ```
> > > I'm happy either way since that file is fairly careful to avoid bloating 
> > > includes.
> > I agree this looks a bit odd, but avoiding an unnecessary include seems 
> > like a good excuse.
> @kadircet , WDYT?
sorry i was on vacation and just got the chance to get back to this.

I don't feel so bad about the include, as the header itself is small-ish and 
only includes StringRef.h, which is already included by this header. So I would 
lean towards accepting this by value and keeping the API clean, rather than 
trying to get away with a forward declaration.

but definitely up to you, i don't feel strongly about it in any case. (as you 
can easily make the argument of header for MemoryBufferRef getting bloated over 
time)


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

https://reviews.llvm.org/D91297

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


[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek added a comment.

no, i upload diff file


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

https://reviews.llvm.org/D94217

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


[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

Yes, I was looking for feedback on this one as I wasn't happy with my design.




Comment at: clang-tools-extra/clangd/index/Background.cpp:449
+  // TODO(kuhnel): is the a better place to store this file?
+  // TODO(kuhnel): do we need a file at all, can we just pass a string to the
+  //   indexer?

kadircet wrote:
> In theory BackgroundIndex only reads headers from the FS, so we can provide 
> an in-memory buffer as the main file itself. `prepareCompilerInstance` in 
> `Compiler.h` (and used by `BackgroundIndex::index` does that exactly).
> 
> It might be better to just have a separate endpoint in BackgroundIndex (as 
> `indexSTLHeaders`) that is invoked once at construction time that enqueues 
> indexing of this phantom file.
> 
> WDYT?
Agree to both. I'll take a look.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
 
+opt IndexSTL{
+"index-stl",

kadircet wrote:
> do we really need to hide this behind a flag ? it sounds like a quite useful 
> feature to me with minimal risk of regressions and unlikely to make anyone 
> upset. in the end we are not doing anything heuristically and just throwing 
> clang on some STL headers, that'll probably be included within the TUs 
> eventually.
> 
> there's always the risk of crashing while parsing some STL headers, but i 
> don't think it is any different than user just including the header manually.
> 
> the biggest problem i can see is people using custom STL headers, but 
> hopefully compile flags interpolation logic should be able to infer the 
> relevant location for those. and in the cases it fails we either index the 
> default STL and suggest people some symbols their implementation might lack, 
> or fail to find STL at all and print some logs for the missing includes.
Back when I was developing embedded software, we couldn't use the STL for 
various reasons (object size, no exceptions, no dynamic memory, ...) and had 
our own, proprietary base library. In such a scenario it would be annoying if 
clangd would be proposing things I could not use in the project. 

So we should have a way for users to disable this. I'm fine if we switch it on 
by default and offer a `--disable-stl-index` flag instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[clang-tools-extra] b83b7d0 - [clangd] NFC, avoid potential ODR violation.

2021-01-08 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2021-01-08T13:29:11+01:00
New Revision: b83b7d08730e2b67d911653f59091b1b311c0213

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

LOG: [clangd] NFC, avoid potential ODR violation.

Added: 


Modified: 
clang-tools-extra/clangd/index/Merge.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/Merge.cpp 
b/clang-tools-extra/clangd/index/Merge.cpp
index 29174ff0d354..0dd0d9e01518 100644
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -164,7 +164,7 @@ void MergedIndex::relations(
 
 // Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). 
If
 // neither is preferred, this returns false.
-bool prefer(const SymbolLocation &L, const SymbolLocation &R) {
+static bool prefer(const SymbolLocation &L, const SymbolLocation &R) {
   if (!L)
 return false;
   if (!R)



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


[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

2021-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D92715#2446479 , @craig.topper 
wrote:

> I think it would be possible to add a new attribute to define these types. 
> But only specific values for parameters would be allowed so that it could 
> only generate the exact types we see here and the future segment load 
> patches. It wouldn't be general purpose like vector_size since the backend 
> design has a contract about how to translate the LMUL value from the scalable 
> type.
>
> This would introduce a new type in the AST or at least new subtype of 
> VectorType. This new type would need to be checked for in multiple places in 
> the compiler to define its behavior. As a datapoint 
> VectorType::SveFixedLengthDataVector appears 20 times. It's not clear that's 
> a reduction in complexity versus following what was already done with 
> AArch64SVEACLETypes.def. But I'm not a frontend expert.

FWIW, when I went to introduce the predecessor to _ExtInt, @rsmith was very 
against using an attribute spelling and wanted a declaration keyword (which is 
partly how _ExtInt ended up how it was).

I don't think it is controversial to be a new AST type however.


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

https://reviews.llvm.org/D92715

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


[clang-tools-extra] c909512 - [clangd] Cleanup a remaining Optional usage, NFC.

2021-01-08 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2021-01-08T13:44:20+01:00
New Revision: c909512fdb9ed63081b0578410430117811b86e8

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

LOG: [clangd] Cleanup a remaining Optional usage, NFC.

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index ebdbd56c286a..1a8474f6c53d 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -828,9 +828,9 @@ struct CompletionRecorder : public CodeCompleteConsumer {
 };
 
 struct ScoredSignature {
-  // When set, requires documentation to be requested from the index with this
-  // ID.
-  llvm::Optional IDForDoc;
+  // When not null, requires documentation to be requested from the index with
+  // this ID.
+  SymbolID IDForDoc;
   SignatureInformation Signature;
   SignatureQualitySignals Quality;
 };
@@ -894,7 +894,7 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
   for (const auto &S : ScoredSignatures) {
 if (!S.IDForDoc)
   continue;
-IndexRequest.IDs.insert(*S.IDForDoc);
+IndexRequest.IDs.insert(S.IDForDoc);
   }
   Index->lookup(IndexRequest, [&](const Symbol &S) {
 if (!S.Documentation.empty())
@@ -939,7 +939,7 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
 
 for (auto &SS : ScoredSignatures) {
   auto IndexDocIt =
-  SS.IDForDoc ? FetchedDocs.find(*SS.IDForDoc) : FetchedDocs.end();
+  SS.IDForDoc ? FetchedDocs.find(SS.IDForDoc) : FetchedDocs.end();
   if (IndexDocIt != FetchedDocs.end())
 SS.Signature.documentation = IndexDocIt->second;
 



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


[PATCH] D93942: [OpenCL] Improve online documentation.

2021-01-08 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added inline comments.



Comment at: clang/docs/UsersManual.rst:2832
+Note that if compiled to bitcode for generic targets such as SPIR,
+portable IR is produced that it can be used with various vendor
+tools as well as open source tools such as `SPIRV-LLVM Translator




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

https://reviews.llvm.org/D93942

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


[clang-tools-extra] ed3b1f9 - [clangd] go-to-implementation on a base class jumps to all subclasses.

2021-01-08 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2021-01-08T13:50:57+01:00
New Revision: ed3b1f906115a0dcd2542fa294d0382a42eb177d

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

LOG: [clangd] go-to-implementation on a base class jumps to all subclasses.

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index c7ec401c6479..ebe03e74a4a7 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -292,13 +292,14 @@ const NamedDecl *getPreferredDecl(const NamedDecl *D) {
   return D;
 }
 
-std::vector findOverrides(llvm::DenseSet IDs,
- const SymbolIndex *Index,
- llvm::StringRef MainFilePath) {
+std::vector findImplementors(llvm::DenseSet IDs,
+RelationKind Predicate,
+const SymbolIndex *Index,
+llvm::StringRef MainFilePath) {
   if (IDs.empty())
 return {};
   RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Predicate = Predicate;
   Req.Subjects = std::move(IDs);
   std::vector Results;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
@@ -458,7 +459,8 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
 });
   }
 
-  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  auto Overrides = findImplementors(VirtualMethods, RelationKind::OverriddenBy,
+Index, MainFilePath);
   Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
@@ -1239,12 +1241,20 @@ std::vector 
findImplementations(ParsedAST &AST, Position Pos,
   }
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  llvm::DenseSet VirtualMethods;
-  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
-if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
-  if (CXXMD->isVirtual())
-VirtualMethods.insert(getSymbolID(ND));
-  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
+  llvm::DenseSet IDs;
+  RelationKind QueryKind;
+  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations)) {
+if (const auto *CXXMD = llvm::dyn_cast(ND)) {
+  if (CXXMD->isVirtual()) {
+IDs.insert(getSymbolID(ND));
+QueryKind = RelationKind::OverriddenBy;
+  }
+} else if (const auto *RD = dyn_cast(ND)) {
+  IDs.insert(getSymbolID(RD));
+  QueryKind = RelationKind::BaseOf;
+}
+  }
+  return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,

diff  --git a/clang-tools-extra/clangd/XRefs.h 
b/clang-tools-extra/clangd/XRefs.h
index eca174f59096..4c745eb9bc51 100644
--- a/clang-tools-extra/clangd/XRefs.h
+++ b/clang-tools-extra/clangd/XRefs.h
@@ -83,7 +83,9 @@ struct ReferencesResult {
   bool HasMore = false;
 };
 
-/// Returns implementations of the virtual function at a specified \p Pos.
+/// Returns implementations at a specified \p Pos:
+///   - overrides for a virtual method;
+///   - subclasses for a base class;
 std::vector findImplementations(ParsedAST &AST, Position Pos,
const SymbolIndex *Index);
 

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 26e06dad9250..508634ec908a 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1611,11 +1611,11 @@ TEST(LocateSymbol, NearbyIdentifier) {
 
 TEST(FindImplementations, Inheritance) {
   llvm::StringRef Test = R"cpp(
-struct Base {
+struct $0^Base {
   virtual void F$1^oo();
   void C$4^oncrete();
 };
-struct Child1 : Base {
+struct $0[[Child1]] : Base {
   void $1[[Fo$3^o]]() override;
   virtual void B$2^ar();
   void Concrete();  // No implementations for concrete methods.
@@ -1625,7 +1625,7 @@ TEST(FindImplementations, Inheritance) {
   void $2[[Bar]]() override;
 };
 void FromReference() {
-  Base* B;
+  $0^Base* B;
   B->Fo$1^o();
   B->C$4^oncrete();
   &Base::Fo$1^o;
@@ -1633,12 +1633,16 @@ TEST(FindImplementations, Inheritance) {
   C1->B$2^ar();
   C1->Fo$3^o();
 }
+// CRTP should work.
+template
+struct $5^TemplateBas

[PATCH] D92749: [clangd] go-to-implementation on a base class jumps to all subclasses.

2021-01-08 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGed3b1f906115: [clangd] go-to-implementation on a base class 
jumps to all subclasses. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92749

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  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
@@ -1611,11 +1611,11 @@
 
 TEST(FindImplementations, Inheritance) {
   llvm::StringRef Test = R"cpp(
-struct Base {
+struct $0^Base {
   virtual void F$1^oo();
   void C$4^oncrete();
 };
-struct Child1 : Base {
+struct $0[[Child1]] : Base {
   void $1[[Fo$3^o]]() override;
   virtual void B$2^ar();
   void Concrete();  // No implementations for concrete methods.
@@ -1625,7 +1625,7 @@
   void $2[[Bar]]() override;
 };
 void FromReference() {
-  Base* B;
+  $0^Base* B;
   B->Fo$1^o();
   B->C$4^oncrete();
   &Base::Fo$1^o;
@@ -1633,12 +1633,16 @@
   C1->B$2^ar();
   C1->Fo$3^o();
 }
+// CRTP should work.
+template
+struct $5^TemplateBase {};
+struct $5[[Child3]] : public TemplateBase {};
   )cpp";
 
   Annotations Code(Test);
   auto TU = TestTU::withCode(Code.code());
   auto AST = TU.build();
-  for (const std::string &Label : {"1", "2", "3", "4"}) {
+  for (StringRef Label : {"0", "1", "2", "3", "4", "5"}) {
 for (const auto &Point : Code.points(Label)) {
   EXPECT_THAT(findImplementations(AST, Point, TU.index().get()),
   UnorderedPointwise(DeclRange(), Code.ranges(Label)))
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -83,7 +83,9 @@
   bool HasMore = false;
 };
 
-/// Returns implementations of the virtual function at a specified \p Pos.
+/// Returns implementations at a specified \p Pos:
+///   - overrides for a virtual method;
+///   - subclasses for a base class;
 std::vector findImplementations(ParsedAST &AST, Position Pos,
const SymbolIndex *Index);
 
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -292,13 +292,14 @@
   return D;
 }
 
-std::vector findOverrides(llvm::DenseSet IDs,
- const SymbolIndex *Index,
- llvm::StringRef MainFilePath) {
+std::vector findImplementors(llvm::DenseSet IDs,
+RelationKind Predicate,
+const SymbolIndex *Index,
+llvm::StringRef MainFilePath) {
   if (IDs.empty())
 return {};
   RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Predicate = Predicate;
   Req.Subjects = std::move(IDs);
   std::vector Results;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
@@ -458,7 +459,8 @@
 });
   }
 
-  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  auto Overrides = findImplementors(VirtualMethods, RelationKind::OverriddenBy,
+Index, MainFilePath);
   Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
@@ -1239,12 +1241,20 @@
   }
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  llvm::DenseSet VirtualMethods;
-  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
-if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
-  if (CXXMD->isVirtual())
-VirtualMethods.insert(getSymbolID(ND));
-  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
+  llvm::DenseSet IDs;
+  RelationKind QueryKind;
+  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations)) {
+if (const auto *CXXMD = llvm::dyn_cast(ND)) {
+  if (CXXMD->isVirtual()) {
+IDs.insert(getSymbolID(ND));
+QueryKind = RelationKind::OverriddenBy;
+  }
+} else if (const auto *RD = dyn_cast(ND)) {
+  IDs.insert(getSymbolID(RD));
+  QueryKind = RelationKind::BaseOf;
+}
+  }
+  return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

How does this approach work with differing language standards. For example with 
an old implementation designed for c++14, `string_view` header won't exist. If 
the implementation is designed to work with c++17, including that header in 
c++14 mode will probably be ifdef... Error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:49
+  /// To do this, isNamespaceVisible should return true on "foo" NamespaceDecl.
+  virtual bool isNamespaceVisible(const NamespaceDecl *NS) const {
+return false;

oh wow, i didn't know about these callbacks at all.

this looks pretty need, but i wonder if we should make it more generic like 
"shouldPrintScope" (or isScopeVisible) and pass not only namespacedecls but the 
declcontext directly to enable short circuiting of type scopes as well?

also it might be nice to note in comments that any parent scope won't be 
visited once this callback returns true (or false in the case of 
shouldPrintScope).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

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


[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

In D94293#2486468 , @njames93 wrote:

> How does this approach work with differing language standards. For example 
> with an old implementation designed for c++14, `string_view` header won't 
> exist. If the implementation is designed to work with c++17, including that 
> header in c++14 mode will probably be ifdef... Error.

As I mentioned in my comments for putting this behind a flag, in such scenarios 
we should just hit some "missing includes" while indexing the phantom file, so 
all should be fine.




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
 
+opt IndexSTL{
+"index-stl",

kuhnel wrote:
> kadircet wrote:
> > do we really need to hide this behind a flag ? it sounds like a quite 
> > useful feature to me with minimal risk of regressions and unlikely to make 
> > anyone upset. in the end we are not doing anything heuristically and just 
> > throwing clang on some STL headers, that'll probably be included within the 
> > TUs eventually.
> > 
> > there's always the risk of crashing while parsing some STL headers, but i 
> > don't think it is any different than user just including the header 
> > manually.
> > 
> > the biggest problem i can see is people using custom STL headers, but 
> > hopefully compile flags interpolation logic should be able to infer the 
> > relevant location for those. and in the cases it fails we either index the 
> > default STL and suggest people some symbols their implementation might 
> > lack, or fail to find STL at all and print some logs for the missing 
> > includes.
> Back when I was developing embedded software, we couldn't use the STL for 
> various reasons (object size, no exceptions, no dynamic memory, ...) and had 
> our own, proprietary base library. In such a scenario it would be annoying if 
> clangd would be proposing things I could not use in the project. 
> 
> So we should have a way for users to disable this. I'm fine if we switch it 
> on by default and offer a `--disable-stl-index` flag instead.
I see. It still feels like those developers would be less inclined to accept 
autocompletion of `std::vector` compared to `xyz::vector` and also our ranking 
should be doing a good job of promoting `xyz::vector` due to its high number of 
usage, compared to `std::vector`.

I am mostly unhappy about the flag as it needs propagation to many components. 
It might be better to have it as a config option, which requires a lot less 
plumbing (at least for bg-index), if you think even the down-ranking of `std` 
symbols wouldn't be a good experience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[clang] 9c4b222 - Revert "Revert "Revert "Revert "Revert "[analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis."""""

2021-01-08 Thread Alexander Belyaev via cfe-commits

Author: Alexander Belyaev
Date: 2021-01-08T14:17:18+01:00
New Revision: 9c4b2225b24de07a728715ce20238803370413ea

URL: 
https://github.com/llvm/llvm-project/commit/9c4b2225b24de07a728715ce20238803370413ea
DIFF: 
https://github.com/llvm/llvm-project/commit/9c4b2225b24de07a728715ce20238803370413ea.diff

LOG: Revert "Revert "Revert "Revert "Revert "[analyzer] NFC: Move path 
diagnostic consumer implementations to libAnalysis."

This reverts commit 6b0ee02747ed22d41e175d15f27025183341e6f8.

Circular dependency again.

Added: 
clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

Modified: 
clang/include/clang/CrossTU/CrossTranslationUnit.h
clang/include/clang/StaticAnalyzer/Core/Analyses.def
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
clang/include/clang/module.modulemap
clang/lib/Analysis/CMakeLists.txt
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 
clang/include/clang/Analysis/CrossTUAnalysisHelper.h
clang/include/clang/Analysis/PathDiagnosticConsumers.def
clang/include/clang/Analysis/PathDiagnosticConsumers.h
clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp
clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp
clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp
clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp
clang/lib/Analysis/TextPathDiagnosticConsumer.cpp



diff  --git a/clang/include/clang/Analysis/CrossTUAnalysisHelper.h 
b/clang/include/clang/Analysis/CrossTUAnalysisHelper.h
deleted file mode 100644
index 500e78ddedcf..
--- a/clang/include/clang/Analysis/CrossTUAnalysisHelper.h
+++ /dev/null
@@ -1,41 +0,0 @@
-//===- CrossTUAnalysisHelper.h - Abstraction layer for CTU --*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-#ifndef LLVM_CLANG_ANALYSIS_CROSS_TU_HELPER_H
-#define LLVM_CLANG_ANALYSIS_CROSS_TU_HELPER_H
-
-#include "llvm/ADT/Optional.h"
-#include "clang/Basic/SourceManager.h"
-
-namespace clang {
-
-class ASTUnit;
-
-/// This class is an abstract interface acting as a bridge between
-/// an analysis that requires lookups across translation units (a user
-/// of that interface) and the facility that implements such lookups
-/// (an implementation of that interface). This is useful to break direct
-/// link-time dependencies between the (possibly shared) libraries in which
-/// the user and the implementation live.
-class CrossTUAnalysisHelper {
-public:
-  /// Determine the original source location in the original TU for an
-  /// imported source location.
-  /// \p ToLoc Source location in the imported-to AST.
-  /// \return Source location in the imported-from AST and the corresponding
-  /// ASTUnit object (the AST was loaded from a file using an internal ASTUnit
-  /// object that is returned here).
-  /// If any error happens (ToLoc is a non-imported source location) empty is
-  /// returned.
-  virtual llvm::Optional>
-  getImportedFromSourceLocationWithPreprocessor(SourceLocation ToLoc) const = 
0;
-
-  virtual ~CrossTUAnalysisHelper() {}
-};
-} // namespace clang
-
-#endif // LLVM_CLANG_ANALYSIS_CROSS_TU_HELPER_H

diff  --git a/clang/include/clang/Analysis/PathDiagnosticConsumers.def 
b/clang/include/clang/Analysis/PathDiagnosticConsumers.def
deleted file mode 100644
index 33d2072fcf31..
--- a/clang/include/clang/Analysis/PathDiagnosticConsumers.def
+++ /dev/null
@@ -1,50 +0,0 @@
-//===-- PathDiagnosticConsumers.def - Visualizing warnings --*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-// This file defines the set of path diagnostic consumers - objects that
-// implement 
diff erent representations of static analysis results.
-//
-//===--===//
-
-#ifndef ANALYSIS_DIAGNOSTICS
-#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
-#endif
-
-ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
- createHTM

[clang] af7cce2 - [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

2021-01-08 Thread Mark Murray via cfe-commits

Author: Mark Murray
Date: 2021-01-08T13:21:11Z
New Revision: af7cce2fa4d19c3cd09607e1d6ea2e0847cc55b7

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

LOG: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer 
authentication extension.

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/AArch64TargetParser.h
llvm/lib/Support/AArch64TargetParser.cpp
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.td
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/lib/Target/AArch64/AArch64SystemOperands.td
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index c1abe8e9f75b..d03bca9cfd90 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -510,6 +510,8 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector &Features,
   HasMTE = true;
 if (Feature == "+tme")
   HasTME = true;
+if (Feature == "+pauth")
+  HasPAuth = true;
 if (Feature == "+i8mm")
   HasMatMul = true;
 if (Feature == "+bf16")

diff  --git a/clang/lib/Basic/Targets/AArch64.h 
b/clang/lib/Basic/Targets/AArch64.h
index bd576680077e..5f24ab6a4d61 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -36,6 +36,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public 
TargetInfo {
   bool HasFP16FML;
   bool HasMTE;
   bool HasTME;
+  bool HasPAuth;
   bool HasLS64;
   bool HasMatMul;
   bool HasSVE2;

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.def 
b/llvm/include/llvm/Support/AArch64TargetParser.def
index f1a5bf734a13..38cc2e753740 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.def
+++ b/llvm/include/llvm/Support/AArch64TargetParser.def
@@ -108,6 +108,7 @@ AARCH64_ARCH_EXT_NAME("f64mm",AArch64::AEK_F64MM,   
"+f64mm", "-f64m
 AARCH64_ARCH_EXT_NAME("tme",  AArch64::AEK_TME, "+tme",   
"-tme")
 AARCH64_ARCH_EXT_NAME("ls64", AArch64::AEK_LS64,"+ls64",  
"-ls64")
 AARCH64_ARCH_EXT_NAME("brbe", AArch64::AEK_BRBE,"+brbe",  
"-brbe")
+AARCH64_ARCH_EXT_NAME("pauth",AArch64::AEK_PAUTH,   "+pauth", 
"-pauth")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.h 
b/llvm/include/llvm/Support/AArch64TargetParser.h
index a3c9c6a30483..35827517d7fc 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.h
+++ b/llvm/include/llvm/Support/AArch64TargetParser.h
@@ -64,6 +64,7 @@ enum ArchExtKind : uint64_t {
   AEK_F64MM =   1ULL << 32,
   AEK_LS64 =1ULL << 33,
   AEK_BRBE =1ULL << 34,
+  AEK_PAUTH =   1ULL << 35,
 };
 
 enum class ArchKind {

diff  --git a/llvm/lib/Support/AArch64TargetParser.cpp 
b/llvm/lib/Support/AArch64TargetParser.cpp
index 62761177c8c2..be595e83dbef 100644
--- a/llvm/lib/Support/AArch64TargetParser.cpp
+++ b/llvm/lib/Support/AArch64TargetParser.cpp
@@ -102,6 +102,8 @@ bool AArch64::getExtensionFeatures(uint64_t Extensions,
 Features.push_back("+rcpc");
   if (Extensions & AEK_BRBE)
 Features.push_back("+brbe");
+  if (Extensions & AEK_PAUTH)
+Features.push_back("+pauth");
 
   return true;
 }

diff  --git a/llvm/lib/Target/AArch64/AArch64.td 
b/llvm/lib/Target/AArch64/AArch64.td
index 2be006bb647f..1f1bf0ac1657 100644
--- a/llvm/lib/Target/AArch64/AArch64.td
+++ b/llvm/lib/Target/AArch64/AArch64.td
@@ -261,8 +261,8 @@ def FeatureDotProd : SubtargetFeature<
 "dotprod", "HasDotProd", "true",
 "Enable dot product support">;
 
-def FeaturePA : SubtargetFeature<
-"pa", "HasPA", "true",
+def FeaturePAuth : SubtargetFeature<
+"pauth", "HasPAuth", "true",
 "Enable v8.3-A Pointer Authentication extension">;
 
 def FeatureJS : SubtargetFeature<
@@ -438,7 +438,7 @@ def HasV8_2aOps : SubtargetFeature<"v8.2a", "HasV8_2aOps", 
"true",
   FeaturePAN_RWV, FeatureRAS, FeatureCCPP]>;
 
 def HasV8_3aOps : SubtargetFeature<"v8.3a", "HasV8_3aOps", "true",
-  "Support ARM v8.3a instructions", [HasV8_2aOps, FeatureRCPC, FeaturePA,
+  "Support ARM v8.3a instructions", [HasV8_2aOps, FeatureRCPC, FeaturePAuth,
   FeatureJS, FeatureCCIDX, FeatureComplxNum]>;
 
 def HasV8_4aOps : SubtargetFeature<"v8.4a", "HasV8_4aOps", "true",
@@ -471,7 +471,7 @@ def Has

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D93375#2486165 , @tbaeder wrote:

> That sounds good but it's different to what this patch is trying to 
> accomplish, isn't it? -ansi already doesn't get passed to the linker.
> E.g. with clang 10.0.1 without this patch:
>
>   ~ ❯ clang test.o -o test -ansi -###
>   clang version 10.0.1 (Fedora 10.0.1-3.fc32)
>   Target: x86_64-unknown-linux-gnu
>   Thread model: posix
>   InstalledDir: /usr/bin
>   clang-10: warning: argument unused during compilation: '-ansi' 
> [-Wunused-command-line-argument]
>"/usr/bin/ld" "--hash-style=gnu" "--build-id" "--eh-frame-hdr" "-m" 
> "elf_x86_64" "-dynamic-linker" "/lib64/ld-linux-x86-64.so.2" "-o" "test" 
> "/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crt1.o" 
> "/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crti.o" 
> "/usr/bin/../lib/gcc/x86_64-redhat-linux/10/crtbegin.o" 
> "-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10" 
> "-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64" 
> "-L/usr/bin/../lib64" "-L/lib/../lib64" "-L/usr/lib/../lib64" 
> "-L/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../.." "-L/usr/bin/../lib" 
> "-L/lib" "-L/usr/lib" "test.o" "-lgcc" "--as-needed" "-lgcc_s" 
> "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
> "/usr/bin/../lib/gcc/x86_64-redhat-linux/10/crtend.o" 
> "/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crtn.o"

Hmm, I think I misunderstood what you had originally written then with:

> -ansi is documented as being the "same as -std=c89", but there are 
> differences when passing it to a link:

Regardless, the request is whether we can add test coverage to ensure we don't 
regress this behavior in the future.


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

https://reviews.llvm.org/D93375

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


[clang] 7d4a8bc - [AArch64] Add +flagm archictecture option, allowing the v8.4a flag modification extension.

2021-01-08 Thread Mark Murray via cfe-commits

Author: Mark Murray
Date: 2021-01-08T13:21:12Z
New Revision: 7d4a8bc417bf75b5e4034674a4255173d0089e68

URL: 
https://github.com/llvm/llvm-project/commit/7d4a8bc417bf75b5e4034674a4255173d0089e68
DIFF: 
https://github.com/llvm/llvm-project/commit/7d4a8bc417bf75b5e4034674a4255173d0089e68.diff

LOG: [AArch64] Add +flagm archictecture option, allowing the v8.4a flag 
modification extension.

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/AArch64TargetParser.h
llvm/lib/Support/AArch64TargetParser.cpp
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64InstrInfo.td
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/test/MC/AArch64/armv8.4a-flag.s
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index d03bca9cfd90..312c822ebb05 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -520,6 +520,8 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector &Features,
   HasLSE = true;
 if (Feature == "+ls64")
   HasLS64 = true;
+if (Feature == "+flagm")
+  HasFlagM = true;
   }
 
   setDataLayout();

diff  --git a/clang/lib/Basic/Targets/AArch64.h 
b/clang/lib/Basic/Targets/AArch64.h
index 5f24ab6a4d61..2809fbce9c88 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -47,6 +47,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public 
TargetInfo {
   bool HasMatmulFP64;
   bool HasMatmulFP32;
   bool HasLSE;
+  bool HasFlagM;
 
   llvm::AArch64::ArchKind ArchKind;
 

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.def 
b/llvm/include/llvm/Support/AArch64TargetParser.def
index 38cc2e753740..5f36b0eecff9 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.def
+++ b/llvm/include/llvm/Support/AArch64TargetParser.def
@@ -109,6 +109,7 @@ AARCH64_ARCH_EXT_NAME("tme",  AArch64::AEK_TME, 
"+tme",   "-tme"
 AARCH64_ARCH_EXT_NAME("ls64", AArch64::AEK_LS64,"+ls64",  
"-ls64")
 AARCH64_ARCH_EXT_NAME("brbe", AArch64::AEK_BRBE,"+brbe",  
"-brbe")
 AARCH64_ARCH_EXT_NAME("pauth",AArch64::AEK_PAUTH,   "+pauth", 
"-pauth")
+AARCH64_ARCH_EXT_NAME("flagm",AArch64::AEK_FLAGM,   "+flagm", 
"-flagm")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.h 
b/llvm/include/llvm/Support/AArch64TargetParser.h
index 35827517d7fc..7c9e245e3889 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.h
+++ b/llvm/include/llvm/Support/AArch64TargetParser.h
@@ -65,6 +65,7 @@ enum ArchExtKind : uint64_t {
   AEK_LS64 =1ULL << 33,
   AEK_BRBE =1ULL << 34,
   AEK_PAUTH =   1ULL << 35,
+  AEK_FLAGM =   1ULL << 36,
 };
 
 enum class ArchKind {

diff  --git a/llvm/lib/Support/AArch64TargetParser.cpp 
b/llvm/lib/Support/AArch64TargetParser.cpp
index be595e83dbef..503a7bd49d15 100644
--- a/llvm/lib/Support/AArch64TargetParser.cpp
+++ b/llvm/lib/Support/AArch64TargetParser.cpp
@@ -104,6 +104,8 @@ bool AArch64::getExtensionFeatures(uint64_t Extensions,
 Features.push_back("+brbe");
   if (Extensions & AEK_PAUTH)
 Features.push_back("+pauth");
+  if (Extensions & AEK_FLAGM)
+Features.push_back("+flagm");
 
   return true;
 }

diff  --git a/llvm/lib/Target/AArch64/AArch64.td 
b/llvm/lib/Target/AArch64/AArch64.td
index 1f1bf0ac1657..002efd700cc2 100644
--- a/llvm/lib/Target/AArch64/AArch64.td
+++ b/llvm/lib/Target/AArch64/AArch64.td
@@ -316,8 +316,8 @@ def FeatureTLB_RMI : SubtargetFeature<
 "tlb-rmi", "HasTLB_RMI", "true",
 "Enable v8.4-A TLB Range and Maintenance Instructions">;
 
-def FeatureFMI : SubtargetFeature<
-"fmi", "HasFMI", "true",
+def FeatureFlagM : SubtargetFeature<
+"flagm", "HasFlagM", "true",
 "Enable v8.4-A Flag Manipulation Instructions">;
 
 // 8.4 RCPC enchancements: LDAPR & STLR instructions with Immediate Offset
@@ -445,7 +445,7 @@ def HasV8_4aOps : SubtargetFeature<"v8.4a", "HasV8_4aOps", 
"true",
   "Support ARM v8.4a instructions", [HasV8_3aOps, FeatureDotProd,
   FeatureNV, FeatureMPAM, FeatureDIT,
   FeatureTRACEV8_4, FeatureAM, FeatureSEL2, FeaturePMU, FeatureTLB_RMI,
-  FeatureFMI, FeatureRCPC_IMMO]>;
+  FeatureFlagM, FeatureRCPC_IMMO]>;
 
 def HasV8_5aOps : SubtargetFeature<
   "v8.5a", "HasV8_5aOps", "true", "Support ARM v8.5a instructions",
@@ -474,7 +474,7 @@ def HasV8_0rOps : SubtargetFeature<
   FeaturePAuth, FeatureRCPC,
   //v8.4
   FeatureDotProd, FeatureFP16FML, FeatureTRACEV8_4,
-  FeatureTLB_RMI, FeatureFMI, FeatureDIT, FeatureSEL2, FeatureRCPC_IMMO,
+  FeatureTLB_RMI, FeatureFlagM, FeatureDIT, Feature

[PATCH] D94081: [AArch64] Add +flagm archictecture option, allowing the v8.4a flag modification extension.

2021-01-08 Thread Mark Murray via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d4a8bc417bf: [AArch64] Add +flagm archictecture option, 
allowing the v8.4a flag modification… (authored by MarkMurrayARM).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94081

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/MC/AArch64/armv8.4a-flag.s
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1408,6 +1408,7 @@
 TEST(TargetParserTest, AArch64ArchExtFeature) {
   const char *ArchExt[][4] = {{"crc", "nocrc", "+crc", "-crc"},
   {"crypto", "nocrypto", "+crypto", "-crypto"},
+  {"flagm", "noflagm", "+flagm", "-flagm"},
   {"fp", "nofp", "+fp-armv8", "-fp-armv8"},
   {"simd", "nosimd", "+neon", "-neon"},
   {"fp16", "nofp16", "+fullfp16", "-fullfp16"},
Index: llvm/test/MC/AArch64/armv8.4a-flag.s
===
--- llvm/test/MC/AArch64/armv8.4a-flag.s
+++ llvm/test/MC/AArch64/armv8.4a-flag.s
@@ -1,13 +1,13 @@
 // RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+v8.4a %s -o - | \
 // RUN: FileCheck %s
 
-// RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+fmi %s -o - 2>&1 | \
+// RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+flagm %s -o - 2>&1 | \
 // RUN: FileCheck %s
 
 // RUN: not llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=-v8.4a %s -o - 2>&1 | \
 // RUN: FileCheck %s --check-prefix=ERROR
 
-// RUN: not llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+v8.4a,-fmi %s -o - 2>&1 | \
+// RUN: not llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+v8.4a,-flagm %s -o - 2>&1 | \
 // RUN: FileCheck %s --check-prefix=ERROR
 
 //--
@@ -30,24 +30,24 @@
 //CHECK-NEXT: rmif x1, #63, #15// encoding: [0x2f,0x84,0x1f,0xba]
 //CHECK-NEXT: rmif xzr, #63, #15   // encoding: [0xef,0x87,0x1f,0xba]
 
-//ERROR:  error: instruction requires: fmi
+//ERROR:  error: instruction requires: flagm
 //ERROR-NEXT: cfinv
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: setf8 w1
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: setf8 wzr
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: setf16 w1
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: setf16 wzr
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: rmif x1, #63, #15
 //ERROR-NEXT: ^
-//ERROR-NEXT: error: instruction requires: fmi
+//ERROR-NEXT: error: instruction requires: flagm
 //ERROR-NEXT: rmif xzr, #63, #15
 //ERROR-NEXT: ^
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -140,7 +140,7 @@
   bool HasSEL2 = false;
   bool HasPMU = false;
   bool HasTLB_RMI = false;
-  bool HasFMI = false;
+  bool HasFlagM = false;
   bool HasRCPC_IMMO = false;
 
   bool HasLSLFast = false;
@@ -513,7 +513,7 @@
   bool hasSEL2() const { return HasSEL2; }
   bool hasPMU() const { return HasPMU; }
   bool hasTLB_RMI() const { return HasTLB_RMI; }
-  bool hasFMI() const { return HasFMI; }
+  bool hasFlagM() const { return HasFlagM; }
   bool hasRCPC_IMMO() const { return HasRCPC_IMMO; }
 
   bool addrSinkUsingGEPs() const override {
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -69,8 +69,8 @@
 def HasTLB_RMI  : Predicate<"Subtarget->hasTLB_RMI()">,
AssemblerPredicate<(all_of FeatureTLB_RMI), "tlb-rmi">;
 
-def HasFMI   : Predicate<"Subtarget->hasFMI()">,
-   AssemblerPredi

[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

2021-01-08 Thread Mark Murray via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf7cce2fa4d1: [AArch64] Add +pauth archictecture option, 
allowing the v8.3a pointer… (authored by MarkMurrayARM).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94083

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AArch64SystemOperands.td
  llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1431,6 +1431,7 @@
   {"rng", "norng", "+rand", "-rand"},
   {"memtag", "nomemtag", "+mte", "-mte"},
   {"tme", "notme", "+tme", "-tme"},
+  {"pauth", "nopauth", "+pauth", "-pauth"},
   {"ssbs", "nossbs", "+ssbs", "-ssbs"},
   {"sb", "nosb", "+sb", "-sb"},
   {"predres", "nopredres", "+predres", "-predres"},
Index: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -4958,7 +4958,7 @@
 AArch64::GPR64RegClass);
   }
 
-  if (STI.hasV8_3aOps()) {
+  if (STI.hasPAuth()) {
 MIRBuilder.buildInstr(AArch64::XPACI, {DstReg}, {MFReturnAddr});
   } else {
 MIRBuilder.buildCopy({Register(AArch64::LR)}, {MFReturnAddr});
@@ -4986,7 +4986,7 @@
 else {
   MFI.setReturnAddressIsTaken(true);
 
-  if (STI.hasV8_3aOps()) {
+  if (STI.hasPAuth()) {
 Register TmpReg = MRI.createVirtualRegister(&AArch64::GPR64RegClass);
 MIRBuilder.buildInstr(AArch64::LDRXui, {TmpReg}, {FrameAddr}).addImm(1);
 MIRBuilder.buildInstr(AArch64::XPACI, {DstReg}, {TmpReg});
Index: llvm/lib/Target/AArch64/AArch64SystemOperands.td
===
--- llvm/lib/Target/AArch64/AArch64SystemOperands.td
+++ llvm/lib/Target/AArch64/AArch64SystemOperands.td
@@ -1336,7 +1336,7 @@
 
 // v8.3a "Pointer authentication extension" registers
 //  Op0Op1 CRn CRmOp2
-let Requires = [{ {AArch64::FeaturePA} }] in {
+let Requires = [{ {AArch64::FeaturePAuth} }] in {
 def : RWSysReg<"APIAKeyLo_EL1", 0b11, 0b000, 0b0010, 0b0001, 0b000>;
 def : RWSysReg<"APIAKeyHi_EL1", 0b11, 0b000, 0b0010, 0b0001, 0b001>;
 def : RWSysReg<"APIBKeyLo_EL1", 0b11, 0b000, 0b0010, 0b0001, 0b010>;
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -126,7 +126,7 @@
   bool HasAES = false;
 
   // ARMv8.3 extensions
-  bool HasPA = false;
+  bool HasPAuth = false;
   bool HasJS = false;
   bool HasCCIDX = false;
   bool HasComplxNum = false;
@@ -186,6 +186,7 @@
   bool HasETE = false;
   bool HasTRBE = false;
   bool HasBRBE = false;
+  bool HasPAUTH = false;
   bool HasSPE_EEF = false;
 
   // HasZeroCycleRegMove - Has zero-cycle register mov instructions.
@@ -450,6 +451,7 @@
   bool hasRandGen() const { return HasRandGen; }
   bool hasMTE() const { return HasMTE; }
   bool hasTME() const { return HasTME; }
+  bool hasPAUTH() const { return HasPAUTH; }
   // Arm SVE2 extensions
   bool hasSVE2AES() const { return HasSVE2AES; }
   bool hasSVE2SM4() const { return HasSVE2SM4; }
@@ -493,7 +495,7 @@
   bool hasPAN_RWV() const { return HasPAN_RWV; }
   bool hasCCPP() const { return HasCCPP; }
 
-  bool hasPA() const { return HasPA; }
+  bool hasPAuth() const { return HasPAuth; }
   bool hasJS() const { return HasJS; }
   bool hasCCIDX() const { return HasCCIDX; }
   bool hasComplxNum() const { return HasComplxNum; }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -33,8 +33,8 @@
 def HasLOR   : Pred

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D93630#2480070 , @vsavchenko wrote:

> In D93630#2479757 , @aaron.ballman 
> wrote:
>
>> There are attributes like `nomerge` which are both a declaration and a 
>> statement attribute, and there are other attributes like `suppress` that are 
>> statement attributes which can reasonably be written on a declaration 
>> statement. So the scenario I am worried about is when the code is being 
>> parsed as a declaration, the user adds one of these attributes and the 
>> decision changes to parse as a statement rather than a declaration. I'm not 
>> yet convinced we can't run into that situation with this change, but the 
>> implicit `int` thing was the only major edge case I could come up with and 
>> I've almost convinced myself we can't run into an issue with it. I think 
>> our parser predicate test (on line 218 of your patch) for whether something 
>> is a declaration statement or not is what's helping avoid most of the 
>> parsing concerns I have because they usually will find some declaration 
>> specifier to help get the right answer.
>
> I actually thought about this.  Essentially there are 3 interesting cases 
> here:
>
> 1. the user has a declaration that is parsed as a declaration only because of 
> the attribute and wants to add a statement attribute
>
>   __attribute__((X)) i = 12;
>   // into
>   __attribute__((X, fallthrough)) i = 12;
>
> We don't want to break working code, so we maintain the rule and parse it as 
> a declaration.
>
> 2. the user has a statement and wants to add a statement attribute
>
>   i = 12; // assignment
>   // into
>   __attribute__((stmt_attr)) i = 12; // still assignment
>
> We don't use the rule and don't break the code, that's why we do it!
>
> 3. the user has a declaration-or-statement attribute on a declaration that is 
> parsed as declaration only because of this attribute
>
>   __attribute__((both-decl-and-stmt)) i = 12; // implicit int declaration
>   // after the Clang update
>   __attribute__((both-decl-and-stmt)) i = 12; // error!
>
> That is a regression, but it looks like statement attributes are pretty rare, 
> let alone declaration-or-statement attributes in C.  Also, it might cause the 
> problem only when the user relied on the "attribute -> declaration" rule.  
> So, it seems pretty low chance, and we get a bit more consistency in what 
> "statement attribute" means.

I agree with these three cases. I did come up with a fourth case: the user 
wants to attribute something that is a statement which declares things but is 
not a declaration statement. e.g., `__attribute__((whatever)) [](){}();` (or 
similar using blocks instead of lambdas). We don't parse this construct on 
trunk (https://godbolt.org/z/3e83of) and I tried it with your patch applied and 
it still wouldn't parse, so it may make for a good test case to add.

>> FWIW, while testing various situations out, I think I found a Clang bug 
>> where we reject valid code (https://godbolt.org/z/ooqMvP) that *might* cause 
>> us problems if we apply this patch and ever fix that rejects valid bug. We 
>> should not be rejecting any of these functions, but we fail to note the 
>> implicit `int` with the declaration within `foo()` (note how the presence of 
>> the `extern` declaration specifier in `quux()` fixes things).
>
> Yikes, that's pretty bad!

I filed https://bugs.llvm.org/show_bug.cgi?id=48672 for it.

 or `__attribute__((suppress)) g() { return 12; }` which may be intended to 
 suppress the "type specifier missing" diagnostic (somewhat amusingly, 
 since the user could also just add the `int` to the declaration since 
 they're changing the code anyway).
>>>
>>> Again, we **do not force** the parser to parse whatever follows the 
>>> statement attribute as a statement.  `__attribute__((suppress))` will not 
>>> change how we parse things here.
>>
>> I agree that it shouldn't (in theory), but I've not been able to convince 
>> myself that it doesn't (in practice). I'm getting there though, and I 
>> appreciate your patience while we discuss the concerns.
>
> I actually want to thank you instead.  It is a pretty tricky situation here 
> with all this, so thank you for going so patiently from one case to another!

Huttah for quality code review practices! :-)

>> Concretely, I think we could use some more C test coverage for the various 
>> places where implicit `int` can appear in a declaration. Here are the places 
>> I could come up with off the top of my head: https://godbolt.org/z/6h3MTr
>
> That's awesome, I'll incorporate that into the patch!

I'm going to see if I can run the patch against our internal corpus here at 
work to see if it shakes out any breakages to real world code. I've not tried 
this before, so I have no idea how long it will take before I get results back, 
but I'll let you know when I have them.


Repository:
  rG LLVM Github Monorepo

CH

[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz updated this revision to Diff 315363.
adamcz marked an inline comment as done.
adamcz added a comment.

Addressed review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1225,6 +1225,9 @@
   if (DC->isFunctionOrMethod())
 return;
 
+  if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
+return;
+
   if (const auto *NS = dyn_cast(DC)) {
 if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
   return AppendScope(DC->getParent(), OS, NameInScope);
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -18,6 +18,7 @@
 
 namespace clang {
 
+class DeclContext;
 class LangOptions;
 class SourceManager;
 class Stmt;
@@ -39,6 +40,15 @@
   virtual std::string remapPath(StringRef Path) const {
 return std::string(Path);
   }
+
+  /// When printing type to be inserted into code in specific context, this
+  /// callback can be used to avoid printing the redundant part of the
+  /// qualifier. For example, when inserting code inside namespace foo, we
+  /// should print bar::SomeType instead of foo::bar::SomeType.
+  /// To do this, shouldPrintScope should return true on "foo" NamespaceDecl.
+  /// The printing stops at the first isScopeVisible() == true, so there will
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *NS) const { return false; }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
+  // Check const qualifier with namespace
+  std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+  [[return y.z + y.z;]]
+})cpp";
+  std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+  return extracted(y);
+})cpp";
+  EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,9 @@
   EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
 R"cpp(const char * x = "test";)cpp");
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
   // expanding types in structured bindings is syntactically invalid.
   EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@
   return SymbolID(USR);
 }
 
-// FIXME: This should be handled while printing underlying decls instead.
 std::string printType(const QualType QT, const DeclContext &CurContext) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  auto Decls =
-  explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
-  if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
-   Decls.front(),
-   /*VisibleNamespaces=*/llvm::ArrayRef{});
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
-  PP.SuppressScope = true;
   PP.SuppressTagKeyword = true;
+  PP.SuppressUnwrittenScope = true;
+
+  class PrintCB : public PrintingCallbacks {
+  public:
+PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
+virtual ~PrintCB() {}
+virtual bool isScopeVisible(const DeclContext *DC) const {
+  return DC->Encloses(CurContext);
+}
+
+  priv

[clang] 0ef2b68 - [OpenCL] Documentation for experimental C++ libs

2021-01-08 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-01-08T13:45:59Z
New Revision: 0ef2b68ff063164a83a37a2a363196d51b76ed8d

URL: 
https://github.com/llvm/llvm-project/commit/0ef2b68ff063164a83a37a2a363196d51b76ed8d
DIFF: 
https://github.com/llvm/llvm-project/commit/0ef2b68ff063164a83a37a2a363196d51b76ed8d.diff

LOG: [OpenCL] Documentation for experimental C++ libs

Started a new doc section about the OpenCL experimental
features and described ongoing work on C++ libraries
e.g. type traits.

Tags: #clang

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

Added: 


Modified: 
clang/docs/OpenCLSupport.rst

Removed: 




diff  --git a/clang/docs/OpenCLSupport.rst b/clang/docs/OpenCLSupport.rst
index 62ba890c3a95..b00a9ef41064 100644
--- a/clang/docs/OpenCLSupport.rst
+++ b/clang/docs/OpenCLSupport.rst
@@ -20,7 +20,7 @@ OpenCL Support
 Clang fully supports all OpenCL C versions from 1.1 to 2.0.
 
 Please refer to `Bugzilla
-`_
+`__
 for the most up to date bug reports.
 
 
@@ -45,3 +45,57 @@ Missing features or with limited support
 - Initialization of objects in `__constant` address spaces is not guaranteed 
to work.
 
 - `addrspace_cast` operator is not supported.
+
+Experimental features
+=
+
+Clang provides the following new WIP features for the developers to experiment
+and provide early feedback or contribute with further improvements.
+Feel free to contact us on `cfe-dev
+`_ or via `Bugzilla
+`__.
+
+C++ libraries for OpenCL
+
+
+There is ongoing work to support C++ standard libraries from `LLVM's libcxx
+`_ in OpenCL kernel code using C++ for OpenCL mode.
+
+It is currently possible to include `type_traits` from C++17 in the kernel
+sources when the following clang extensions are enabled
+``__cl_clang_function_pointers`` and ``__cl_clang_variadic_functions``,
+see :doc:`LanguageExtensions` for more details. The use of non-conformant
+features enabled by the extensions does not expose non-conformant behavior
+beyond the compilation i.e. does not get generated in IR or binary.
+The extension only appear in metaprogramming
+mechanism to identify or verify the properties of types. This allows to provide
+the full C++ functionality without a loss of portability. To avoid unsafe use
+of the extensions it is recommended that the extensions are disabled directly
+after the header include.
+
+**Example of Use**:
+
+The example of kernel code with `type_traits` is illustrated here.
+
+.. code-block:: c++
+
+  #pragma OPENCL EXTENSION __cl_clang_function_pointers : enable
+  #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
+  #include 
+  #pragma OPENCL EXTENSION __cl_clang_function_pointers : disable
+  #pragma OPENCL EXTENSION __cl_clang_variadic_functions : disable
+
+  using sint_type = std::make_signed::type;
+
+  __kernel void foo() {
+static_assert(!std::is_same::value);
+  }
+
+The possible clang invocation to compile the example is as follows:
+
+   .. code-block:: console
+
+ $ clang -cl-std=clc++  -I/include test.cl
+
+Note that `type_traits` is a header only library and therefore no extra
+linking step against the standard libraries is required.



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


[PATCH] D94188: [OpenCL] Documentation for experimental C++ libraries support

2021-01-08 Thread Anastasia Stulova via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Anastasia marked an inline comment as done.
Closed by commit rG0ef2b68ff063: [OpenCL] Documentation for experimental C++ 
libs (authored by Anastasia).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D94188?vs=315173&id=315364#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94188

Files:
  clang/docs/OpenCLSupport.rst


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -20,7 +20,7 @@
 Clang fully supports all OpenCL C versions from 1.1 to 2.0.
 
 Please refer to `Bugzilla
-`_
+`__
 for the most up to date bug reports.
 
 
@@ -45,3 +45,57 @@
 - Initialization of objects in `__constant` address spaces is not guaranteed 
to work.
 
 - `addrspace_cast` operator is not supported.
+
+Experimental features
+=
+
+Clang provides the following new WIP features for the developers to experiment
+and provide early feedback or contribute with further improvements.
+Feel free to contact us on `cfe-dev
+`_ or via `Bugzilla
+`__.
+
+C++ libraries for OpenCL
+
+
+There is ongoing work to support C++ standard libraries from `LLVM's libcxx
+`_ in OpenCL kernel code using C++ for OpenCL mode.
+
+It is currently possible to include `type_traits` from C++17 in the kernel
+sources when the following clang extensions are enabled
+``__cl_clang_function_pointers`` and ``__cl_clang_variadic_functions``,
+see :doc:`LanguageExtensions` for more details. The use of non-conformant
+features enabled by the extensions does not expose non-conformant behavior
+beyond the compilation i.e. does not get generated in IR or binary.
+The extension only appear in metaprogramming
+mechanism to identify or verify the properties of types. This allows to provide
+the full C++ functionality without a loss of portability. To avoid unsafe use
+of the extensions it is recommended that the extensions are disabled directly
+after the header include.
+
+**Example of Use**:
+
+The example of kernel code with `type_traits` is illustrated here.
+
+.. code-block:: c++
+
+  #pragma OPENCL EXTENSION __cl_clang_function_pointers : enable
+  #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
+  #include 
+  #pragma OPENCL EXTENSION __cl_clang_function_pointers : disable
+  #pragma OPENCL EXTENSION __cl_clang_variadic_functions : disable
+
+  using sint_type = std::make_signed::type;
+
+  __kernel void foo() {
+static_assert(!std::is_same::value);
+  }
+
+The possible clang invocation to compile the example is as follows:
+
+   .. code-block:: console
+
+ $ clang -cl-std=clc++  -I/include test.cl
+
+Note that `type_traits` is a header only library and therefore no extra
+linking step against the standard libraries is required.


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -20,7 +20,7 @@
 Clang fully supports all OpenCL C versions from 1.1 to 2.0.
 
 Please refer to `Bugzilla
-`_
+`__
 for the most up to date bug reports.
 
 
@@ -45,3 +45,57 @@
 - Initialization of objects in `__constant` address spaces is not guaranteed to work.
 
 - `addrspace_cast` operator is not supported.
+
+Experimental features
+=
+
+Clang provides the following new WIP features for the developers to experiment
+and provide early feedback or contribute with further improvements.
+Feel free to contact us on `cfe-dev
+`_ or via `Bugzilla
+`__.
+
+C++ libraries for OpenCL
+
+
+There is ongoing work to support C++ standard libraries from `LLVM's libcxx
+`_ in OpenCL kernel code using C++ for OpenCL mode.
+
+It is currently possible to include `type_traits` from C++17 in the kernel
+sources when the following clang extensions are enabled
+``__cl_clang_function_pointers`` and ``__cl_clang_variadic_functions``,
+see :doc:`LanguageExtensions` for more details. The use of non-conformant
+features enabled by the extensions does not expose non-conformant behavior
+beyond the compilation i.e. does not get generated in IR or binary.
+The extension only appear in metaprogramming
+mechanism

[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

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


[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for this effort!




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11087
+// TCB warnings
+def err_tcb_conflicting_attributes : Error<
+  "attributes '%0(\"%2\")' and '%1(\"%2\")' are mutually exclusive">;

NoQ wrote:
> aaron.ballman wrote:
> > NoQ wrote:
> > > aaron.ballman wrote:
> > > > NoQ wrote:
> > > > > Do i understand correctly that while "unknown attribute" is a warning 
> > > > > ("must be an attribute for some other compiler, let's ignore"), 
> > > > > misuse of a known attribute is typically an error ("ok, whatever you 
> > > > > meant here, i have an opinion about what this really means and i 
> > > > > don't like it")?
> > > > It depends strongly on the attribute and the problem at hand. My usual 
> > > > rule of thumb is: if the attribute is being ignored because the code 
> > > > makes no sense, it's an error, but if the attribute can still be useful 
> > > > (perhaps after some adjustment), then warn. e.g., putting a calling 
> > > > convention attribute on a variable of type `int` makes no sense -- that 
> > > > should probably err rather than warn and ignore because the user did 
> > > > something baffling.
> > > > 
> > > > I could go either way on this one. Giving an error and forcing the user 
> > > > to say what they mean is appealing, but it would also be defensible to 
> > > > warn about the conflicting attribute and ignore just that one 
> > > > (retaining the original attribute). I would say leave it as an error 
> > > > and we can downgrade to a warning later if there's feedback from 
> > > > compelling real world use cases.
> > > > retaining the original attribute
> > > 
> > > There's an interesting aspect of error recovery here (which i didn't 
> > > address yet but added FIXMEs). We probably shouldn't emit additional 
> > > warnings based on the attributes about which we've already reported a 
> > > conflict error; that's just unnecessary clutter. That'd mean that we 
> > > should always discard the non-leaf attribute and keep the leaf attribute 
> > > (if we can at all discard attributes). Or keep both and repeat the 
> > > conflict check during checking in order to suppress the warning.
> > > That'd mean that we should always discard the non-leaf attribute and keep 
> > > the leaf attribute (if we can at all discard attributes). Or keep both 
> > > and repeat the conflict check during checking in order to suppress the 
> > > warning.
> > 
> > You can do `TheDecl->dropAttr();` to drop the non-leaf 
> > attributes (or, if easy, it's better to not add the problematic attribute 
> > in the first place). Would you like to do that work in this patch or as a 
> > follow-up? (I'm fine either way.)
> Hmm, that's indeed an easy and reasonable solution. Ideally i should only 
> drop attributes with the same argument but that's too far into the area of 
> diminishing returns for me :)
Yeah, I don't think we need to go that far unless there's some real world use 
cases demonstrating the need.


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

https://reviews.llvm.org/D91898

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


[PATCH] D94038: [WebAssembly] Rename wasm_rethrow_in_catch intrinsic/builtin

2021-01-08 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 315366.
aheejin added a comment.

Remove an extra word


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94038

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/test/CodeGen/builtins-wasm.c
  clang/test/CodeGenCXX/wasm-eh.cpp
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
  llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -54,7 +54,7 @@
 ; CHECK-NEXT:  call i8* @__cxa_begin_catch(i8* %[[EXN]])
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %entry, %catch
@@ -125,7 +125,7 @@
   catchret from %6 to label %try.cont7
 
 rethrow:  ; preds = %catch.start3
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %6) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %6) ]
   unreachable
 
 try.cont7:; preds = %try.cont, %catch4
@@ -189,7 +189,7 @@
   catchret from %7 to label %try.cont
 
 rethrow5: ; preds = %catch.start3
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %7) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %7) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch, %catch6
@@ -197,7 +197,7 @@
   catchret from %1 to label %try.cont9
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont9:; preds = %entry, %try.cont
@@ -274,7 +274,7 @@
   catchret from %6 to label %try.cont
 
 rethrow:  ; preds = %catch.start3
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %6) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %6) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch.start, %catch4
@@ -380,7 +380,7 @@
   catchret from %14 to label %try.cont
 
 rethrow10:; preds = %catch.start8
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %14) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %14) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch.start3, %catch11
@@ -395,7 +395,7 @@
   catchret from %1 to label %try.cont19
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont19:   ; preds = %entry, %try.cont16
@@ -604,7 +604,7 @@
 declare i32 @llvm.wasm.get.ehselector(token)
 declare i32 @llvm.eh.typeid.for(i8*)
 declare void @llvm.wasm.throw(i32, i8*)
-declare void @llvm.wasm.rethrow.in.catch()
+declare void @llvm.wasm.rethrow()
 declare i8* @__cxa_begin_catch(i8*)
 declare void @__cxa_end_catch()
 declare void @__clang_call_terminate(i8*)
Index: llvm/test/CodeGen/WebAssembly/exception.ll
===
--- llvm/test/CodeGen/WebAssembly/exception.ll
+++ llvm/test/CodeGen/WebAssembly/exception.ll
@@ -70,7 +70,7 @@
   catchret from %1 to label %try.cont
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %catch, %entry
@@ -258,7 +258,7 @@
   catchret from %1 to label %try.cont
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %invoke.cont1, %entry
@@ -368,7 +368,7 @@
 declare void @llvm.wasm.throw(i32, i8*)
 declare i8* @llvm.

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

I've set the repository to rG  
LLVM. Could you please rebase and update the patch so that the CI gets 
triggered?

I hope it is enough to just reupload a patch. I have heard that sometimes you 
might need to reupload the diff twice, once removing the repo and then setting 
it back, but I'm don't know what the correct recipe is (I always use arc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94217

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


[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

Please rebase this patch too to trigger CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94206

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


[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:893
+def warn_pragma_pack_identifer_not_supported : Warning<
+  "specifying an identifier within pragma pack is not supported, identifier is 
ignored">,
+  InGroup;

Xiangling_L wrote:
> aaron.ballman wrote:
> > If the user wrote an identifier, it seems like there's a strong chance that 
> > ignoring the identifier will result in incorrect behavior that would lead 
> > to hard-to-find ODR issues. Should this scenario be an error rather than a 
> > warning where the identifier is ignored?
> Could you show a more concrete example or give more details on how possible 
> incorrect behavior would lead to hard-to-find ODR issues?
I'm not super familiar with this feature, so what I'm thinking of may not be a 
real issue in practice. The case I was thinking of is where the user does 
`#pragma pack(push, r1, 2)` to push a packing by name followed by `#pragma 
pack(push, r2, 4)`, and the later pops come in the reverse order (`#pragma 
pack(pop, r1)` followed by `#pragma pack(pop, r2)`). The first pop will 
actually pop the `r2` packing and not the `r1` packing, which could cause the 
field layout to be different from what the user expected.



Comment at: clang/include/clang/Sema/Sema.h:524-525
+return AlignPackInfo(M, PackNumber, IsXL);
+  else
+return AlignPackInfo(M, IsXL);
+}

No `else` after a `return`.



Comment at: clang/include/clang/Sema/Sema.h:512
+static AlignPackInfo getFromRawEncoding(unsigned Encoding) {
+  static_assert(sizeof(AlignPackInfo) == sizeof(uint32_t),
+"Size is not correct");

Xiangling_L wrote:
> aaron.ballman wrote:
> > I would feel more comfortable with this assertion if the class was using 
> > bit-fields to pack the values together. As it stands, we're kind of hoping 
> > that `bool`, `Mode`, and `unsigned char` will all pack in a particular way 
> > (and `bool`'s representation is implementation-defined).
> Yeah, good point. I will move it back to previous bitfields version.
Oh, that's different from what I was expecting -- I was thinking you'd change 
the data member fields of the class to be bit-fields.

Either way works for me, though, so no change necessary unless you prefer that 
approach.



Comment at: clang/include/clang/Sema/Sema.h:527
+
+unsigned char getPackNumber() const { return PackNumber; }
+

Xiangling_L wrote:
> aaron.ballman wrote:
> > Given that the ctor takes an `int` for this value, should this be returning 
> > an `int`?
> As we know the pack number would not exceed 16 bytes, so the intention of 
> using `unsigned char` here is to save space taken by AlignPackInfo. And 
> that's why I added the diagnostics and assertion in the ctor to make sure the 
> value won't be truncated.
We don't save any space with this function signature though -- the return value 
will most likely wind up being implicitly promoted to `int` anyway. I think the 
fact that we store it as a smaller datatype internally is an implementation 
detail and as far as the consumer of this class is concerned, the pack number 
is an integer type where we don't care about the size. tbh, because the pack 
numbers are always positive, I'd have the ctor and this function both deal with 
an `unsigned` rather than an `int`.

(Note, I'm not talking about the type of the underlying field -- I think it's 
fine to continue to use a smaller datatype there for space savings. I'm only 
talking about the types in the public interface.)


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

https://reviews.llvm.org/D87702

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


[clang] e97071d - [NFC] Renaming PackStack to AlignPackStack

2021-01-08 Thread Xiangling Liao via cfe-commits

Author: Xiangling Liao
Date: 2021-01-08T09:15:11-05:00
New Revision: e97071d7952068bbb5fee7bf9e46f304044d4aee

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

LOG: [NFC] Renaming PackStack to AlignPackStack

This patch renames PackStack and related variable names to also contain align 
across Clang.
As it is right now, Clang already uses one stack to record the information from 
both #pragma
align and #pragma pack. Leaving it as PackStack is confusing, and could cause 
people to
ignore #pragma align when developing code that interacts with PackStack.

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

Added: 
clang/test/Sema/Inputs/pragma-align-pack1.h
clang/test/Sema/misleading-pragma-align-pack-diagnostics.c

Modified: 
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6b81494e8eff..acc3184aea97 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -568,14 +568,14 @@ class Sema final {
   // #pragma pack.
   // Sentinel to represent when the stack is set to mac68k alignment.
   static const unsigned kMac68kAlignmentSentinel = ~0U;
-  PragmaStack PackStack;
-  // The current #pragma pack values and locations at each #include.
-  struct PackIncludeState {
+  PragmaStack AlignPackStack;
+  // The current #pragma align/pack values and locations at each #include.
+  struct AlignPackIncludeState {
 unsigned CurrentValue;
 SourceLocation CurrentPragmaLocation;
 bool HasNonDefaultValue, ShouldWarnOnInclude;
   };
-  SmallVector PackIncludeStack;
+  SmallVector AlignPackIncludeStack;
   // Segment #pragmas.
   PragmaStack DataSegStack;
   PragmaStack BSSSegStack;
@@ -9721,14 +9721,14 @@ class Sema final {
   void ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
StringRef SlotLabel, Expr *Alignment);
 
-  enum class PragmaPackDiagnoseKind {
+  enum class PragmaAlignPackDiagnoseKind {
 NonDefaultStateAtInclude,
 ChangedStateAtExit
   };
 
-  void DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind,
-SourceLocation IncludeLoc);
-  void DiagnoseUnterminatedPragmaPack();
+  void DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind,
+ SourceLocation IncludeLoc);
+  void DiagnoseUnterminatedPragmaAlignPack();
 
   /// ActOnPragmaMSStruct - Called on well formed \#pragma ms_struct [on|off].
   void ActOnPragmaMSStruct(PragmaMSStructKind Kind);

diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index b4fd9e95d28f..e9fc202f8d1d 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -681,8 +681,8 @@ class TypeIdx {
 
   MODULAR_CODEGEN_DECLS = 60,
 
-  /// Record code for \#pragma pack options.
-  PACK_PRAGMA_OPTIONS = 61,
+  /// Record code for \#pragma align/pack options.
+  ALIGN_PACK_PRAGMA_OPTIONS = 61,
 
   /// The stack of open #ifs/#ifdefs recorded in a preamble.
   PP_CONDITIONAL_STACK = 62,

diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 94491e45b55b..1dc1be00ea3a 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -869,17 +869,17 @@ class ASTReader
   llvm::SmallVector FpPragmaStack;
   llvm::SmallVector FpPragmaStrings;
 
-  /// The pragma pack state.
-  Optional PragmaPackCurrentValue;
-  SourceLocation PragmaPackCurrentLocation;
-  struct PragmaPackStackEntry {
+  /// The pragma align/pack state.
+  Optional PragmaAlignPackCurrentValue;
+  SourceLocation PragmaAlignPackCurrentLocation;
+  struct PragmaAlignPackStackEntry {
 unsigned Value;
 SourceLocation Location;
 SourceLocation PushLocation;
 StringRef SlotLabel;
   };
-  llvm::SmallVector PragmaPackStack;
-  llvm::SmallVector PragmaPackStrings;
+  llvm::SmallVector PragmaAlignPackStack;
+  llvm::SmallVector PragmaAlignPackStrings;
 
   /// The OpenCL extension settings.
   OpenCLOptions OpenCLExtensions;

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 83df76b967c4..975fe9b81250 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -120,8 +120,9 @@ class SemaPPCallbacks : public PPCallbacks {
 }
 
 IncludeStack.push_back(IncludeLoc);
-S->DiagnoseNonDef

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

2021-01-08 Thread Xiangling Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe97071d79520: [NFC] Renaming PackStack to AlignPackStack 
(authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93901

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/Inputs/pragma-align-pack1.h
  clang/test/Sema/misleading-pragma-align-pack-diagnostics.c

Index: clang/test/Sema/misleading-pragma-align-pack-diagnostics.c
===
--- /dev/null
+++ clang/test/Sema/misleading-pragma-align-pack-diagnostics.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DALIGN_SET_HERE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DRECORD_ALIGN -verify %s
+
+#ifdef ALIGN_SET_HERE
+#pragma align = natural // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
+// expected-warning@+9 {{the current #pragma pack alignment value is modified in the included file}}
+#endif
+
+#ifdef RECORD_ALIGN
+#pragma align = mac68k
+// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}}
+// expected-warning@+3 {{non-default #pragma pack value changes the alignment of struct or union members in the included file}}
+#endif
+
+#include "pragma-align-pack1.h"
+
+#ifdef RECORD_ALIGN
+#pragma align = reset
+#endif
Index: clang/test/Sema/Inputs/pragma-align-pack1.h
===
--- /dev/null
+++ clang/test/Sema/Inputs/pragma-align-pack1.h
@@ -0,0 +1,11 @@
+#ifdef ALIGN_SET_HERE
+#pragma align = mac68k
+// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}}
+// expected-warning@-2 {{unterminated '#pragma pack (push, ...)' at end of file}}
+#endif
+
+#ifdef RECORD_ALIGN
+struct S {
+  int x;
+};
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4151,24 +4151,24 @@
   Stream.EmitRecord(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS, Record);
 }
 
-/// Write the state of 'pragma pack' at the end of the module.
+/// Write the state of 'pragma align/pack' at the end of the module.
 void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) {
-  // Don't serialize pragma pack state for modules, since it should only take
-  // effect on a per-submodule basis.
+  // Don't serialize pragma align/pack state for modules, since it should only
+  // take effect on a per-submodule basis.
   if (WritingModule)
 return;
 
   RecordData Record;
-  Record.push_back(SemaRef.PackStack.CurrentValue);
-  AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record);
-  Record.push_back(SemaRef.PackStack.Stack.size());
-  for (const auto &StackEntry : SemaRef.PackStack.Stack) {
+  Record.push_back(SemaRef.AlignPackStack.CurrentValue);
+  AddSourceLocation(SemaRef.AlignPackStack.CurrentPragmaLocation, Record);
+  Record.push_back(SemaRef.AlignPackStack.Stack.size());
+  for (const auto &StackEntry : SemaRef.AlignPackStack.Stack) {
 Record.push_back(StackEntry.Value);
 AddSourceLocation(StackEntry.PragmaLocation, Record);
 AddSourceLocation(StackEntry.PragmaPushLocation, Record);
 AddString(StackEntry.StackSlotLabel, Record);
   }
-  Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record);
+  Stream.EmitRecord(ALIGN_PACK_PRAGMA_OPTIONS, Record);
 }
 
 /// Write the state of 'pragma float_control' at the end of the module.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3742,25 +3742,25 @@
   ForceCUDAHostDeviceDepth = Record[0];
   break;
 
-case PACK_PRAGMA_OPTIONS: {
+case ALIGN_PACK_PRAGMA_OPTIONS: {
   if (Record.size() < 3) {
 Error("invalid pragma pack record");
 return Failure;
   }
-  PragmaPackCurrentValue = Record[0];
-  PragmaPackCurrentLocation = ReadSourceLocation(F, Record[1]);
+  PragmaAlignPackCurrentValue = Record[0];
+  PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]);
   unsigned NumStackEntries = Record[2];
   unsigned Idx = 3;
   // Reset the stack when importing a new module.
-  PragmaPackStack.clear();
+  PragmaAlignPackStack.clear();
   for (unsigned I = 0; I < NumStackEntries; ++I) {
-PragmaPackStackEntry Entry;
+PragmaAlignPackStackEntry Entry;

[PATCH] D94293: [clangd] automatically index STL

2021-01-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D94293#2486468 , @njames93 wrote:

> How does this approach work with differing language standards. For example 
> with an old implementation designed for c++14, `string_view` header won't 
> exist. If the implementation is designed to work with c++17, including that 
> header in c++14 mode will probably be ifdef... Error.

Yeah, this seems at least annoying. There's some spectrum of:

- build indexes per language version and dynamically switch between them based 
on the file
- make this variable in some global way (e.g. a flag, or first file opened 
chooses the language version)
- language version is always (e.g.) c++14, hopefully it's better than nothing

Similar to the no-stdlib question and the multiple-languages question, this 
points to a separate index layer being a stronger design to allow dynamically 
swapping it (though worse can be better, too).




Comment at: clang-tools-extra/clangd/ClangdServer.h:161
+// insertion
+bool IndexSTL = false;
+

nit: "the standard library"

for two reasons:
 - "STL" is no longer the correct name, even for C++
 - languages other than C++ have standard libraries (C in particular) that we 
could also control with this flag



Comment at: clang-tools-extra/clangd/index/Background.cpp:451
+  //   indexer?
+  const Path STLHeaderPath = SourceRoot + "/.stl_header.h";
+  vlog("Indexing STL headers from {0}", STLHeaderPath);

separate from the issue of virtual vs real file, it's not clear to me why we 
*want* this file to be close to the project rather than in some completely 
anonymous location.

(I have a few guesses, but...)



Comment at: clang-tools-extra/clangd/index/Background.h:160
+// TODO(kuhnel): Only index if enqueueing C++ file.
+indexSTLHeaders(ChangedFiles);
   }

this can happen 0+ times (roughly it happens once every time an enumerable CDB 
is discovered).

Instead, we'd want it to happen exactly once, or possibly 0-1 times if we're 
going to be language-sensitive.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
 
+opt IndexSTL{
+"index-stl",

kuhnel wrote:
> kadircet wrote:
> > do we really need to hide this behind a flag ? it sounds like a quite 
> > useful feature to me with minimal risk of regressions and unlikely to make 
> > anyone upset. in the end we are not doing anything heuristically and just 
> > throwing clang on some STL headers, that'll probably be included within the 
> > TUs eventually.
> > 
> > there's always the risk of crashing while parsing some STL headers, but i 
> > don't think it is any different than user just including the header 
> > manually.
> > 
> > the biggest problem i can see is people using custom STL headers, but 
> > hopefully compile flags interpolation logic should be able to infer the 
> > relevant location for those. and in the cases it fails we either index the 
> > default STL and suggest people some symbols their implementation might 
> > lack, or fail to find STL at all and print some logs for the missing 
> > includes.
> Back when I was developing embedded software, we couldn't use the STL for 
> various reasons (object size, no exceptions, no dynamic memory, ...) and had 
> our own, proprietary base library. In such a scenario it would be annoying if 
> clangd would be proposing things I could not use in the project. 
> 
> So we should have a way for users to disable this. I'm fine if we switch it 
> on by default and offer a `--disable-stl-index` flag instead.
Yeah, this is a good point.

Having this configurable would be valuable, but the useful granularity for such 
an option is per-file (Config) rather than a global clangd flag, as it's 
codebase-specific.

Unfortunately that doesn't really square with having it in the background index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94293

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


[PATCH] D94289: [clangd] Add go-to-def metric.

2021-01-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/XRefs.cpp:375
   VirtualMethods.insert(getSymbolID(CMD));
+  LocateASTReferentMetric.record(1, "go-to-overrides");
+}

name could be a bit more explicit, particularly I'd confuse it with go-to on 
the override keyword.

Maybe `method-to-override` and `method-to-base`?



Comment at: clang-tools-extra/clangd/XRefs.cpp:407
 // it's more useful to navigate to the template declaration.
 if (auto *CTSD = dyn_cast(D)) {
   if (TouchedIdentifier &&

template-specialization-to-primary?



Comment at: clang-tools-extra/clangd/XRefs.cpp:426
  ID->getName() == TouchedIdentifier->text(SM)))
   AddResultDecl(ID);
 

objc-category-to-class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94289

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


[PATCH] D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what th

2021-01-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Yes, I was getting deja vu reading the description. Thanks though!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94265

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 315373.
serge-sans-paille added a comment.

Update codebase and testbed to reflect recent discussion.


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }// expected-warning {{'foo__bar' is a reserved identifier}}
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _barbouille() { return 0; } // expected-warning {{'_barbouille' is a reserved identifier}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template  constexpr bool __toucan = true; // expected-warning {{'__toucan' is a reserved identifier}}
+
+template 
+concept _Barbotine = __toucan; // expected-warning {{'_Barbotine' is a reserved identifier}}
+
+template  // expected-warning {{'__' is a reserved identifier}}
+struct BarbeNoire {};
+
+template  // expected-warning {{'__' is a reserved identifier}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{'_Barbidur' is a reserved identifier}}
+
+struct __barbidou {}; // expected-warning {{'__barbidou' is a reserved identifier}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{'__barbouille' is a reserved identifier}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{'__babar' is a reserved identifier}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa { // expected-warning {{'__barbapapa' is a reserved identifier}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;   // expected-warning {{'_Barbalala' is a reserved identifier}}
+};
+
+enum class __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some,   // expected-warning {{'__some' is a reserved identifier}}
+  _Other,   // expected-warning {{'_Other' is a reserved identifier}}
+  _other// no-warning
+};
+
+enum _Menu { // expected-warning {{'_Menu' is a reserved identifier}}
+  _OtheR_,   // expected-warning {{'_OtheR_' is a reserved identifier}}
+  _other_// expected-warning {{'_other_' is a reserved identifier}}
+};
+
+enum {
+  __some, // expected-warning {{'__some' is a reserved identifier}}
+  _Other, // expected-warning {{'_Other' is a reserved identifier}}
+  _other  // expected-warning {{'_other' is a reserved identifier}}
+};
+
+static union {
+  int _barbeFleurie; // expected-warning {{'_barbeFleurie' is a reserved identifier}}
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{'_Barbamama' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+// we skip this one because it's not at top-level.
+int _barbatruc; // expected-warning {{'_barbatruc' is a reserved identifier}}
+}
+
+long double operator"" _BarbeBleue(long double) // no-warning
+{
+  return 0.;
+}
Index: clang/test/Sema/reserved-identifier.c
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wreserved-identifier -Wno-visibility %s
+
+#define __oof foo__ // expected-warning {{macro name is a reserved identifier}}
+
+int foo__bar() { return 0; }// no-warning
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _foo() { return 0; }// expected-warning {{'_foo' is a reserved identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+  goto __reserved;
+__reserved: // expected-warning {{'__re

[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.



Comment at: clang/test/Sema/reserved-identifier.cpp:58
+// we skip this one because it's not at top-level.
+int _barbatruc; // no-warning
+}

aaron.ballman wrote:
> This is another case that I'm on the fence about failing to warn on because 
> the name `_barbatruc` would conflict with a name introduced by the 
> implementation. Another interesting variant of the same problem, for C++:
> ```
> static union {
>   int _field;
> };
> ```
> I wonder if the rule should not be whether the declaration is at file scope 
> or not, but whether the declared identifier can be found at file scope or not?
> whether the declared identifier can be found at file scope or not?

100% agree. As

```
static union {
  int _field;
};
int _field;
```

is invalid, I considered that one and tested it.



Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}

aaron.ballman wrote:
> aaron.ballman wrote:
> > You should also have some tests for:
> > ```
> > template 
> > void _Foobar(); // Even though it's not instantiated, it's still reserved.
> > 
> > template  // Reserved
> > void whatever();
> > 
> > void func() {
> >   int array[10];
> >   for (auto _A : array) // Reserved
> > ;
> > }
> > 
> > class _C { // Reserved
> > public:
> >   _C(); // Not reserved
> > };
> > 
> > unsigned operator "" huttah(unsigned long long); // Reserved 
> > (http://eel.is/c++draft/usrlit.suffix#1)
> > 
> > unsigned operator "" _W(unsigned long long); // Reserved
> > unsigned operator "" _w(unsigned long long); // Reserved
> > 
> > static unsigned operator "" _X(unsigned long long); // Not reserved
> > static unsigned operator "" _x(unsigned long long); // Not reserved
> > ```
> I think some of these tests are still missing. I'm especially worried about 
> the user-defined literal cases being diagnosed, as we'd be warning to not do 
> the exact thing users are expected to do.
User defined literal tested below and behave as expected.


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

https://reviews.llvm.org/D93095

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


[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

2021-01-08 Thread Steve Scalpone via Phabricator via cfe-commits
sscalpone added a comment.

@awarzynski, perhaps I am being too dramatic by wondering if we are 
compromising support for one or the other language in order to support a common 
definition.  In the case of -I, there are language-specific semantics that are 
important and interesting, such as the relationship in clang between -I and 
-isystem, and the relationship in flang between -I and -module.  Maybe the 
right answer is to allow the definition of duplicate options that are 
differentiated by language?

+1 @andreil99


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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


[PATCH] D75041: [clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'

2021-01-08 Thread Whisperity via Phabricator via cfe-commits
whisperity updated this revision to Diff 315384.
whisperity added a comment.
Herald added a subscriber: shchenz.

**Ignore one-way implicit conversions**

One-way implicit conversions introduce too much noise, and except in very odd 
circumstances, do not provide a meaningful result.

The stalwart example

  void f(const Base& bp, const Derived& dp)

contains just one conversion, `dp -> bp`, which, if called in a swapped way, 
will 99.9...% of the cases will result in a compile error (*)

  Base b;
  Derived d;
  f(d, b);

Thus, there is no reason to model this, at all.

(*): Unless an `operator Derived()` exists in `Base`.


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

https://reviews.llvm.org/D75041

Files:
  
clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
  
clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst
  
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-implicits.c
  
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-implicits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-minlen3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c

Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
===
--- clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
@@ -32,3 +32,6 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent parameters for 'equals2' of similar type ('S') are
 // CHECK-MESSAGES: :[[@LINE-2]]:15: note: the first parameter in this range is 'l'
 // CHECK-MESSAGES: :[[@LINE-3]]:20: note: the last parameter in this range is 'r'
+
+void ptrs(int *i, long *l) {}
+// NO-WARN: Mixing fundamentals' pointers is diagnosed by compiler warnings.
Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-minlen3.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-minlen3.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-minlen3.cpp
@@ -203,3 +203,20 @@
 // CHECK-MESSAGES: :[[@LINE-6]]:25: note: after resolving type aliases, type of parameter 'Background' is 'OldSchoolTermColour'
 // CHECK-MESSAGES: :[[@LINE-6]]:25: note: after resolving type aliases, type of parameter 'Foreground' is 'OldSchoolTermColour'
 // CHECK-MESSAGES: :[[@LINE-6]]:25: note: after resolving type aliases, type of parameter 'Border' is 'OldSchoolTermColour'
+
+// NO-WARN: Implicit conversions should not warn if the check option is turned off.
+
+void integral1(signed char sc, int si) {}
+
+struct FromInt {
+  FromInt(int);
+};
+void userConv1(int i, FromInt fi) {}
+
+struct Base {};
+struct Derived1 : Base {};
+struct Derived2 : Base {};
+void upcasting(Base *bp, Derived1 *d1p, Derived2 *d2p) {}
+void upcasting_ref(const Base &br, const Derived1 &d1r, const Derived2 &d2r) {}
+
+// END of NO-WARN.
Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-implicits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-implicits.cpp
@@ -0,0 +1,308 @@
+// RUN: %check_clang_tidy %s \
+// RUN:   experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.ImplicitConversion, value: 1} \
+// RUN:   ]}' --
+
+void compare(int a, int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 3 adjacent parameters for 'compare' of similar type ('int') are easily swapped by mistake [experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-3]]:32: note: the last parameter in this range is 'c'
+
+void b

[clang] 7be2715 - [WebAssembly] Rename wasm_rethrow_in_catch intrinsic/builtin

2021-01-08 Thread Heejin Ahn via cfe-commits

Author: Heejin Ahn
Date: 2021-01-08T06:55:04-08:00
New Revision: 7be271537e97018c56a714a90106f1e25e32f4db

URL: 
https://github.com/llvm/llvm-project/commit/7be271537e97018c56a714a90106f1e25e32f4db
DIFF: 
https://github.com/llvm/llvm-project/commit/7be271537e97018c56a714a90106f1e25e32f4db.diff

LOG: [WebAssembly] Rename wasm_rethrow_in_catch intrinsic/builtin

`wasm_rethrow_in_catch` intrinsic and builtin are used in order to
rethrow an exception when the exception is caught but there is no
matching clause within the current `catch`. For example,
```
try {
  foo();
} catch (int n) {
  ...
}
```
If the caught exception does not correspond to C++ `int` type, it should
be rethrown. These intrinsic/builtin were renamed `rethrow_in_catch`
because at the time I thought there would be another intrinsic for C++'s
`throw` keyword, which rethrows an exception. It turned out that `throw`
keyword doesn't require wasm's `rethrow` instruction, so we rename
`rethrow_in_catch` to just `rethrow` here.

Reviewed By: dschuff, tlively

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsWebAssembly.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGException.cpp
clang/test/CodeGen/builtins-wasm.c
clang/test/CodeGenCXX/wasm-eh.cpp
llvm/include/llvm/IR/IntrinsicsWebAssembly.td
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
llvm/test/CodeGen/WebAssembly/exception.ll
llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsWebAssembly.def 
b/clang/include/clang/Basic/BuiltinsWebAssembly.def
index 84482082095e..080c6b5c3a40 100644
--- a/clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ b/clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -38,7 +38,7 @@ BUILTIN(__builtin_wasm_max_f64, "ddd", "nc")
 
 // Exception handling builtins.
 TARGET_BUILTIN(__builtin_wasm_throw, "vIUiv*", "r", "exception-handling")
-TARGET_BUILTIN(__builtin_wasm_rethrow_in_catch, "v", "r", "exception-handling")
+TARGET_BUILTIN(__builtin_wasm_rethrow, "v", "r", "exception-handling")
 
 // Atomic wait and notify.
 TARGET_BUILTIN(__builtin_wasm_memory_atomic_wait32, "ii*iLLi", "n", "atomics")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index ea39d64e16f1..cf84ad34e1ec 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -16583,8 +16583,8 @@ Value 
*CodeGenFunction::EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
 Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_throw);
 return Builder.CreateCall(Callee, {Tag, Obj});
   }
-  case WebAssembly::BI__builtin_wasm_rethrow_in_catch: {
-Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow_in_catch);
+  case WebAssembly::BI__builtin_wasm_rethrow: {
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
   case WebAssembly::BI__builtin_wasm_memory_atomic_wait32: {

diff  --git a/clang/lib/CodeGen/CGException.cpp 
b/clang/lib/CodeGen/CGException.cpp
index f8a486909e41..7a64963183bc 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -1272,7 +1272,7 @@ void CodeGenFunction::ExitCXXTryStmt(const CXXTryStmt &S, 
bool IsFnTryBlock) {
 assert(RethrowBlock != WasmCatchStartBlock && RethrowBlock->empty());
 Builder.SetInsertPoint(RethrowBlock);
 llvm::Function *RethrowInCatchFn =
-CGM.getIntrinsic(llvm::Intrinsic::wasm_rethrow_in_catch);
+CGM.getIntrinsic(llvm::Intrinsic::wasm_rethrow);
 EmitNoreturnRuntimeCallOrInvoke(RethrowInCatchFn, {});
   }
 

diff  --git a/clang/test/CodeGen/builtins-wasm.c 
b/clang/test/CodeGen/builtins-wasm.c
index 83924b48542e..d8b61f5d285e 100644
--- a/clang/test/CodeGen/builtins-wasm.c
+++ b/clang/test/CodeGen/builtins-wasm.c
@@ -49,10 +49,10 @@ void throw(void *obj) {
   // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
 }
 
-void rethrow_in_catch(void) {
-  return __builtin_wasm_rethrow_in_catch();
-  // WEBASSEMBLY32: call void @llvm.wasm.rethrow.in.catch()
-  // WEBASSEMBLY64: call void @llvm.wasm.rethrow.in.catch()
+void rethrow(void) {
+  return __builtin_wasm_rethrow();
+  // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
+  // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
 
 int memory_atomic_wait32(int *addr, int expected, long long timeout) {

diff  --git a/clang/test/CodeGenCXX/wasm-eh.cpp 
b/clang/test/CodeGenCXX/wasm-eh.cpp
index cc467b42beee..d3c8400124b3 100644
--- a/clang/test/CodeGenCXX/wasm-eh.cpp
+++ b/clang/test/CodeGenCXX/wasm-eh.cpp
@@ -63,7 +63,7 @@ void test0() {
 // CHECK-NEXT:   br label %[[TRY_CONT_BB]]
 
 // CHECK: [[RETHROW_BB]]:
-// CHECK-NEXT:   call 

[PATCH] D94038: [WebAssembly] Rename wasm_rethrow_in_catch intrinsic/builtin

2021-01-08 Thread Heejin Ahn via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7be271537e97: [WebAssembly] Rename wasm_rethrow_in_catch 
intrinsic/builtin (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94038

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/test/CodeGen/builtins-wasm.c
  clang/test/CodeGenCXX/wasm-eh.cpp
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
  llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -54,7 +54,7 @@
 ; CHECK-NEXT:  call i8* @__cxa_begin_catch(i8* %[[EXN]])
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %entry, %catch
@@ -125,7 +125,7 @@
   catchret from %6 to label %try.cont7
 
 rethrow:  ; preds = %catch.start3
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %6) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %6) ]
   unreachable
 
 try.cont7:; preds = %try.cont, %catch4
@@ -189,7 +189,7 @@
   catchret from %7 to label %try.cont
 
 rethrow5: ; preds = %catch.start3
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %7) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %7) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch, %catch6
@@ -197,7 +197,7 @@
   catchret from %1 to label %try.cont9
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont9:; preds = %entry, %try.cont
@@ -274,7 +274,7 @@
   catchret from %6 to label %try.cont
 
 rethrow:  ; preds = %catch.start3
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %6) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %6) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch.start, %catch4
@@ -380,7 +380,7 @@
   catchret from %14 to label %try.cont
 
 rethrow10:; preds = %catch.start8
-  invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %14) ]
+  invoke void @llvm.wasm.rethrow() [ "funclet"(token %14) ]
   to label %unreachable unwind label %ehcleanup
 
 try.cont: ; preds = %catch.start3, %catch11
@@ -395,7 +395,7 @@
   catchret from %1 to label %try.cont19
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont19:   ; preds = %entry, %try.cont16
@@ -604,7 +604,7 @@
 declare i32 @llvm.wasm.get.ehselector(token)
 declare i32 @llvm.eh.typeid.for(i8*)
 declare void @llvm.wasm.throw(i32, i8*)
-declare void @llvm.wasm.rethrow.in.catch()
+declare void @llvm.wasm.rethrow()
 declare i8* @__cxa_begin_catch(i8*)
 declare void @__cxa_end_catch()
 declare void @__clang_call_terminate(i8*)
Index: llvm/test/CodeGen/WebAssembly/exception.ll
===
--- llvm/test/CodeGen/WebAssembly/exception.ll
+++ llvm/test/CodeGen/WebAssembly/exception.ll
@@ -70,7 +70,7 @@
   catchret from %1 to label %try.cont
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %catch, %entry
@@ -258,7 +258,7 @@
   catchret from %1 to label %try.cont
 
 rethrow:  ; preds = %catch.start
-  call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
+  call void @llvm.wasm.rethrow() [ "funclet"(token %1) ]
   unreachable
 
 try.cont: ; preds = %inv

[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}

serge-sans-paille wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > You should also have some tests for:
> > > ```
> > > template 
> > > void _Foobar(); // Even though it's not instantiated, it's still reserved.
> > > 
> > > template  // Reserved
> > > void whatever();
> > > 
> > > void func() {
> > >   int array[10];
> > >   for (auto _A : array) // Reserved
> > > ;
> > > }
> > > 
> > > class _C { // Reserved
> > > public:
> > >   _C(); // Not reserved
> > > };
> > > 
> > > unsigned operator "" huttah(unsigned long long); // Reserved 
> > > (http://eel.is/c++draft/usrlit.suffix#1)
> > > 
> > > unsigned operator "" _W(unsigned long long); // Reserved
> > > unsigned operator "" _w(unsigned long long); // Reserved
> > > 
> > > static unsigned operator "" _X(unsigned long long); // Not reserved
> > > static unsigned operator "" _x(unsigned long long); // Not reserved
> > > ```
> > I think some of these tests are still missing. I'm especially worried about 
> > the user-defined literal cases being diagnosed, as we'd be warning to not 
> > do the exact thing users are expected to do.
> User defined literal tested below and behave as expected.
@aaron.ballman, the "not reserved" comment re: `_X` for the literal operator 
using the `operator string-literal identifier` form above seems suspect to me. 
See `_Bq` example in [over.literal].


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

https://reviews.llvm.org/D93095

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/test/Sema/reserved-identifier.cpp:77
+
+long double operator"" _BarbeBleue(long double) // no-warning
+{

This should get a warning, since it's using an identifier "reserved for any 
use."
https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier
http://eel.is/c++draft/lex.name#3.1
If the implementation predefines `#define _BarbeBleue 42` (which the 
implementation is permitted to do), then this code won't compile.


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

https://reviews.llvm.org/D93095

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


[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

2021-01-08 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

FWIW I think it would be good to have a bit more details in the description for 
changes such as this, like a link to the public docs for the extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94083

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


[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

2021-01-08 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

I was just about to write an issue to the CoreGuidelines project, but I was 
@vingeldal has posted about "relatedness" before me: 
https://github.com/isocpp/CppCoreGuidelines/issues/1579. It was in the list of 
//closed// issues.

Herb Sutter has changed the rule's title to //"Avoid adjacent parameters of the 
same type when changing the argument order would change meaning"//. Now the 
phrase `related` has been removed from the title, and the body of the rule does 
not contain any mention to this phrase either.

//`change of meaning`// is still hard to prove from a Tidy standpoint (or could 
be incomputable to prove in the first place...), but a concept that can be 
grasped with much less mental effort.

The guideline itself still says that the //Enforcement// is to simply warn if 
two parameters have the same type. We are doing much more here already with 
making the Tidy output much more user friendly and less bloatful.


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

https://reviews.llvm.org/D69560

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 315390.
serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added a comment.

Ignore forward declaration of tagdecl


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }// expected-warning {{'foo__bar' is a reserved identifier}}
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _barbouille() { return 0; } // expected-warning {{'_barbouille' is a reserved identifier}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template  constexpr bool __toucan = true; // expected-warning {{'__toucan' is a reserved identifier}}
+
+template 
+concept _Barbotine = __toucan; // expected-warning {{'_Barbotine' is a reserved identifier}}
+
+template  // expected-warning {{'__' is a reserved identifier}}
+struct BarbeNoire {};
+
+template  // expected-warning {{'__' is a reserved identifier}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{'_Barbidur' is a reserved identifier}}
+
+struct __barbidou {}; // expected-warning {{'__barbidou' is a reserved identifier}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{'__barbouille' is a reserved identifier}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{'__babar' is a reserved identifier}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa { // expected-warning {{'__barbapapa' is a reserved identifier}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;   // expected-warning {{'_Barbalala' is a reserved identifier}}
+};
+
+enum class __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some,   // expected-warning {{'__some' is a reserved identifier}}
+  _Other,   // expected-warning {{'_Other' is a reserved identifier}}
+  _other// no-warning
+};
+
+enum _Menu { // expected-warning {{'_Menu' is a reserved identifier}}
+  _OtheR_,   // expected-warning {{'_OtheR_' is a reserved identifier}}
+  _other_// expected-warning {{'_other_' is a reserved identifier}}
+};
+
+enum {
+  __some, // expected-warning {{'__some' is a reserved identifier}}
+  _Other, // expected-warning {{'_Other' is a reserved identifier}}
+  _other  // expected-warning {{'_other' is a reserved identifier}}
+};
+
+static union {
+  int _barbeFleurie; // expected-warning {{'_barbeFleurie' is a reserved identifier}}
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{'_Barbamama' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+// we skip this one because it's not at top-level.
+int _barbatruc; // expected-warning {{'_barbatruc' is a reserved identifier}}
+}
+
+long double operator"" _BarbeBleue(long double) // no-warning
+{
+  return 0.;
+}
Index: clang/test/Sema/reserved-identifier.c
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wreserved-identifier -Wno-visibility %s
+
+#define __oof foo__ // expected-warning {{macro name is a reserved identifier}}
+
+int foo__bar() { return 0; }// no-warning
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _foo() { return 0; }// expected-warning {{'_foo' is a reserved identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =   // expected-warning {{'__1' is a reserved identifier}}
+  _Reserved;   // no-warning
+  goto __reserved;
+__reser

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

2021-01-08 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

In D86841#2484705 , @atmnpatel wrote:

> I'm happy to add a patch amending this, the reason it wasn't done that way 
> was because at the time and even now, out of icc/clang/msvc/gcc, gcc seems to 
> be the only one that happily removed such loops in C++ 
> (https://godbolt.org/z/W9vj99), and I didn't have a particularly strong 
> opinion on which way we should lean at the time.

I think ideally we would make this decision on what the standard allows. AFAICT 
the C++ spec for iteration statements (http://eel.is/c++draft/stmt.iter) does 
not have the same escape hatch as C. So IMO the current behavior is surprising 
and potentially leads to confusion for users (e.g. wondering why are some loops 
removed, but others not, even if the C++ spec allows for both to be removed).

There may be practical reasons for opting to be more conservative (e.g. if it 
would break a large number of user codes), but so far there seems to be no such 
indication in the discussion. It would probably be good to also implement 
`-ffinite-loops` as suggested by @xbolva00 so users can better control this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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


[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 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-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:68
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");

can you also test for shortening? e.g.

```
void ns::Func() { aut^o x = new ns::Class::Nested{}; }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

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


[PATCH] D94039: [WebAssembly] Update WasmEHPrepare for the new spec

2021-01-08 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin marked an inline comment as done.
aheejin added a comment.

In D94039#2480002 , @tlively wrote:

> In the description I think "but LLVM does not have a way of that kind of 
> behavior" is missing the word "modeling" => "but LLVM does not have a way of 
> modeling that kind of behavior"

Thanks. Fixed.




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td:166
 let isCodeGenOnly = 1, hasSideEffects = 1 in
-defm EXTRACT_EXCEPTION_I32 : NRI<(outs I32:$dst), (ins),
- [(set I32:$dst, 
(int_wasm_extract_exception))],
+defm EXTRACT_EXCEPTION_I32 : NRI<(outs I32:$dst), (ins), [],
  "extract_exception\t$dst">;

tlively wrote:
> Why do we need to keep this instruction definition? Is it removed in a later 
> diff in this stack?
It is removed in D94041. I tried to keep removal of `exnref` and related 
instructions (`br_on_exn`, and also this, because this pseudoinstruction is 
used to support `br_on_exn`) in a separate CL because it was all mechanical 
stuff but touched a broad range of files, so mixing it with CLs that actually 
change stuff could look confusing.



Comment at: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll:611
 declare void @__clang_call_terminate(i8*)
+declare void @_ZSt9terminatev()
 

tlively wrote:
> What is this addition for?
I think I added this in relation to the terminate pad handling (D94050) but 
while slicing up the whole thing into several CLs this was somehow put into 
this CL. But come to think of it I think this is not really necessary even in 
D94050 because we don't run `llc` on this file anyway. Will delete this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94039

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


[PATCH] D94039: [WebAssembly] Update WasmEHPrepare for the new spec

2021-01-08 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 315392.
aheejin marked an inline comment as done.
aheejin added a comment.

Delete `declare void @_ZSt9terminatev()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94039

Files:
  clang/test/CodeGenCXX/wasm-eh.cpp
  llvm/include/llvm/CodeGen/WasmEHFuncInfo.h
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/CodeGen/WasmEHPrepare.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
  llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
  llvm/test/CodeGen/WebAssembly/eh-lsda.ll
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -37,7 +37,7 @@
   br i1 %matches, label %catch, label %rethrow
 ; CHECK: catch.start:
 ; CHECK-NEXT:   %[[CATCHPAD:.*]] = catchpad
-; CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.extract.exception()
+; CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.catch(i32 0)
 ; CHECK-NEXT:   call void @llvm.wasm.landingpad.index(token %[[CATCHPAD]], i32 0)
 ; CHECK-NEXT:   store i32 0, i32* getelementptr inbounds ({ i32, i8*, i32 }, { i32, i8*, i32 }* @__wasm_lpad_context, i32 0, i32 0)
 ; CHECK-NEXT:   %[[LSDA:.*]] = call i8* @llvm.wasm.lsda()
@@ -473,7 +473,7 @@
   unreachable
 ; CHECK: terminate:
 ; CHECK-NEXT: cleanuppad
-; CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.extract.exception
+; CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.catch
 ; CHECK-NEXT:   call void @__clang_call_terminate(i8* %[[EXN]])
 }
 
Index: llvm/test/CodeGen/WebAssembly/exception.ll
===
--- llvm/test/CodeGen/WebAssembly/exception.ll
+++ llvm/test/CodeGen/WebAssembly/exception.ll
@@ -1,5 +1,9 @@
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling -verify-machineinstrs | FileCheck -allow-deprecated-dag-overlap %s
-; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling
+; TODO Reenable disabled lines after updating the backend to the new spec
+; R UN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling -verify-machineinstrs | FileCheck -allow-deprecated-dag-overlap %s
+; R UN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling
+
+; FIXME A temporary RUN line to make the test pass. Remove this later.
+; RUN: true
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
Index: llvm/test/CodeGen/WebAssembly/eh-lsda.ll
===
--- llvm/test/CodeGen/WebAssembly/eh-lsda.ll
+++ llvm/test/CodeGen/WebAssembly/eh-lsda.ll
@@ -1,4 +1,9 @@
-; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling | FileCheck -allow-deprecated-dag-overlap %s
+; TODO Reenable disabled lines after updating the backend to the new spec
+; R UN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling | FileCheck -allow-deprecated-dag-overlap %s
+
+; FIXME A temporary RUN line to make the test pass. Remove this later.
+; RUN: true
+
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
Index: llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
===
--- llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
+++ llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
@@ -1,10 +1,14 @@
 ; REQUIRES: asserts
-; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling | FileCheck %s
-; RUN: llc < %s -disable-wasm-fallthrough-return-opt -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling
-; RUN: llc < %s -O0 -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -verify-machineinstrs -exception-model=wasm -mattr=+exception-handling | FileCheck %s --check-prefix=NOOPT
-; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-kee

[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

2021-01-08 Thread Momchil Velikov via Phabricator via cfe-commits
chill added a comment.

In D94083#2486800 , @fhahn wrote:

> FWIW I think it would be good to have a bit more details in the description 
> for changes such as this, like a link to the public docs for the extension.

I'm sorry, I assumed this information was shared when the extension itself 
((this patch just adds the command line options),
was added to LLVM three years ago in https://reviews.llvm.org/D36517

The public documentation is in the Armv8-A ARM at 
https://developer.arm.com/documentation/ddi0487/latest
in section "D5 .1.5 Pointer authentication in 
AArch64 state"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94083

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


[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz updated this revision to Diff 315398.
adamcz marked an inline comment as done.
adamcz added a comment.

final review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1225,6 +1225,9 @@
   if (DC->isFunctionOrMethod())
 return;
 
+  if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
+return;
+
   if (const auto *NS = dyn_cast(DC)) {
 if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
   return AppendScope(DC->getParent(), OS, NameInScope);
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -18,6 +18,7 @@
 
 namespace clang {
 
+class DeclContext;
 class LangOptions;
 class SourceManager;
 class Stmt;
@@ -39,6 +40,15 @@
   virtual std::string remapPath(StringRef Path) const {
 return std::string(Path);
   }
+
+  /// When printing type to be inserted into code in specific context, this
+  /// callback can be used to avoid printing the redundant part of the
+  /// qualifier. For example, when inserting code inside namespace foo, we
+  /// should print bar::SomeType instead of foo::bar::SomeType.
+  /// To do this, shouldPrintScope should return true on "foo" NamespaceDecl.
+  /// The printing stops at the first isScopeVisible() == true, so there will
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *NS) const { return false; }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
+  // Check const qualifier with namespace
+  std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+  [[return y.z + y.z;]]
+})cpp";
+  std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+  return extracted(y);
+})cpp";
+  EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@
   EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
 R"cpp(const char * x = "test";)cpp");
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+  EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+"void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; }");
+
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
   // expanding types in structured bindings is syntactically invalid.
   EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@
   return SymbolID(USR);
 }
 
-// FIXME: This should be handled while printing underlying decls instead.
 std::string printType(const QualType QT, const DeclContext &CurContext) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  auto Decls =
-  explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
-  if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
-   Decls.front(),
-   /*VisibleNamespaces=*/llvm::ArrayRef{});
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
-  PP.SuppressScope = true;
   PP.SuppressTagKeyword = true;
+  PP.SuppressUnwrittenScope = true;
+
+  class PrintCB : public PrintingCallbacks {
+  public:
+PrintCB(const DeclContext *CurContext) : CurContext(CurC

[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:51
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *NS) const { return false; }
 };

nit: s/NS/DC/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

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


[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz updated this revision to Diff 315401.
adamcz added a comment.

s/NS/DC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1225,6 +1225,9 @@
   if (DC->isFunctionOrMethod())
 return;
 
+  if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
+return;
+
   if (const auto *NS = dyn_cast(DC)) {
 if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
   return AppendScope(DC->getParent(), OS, NameInScope);
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -18,6 +18,7 @@
 
 namespace clang {
 
+class DeclContext;
 class LangOptions;
 class SourceManager;
 class Stmt;
@@ -39,6 +40,15 @@
   virtual std::string remapPath(StringRef Path) const {
 return std::string(Path);
   }
+
+  /// When printing type to be inserted into code in specific context, this
+  /// callback can be used to avoid printing the redundant part of the
+  /// qualifier. For example, when inserting code inside namespace foo, we
+  /// should print bar::SomeType instead of foo::bar::SomeType.
+  /// To do this, shouldPrintScope should return true on "foo" NamespaceDecl.
+  /// The printing stops at the first isScopeVisible() == true, so there will
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *DC) const { return false; }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
+  // Check const qualifier with namespace
+  std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+  [[return y.z + y.z;]]
+})cpp";
+  std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+  return extracted(y);
+})cpp";
+  EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@
   EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
 R"cpp(const char * x = "test";)cpp");
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+  EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+"void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; }");
+
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
   // expanding types in structured bindings is syntactically invalid.
   EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@
   return SymbolID(USR);
 }
 
-// FIXME: This should be handled while printing underlying decls instead.
 std::string printType(const QualType QT, const DeclContext &CurContext) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  auto Decls =
-  explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
-  if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
-   Decls.front(),
-   /*VisibleNamespaces=*/llvm::ArrayRef{});
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
-  PP.SuppressScope = true;
   PP.SuppressTagKeyword = true;
+  PP.SuppressUnwrittenScope = true;
+
+  class PrintCB : public PrintingCallbacks {
+  public:
+PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
+virtual ~PrintCB() {}
+virtual bool

[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

Does it make sense to return `true` when there are no inner matchers? I would 
have thought that that if there are no matchers, nothing would match (so we'd 
return `false`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94126

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


[PATCH] D94259: [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4af86581e80: [clangd] Fix type printing in the presence of 
qualifiers (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94259

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1225,6 +1225,9 @@
   if (DC->isFunctionOrMethod())
 return;
 
+  if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
+return;
+
   if (const auto *NS = dyn_cast(DC)) {
 if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
   return AppendScope(DC->getParent(), OS, NameInScope);
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -18,6 +18,7 @@
 
 namespace clang {
 
+class DeclContext;
 class LangOptions;
 class SourceManager;
 class Stmt;
@@ -39,6 +40,15 @@
   virtual std::string remapPath(StringRef Path) const {
 return std::string(Path);
   }
+
+  /// When printing type to be inserted into code in specific context, this
+  /// callback can be used to avoid printing the redundant part of the
+  /// qualifier. For example, when inserting code inside namespace foo, we
+  /// should print bar::SomeType instead of foo::bar::SomeType.
+  /// To do this, shouldPrintScope should return true on "foo" NamespaceDecl.
+  /// The printing stops at the first isScopeVisible() == true, so there will
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *DC) const { return false; }
 };
 
 /// Describes how types, statements, expressions, and declarations should be
Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
+  // Check const qualifier with namespace
+  std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+  [[return y.z + y.z;]]
+})cpp";
+  std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+  return extracted(y);
+})cpp";
+  EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@
   EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
 R"cpp(const char * x = "test";)cpp");
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+  EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+"void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; }");
+
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
   // expanding types in structured bindings is syntactically invalid.
   EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@
   return SymbolID(USR);
 }
 
-// FIXME: This should be handled while printing underlying decls instead.
 std::string printType(const QualType QT, const DeclContext &CurContext) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  auto Decls =
-  explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
-  if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
-   Decls.front(),
-   /*VisibleNamespaces=*/llvm::ArrayRef{});
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
-  PP.SuppressScope = true;
   PP.SuppressTagKeyword = true;
+  PP.SuppressUnwrittenScope = true;
+
+  class 

[clang] d4af865 - [clangd] Fix type printing in the presence of qualifiers

2021-01-08 Thread Adam Czachorowski via cfe-commits

Author: Adam Czachorowski
Date: 2021-01-08T17:00:39+01:00
New Revision: d4af86581e80ef0f7a6f4a4fff1c97260a726e71

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

LOG: [clangd] Fix type printing in the presence of qualifiers

When printing QualType with qualifiers like "const", or pointing to an
elaborated type, we would print garbage like:
  std::const std::vector&
with the initial std:: being calculated correctly, but inserted in the
wrong place and the second std:: not removed (due to elaborated type).

This affected, among others, ExtractFunction and ExpandAuto tweaks.

This change introduces a new callback to PrintingPolicy, which allows us
to influence the printing of namespace qualifiers. In the future, the
same callback can be used to improve handling of "using namespace"
directives as well.

Fixes:
  https://github.com/clangd/clangd/issues/640 (ExtractFunction)
  https://github.com/clangd/clangd/issues/264 (ExpandAuto)
  First point of https://github.com/clangd/clangd/issues/524

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 39ab48843b28..16298f3e0326 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, 
const MacroInfo *MI,
   return SymbolID(USR);
 }
 
-// FIXME: This should be handled while printing underlying decls instead.
 std::string printType(const QualType QT, const DeclContext &CurContext) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  auto Decls =
-  explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
-  if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
-   Decls.front(),
-   
/*VisibleNamespaces=*/llvm::ArrayRef{});
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
-  PP.SuppressScope = true;
   PP.SuppressTagKeyword = true;
+  PP.SuppressUnwrittenScope = true;
+
+  class PrintCB : public PrintingCallbacks {
+  public:
+PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
+virtual ~PrintCB() {}
+virtual bool isScopeVisible(const DeclContext *DC) const {
+  return DC->Encloses(CurContext);
+}
+
+  private:
+const DeclContext *CurContext;
+  };
+  PrintCB PCB(&CurContext);
+  PP.Callbacks = &PCB;
+
   QT.print(OS, PP);
   return OS.str();
 }

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
index ba783e551e84..6c96ecc2332f 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@ TEST_F(ExpandAutoTypeTest, Test) {
   EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
 R"cpp(const char * x = "test";)cpp");
 
+  EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+  EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+"void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; 
}");
+
   EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
   // expanding types in structured bindings is syntactically invalid.
   EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");

diff  --git 
a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 2033b479896b..94cc4b0a0d84 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@ void f(const int c) {
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
+  // Check const qualifier with namespace
+  std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+  [[return y.z + y.z;]]
+})cpp";
+  std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+  return extracted(y);
+})cpp";
+  EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() {

[PATCH] D93095: Introduce -Wreserved-identifier

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Decl.cpp:1099
+const DeclContext *CurrentContext = getDeclContext();
+while (true) {
+  if (isa(CurrentContext))

Rather than trying to manually decide whether the name will be exposed at the 
top level, I wonder if we should use `Sema::LookupName()` to try to find the 
name in the global scope. That would move this code from `AST` into `Sema` to 
avoid layering issues, so it'd be something more like `bool 
Sema::IsDeclReserved(const NamedDecl *ND) const` or `bool 
Sema::IsIdentifierReserved(const IdentifierInfo *II) const`.

The idea is to perform the lookup in global scope and if the lookup finds the 
identifier, then regardless of how it got there (anonymous union field, 
anonymous namespaces, enumeration constants, etc), we know it may conflict with 
an external declaration and diagnose.

WDYT?



Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}

hubert.reinterpretcast wrote:
> serge-sans-paille wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > You should also have some tests for:
> > > > ```
> > > > template 
> > > > void _Foobar(); // Even though it's not instantiated, it's still 
> > > > reserved.
> > > > 
> > > > template  // Reserved
> > > > void whatever();
> > > > 
> > > > void func() {
> > > >   int array[10];
> > > >   for (auto _A : array) // Reserved
> > > > ;
> > > > }
> > > > 
> > > > class _C { // Reserved
> > > > public:
> > > >   _C(); // Not reserved
> > > > };
> > > > 
> > > > unsigned operator "" huttah(unsigned long long); // Reserved 
> > > > (http://eel.is/c++draft/usrlit.suffix#1)
> > > > 
> > > > unsigned operator "" _W(unsigned long long); // Reserved
> > > > unsigned operator "" _w(unsigned long long); // Reserved
> > > > 
> > > > static unsigned operator "" _X(unsigned long long); // Not reserved
> > > > static unsigned operator "" _x(unsigned long long); // Not reserved
> > > > ```
> > > I think some of these tests are still missing. I'm especially worried 
> > > about the user-defined literal cases being diagnosed, as we'd be warning 
> > > to not do the exact thing users are expected to do.
> > User defined literal tested below and behave as expected.
> @aaron.ballman, the "not reserved" comment re: `_X` for the literal operator 
> using the `operator string-literal identifier` form above seems suspect to 
> me. See `_Bq` example in [over.literal].
Thanks, @hubert.reinterpretcast, you're correct -- I forgot just how hard we 
made the rules for UDLs when we decided to require a leading underscore. :-(


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

https://reviews.llvm.org/D93095

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


[PATCH] D94314: [clangd] Add missing "override" to fix the build.

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Follow-up to d4af86581e80ef0f7a6f4a4fff1c97260a726e71 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94314

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


Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 


Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

2021-01-08 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

Ping, @aaron.ballman?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

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


[clang-tools-extra] 2e1bb79 - [clangd] Add missing "override" to fix the build.

2021-01-08 Thread Adam Czachorowski via cfe-commits

Author: Adam Czachorowski
Date: 2021-01-08T17:24:47+01:00
New Revision: 2e1bb7940a4ddc847cebd25092d10f40866a7fad

URL: 
https://github.com/llvm/llvm-project/commit/2e1bb7940a4ddc847cebd25092d10f40866a7fad
DIFF: 
https://github.com/llvm/llvm-project/commit/2e1bb7940a4ddc847cebd25092d10f40866a7fad.diff

LOG: [clangd] Add missing "override" to fix the build.

Follow-up to d4af86581e80ef0f7a6f4a4fff1c97260a726e71

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 16298f3e0326..8af4cbb19a3d 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@ std::string printType(const QualType QT, const DeclContext 
&CurContext) {
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 



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


[PATCH] D94314: [clangd] Add missing "override" to fix the build.

2021-01-08 Thread Adam Czachorowski via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e1bb7940a4d: [clangd] Add missing "override" to 
fix the build. (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94314

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


Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 


Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D69560#2486806 , @whisperity wrote:

> I was just about to write an issue to the CoreGuidelines project, but I was 
> @vingeldal has posted about "relatedness" before me: 
> https://github.com/isocpp/CppCoreGuidelines/issues/1579. It was in the list 
> of //closed// issues.
>
> Herb Sutter has changed the rule's title to //"Avoid adjacent parameters of 
> the same type when changing the argument order would change meaning"//. Now 
> the phrase `related` has been removed from the title, and the body of the 
> rule does not contain any mention to this phrase either.
>
> //`change of meaning`// is still hard to prove from a Tidy standpoint (or 
> could be incomputable to prove in the first place...), but a concept that can 
> be grasped with much less mental effort.
>
> The guideline itself still says that the //Enforcement// is to simply warn if 
> two parameters have the same type. We are doing much more here already with 
> making the Tidy output much more user friendly and less bloatful.

Thank you for continuing to work on this and working with the rule authors. I 
agree that "changes meaning" would be really hard to gauge within clang-tidy 
(but is more precise for a human auditing the code). In theory, we could take 
the command line options and use them to compile a function to LLVM IR, swap 
two adjacent parameter identifiers and recompile, then see if the IR is the 
same or not. But my gut feeling is that this would make the diagnostic behavior 
very mysterious to users (you change command line flags from -O2 to -O0 and 
suddenly you get different diagnostics because the optimizations are different) 
and is not something we'd want to do.

> In addition to all this, and tracking back to the talk about default 
> configuration... @aaron.ballman, I have some questions. I've seen that Tidy 
> now supports "check aliases". Do you think moving forward in a way that we 
> rename this check > and put it into another group (maybe calling it 
> bugprone-adjacent-parameters-of-same-type) with more sensible defaults and 
> offering a version of the check under cppcoreguidelines- with the defaults 
> more matching the guideline rule > would be a good way forward, eventually? 
> Implicit conversions seem to be a real edge of the analysis (hence why I 
> pinned my paper about that problem, also, this was the novelty, the rest of 
> the mixability analysis was done decades
> ago), and seem to hurt more than simple type equality. However, the guideline 
> is cautious about only warning for same type, and considers const T* and T* 
> already distinct.

Sorry, I missed your question from earlier. I think using a check alias with 
different option defaults is a workable approach. We may have to live with the 
fact that C++ Core Guideline check is really chatty and only useful for 
programs in very specific situations (and that's fine, we do that for other 
check guidelines sometimes). Having a check with better default options for our 
users is a good compromise.


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

https://reviews.llvm.org/D69560

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


[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2021-01-08 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

In D86694#2463429 , @russell.gallop 
wrote:

> - With scudo all lit tests (generally) pass, but there are some intermittent 
> failures (Access Violation) on the following tests:
>
>> ExecutionEngine/MCJIT/test-global-init-nonzero-sm-pic.ll
>> ExecutionEngine/MCJIT/test-ptr-reloc-sm-pic.ll
>> ExecutionEngine/MCJIT/multi-module-sm-pic-a.ll
>
> This happens with VS2019 or clang-cl as the toolchain. I've run multiple 
> times without scudo and don't see these. Unfortunately I haven't been able to 
> get this to fail in a debugger.

Could you possibly comment out `sys::PrintStackTraceOnErrorSignal` in 
`InitLLVM.cpp`, run specifically those tests several times through LIT until 
they crash, and attach when the popup shows up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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


[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

2021-01-08 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, ABataev, sstefan1.
Herald added subscribers: guansong, bollu, yaxunl.
jdoerfert requested review of this revision.
Herald added a project: clang.

Whenever we enter a new OpenMP data environment we want to enter a
function to simplify reasoning. Later we probably want to remove the
entire specialization wrt. the if clause and pass the result to the
runtime, for now this should fix PR48686.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94315

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/parallel_if_codegen.cpp

Index: clang/test/OpenMP/parallel_if_codegen.cpp
===
--- clang/test/OpenMP/parallel_if_codegen.cpp
+++ clang/test/OpenMP/parallel_if_codegen.cpp
@@ -7,6 +7,7 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple %itanium_abi_triple -emit-llvm %s -disable-O0-optnone -o - | FileCheck %s --check-prefix=WOPT
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
@@ -96,14 +97,20 @@
   return tmain(Arg);
 }
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN4]]
 // CHECK: call {{.*}}void @{{.+}}fn4
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN5]]
 // CHECK: call {{.*}}void @{{.+}}fn5
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN6]]
 // CHECK: call {{.*}}void @{{.+}}fn6
 // CHECK: ret void
@@ -129,14 +136,20 @@
 // CHECK: br label %[[OMP_END]]
 // CHECK: [[OMP_END]]
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN1]]
 // CHECK: call {{.*}}void @{{.+}}fn1
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN2]]
 // CHECK: call {{.*}}void @{{.+}}fn2
 // CHECK: ret void
 
+// WOPT: noinline
+// WOPT-NOT: optnone
 // CHECK: define internal {{.*}}void [[CAP_FN3]]
 // CHECK: call {{.*}}void @{{.+}}fn3
 // CHECK: ret void
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -1576,11 +1576,6 @@
   auto *OutlinedFun =
   cast(CGOpenMPRuntime::emitParallelOutlinedFunction(
   D, ThreadIDVar, InnermostKind, CodeGen));
-  if (CGM.getLangOpts().Optimize) {
-OutlinedFun->removeFnAttr(llvm::Attribute::NoInline);
-OutlinedFun->removeFnAttr(llvm::Attribute::OptimizeNone);
-OutlinedFun->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
   IsInTargetMasterThreadRegion = PrevIsInTargetMasterThreadRegion;
   IsInTTDRegion = PrevIsInTTDRegion;
   if (getExecutionMode() != CGOpenMPRuntimeGPU::EM_SPMD &&
@@ -1698,11 +1693,6 @@
   CodeGen.setAction(Action);
   llvm::Function *OutlinedFun = CGOpenMPRuntime::emitTeamsOutlinedFunction(
   D, ThreadIDVar, InnermostKind, CodeGen);
-  if (CGM.getLangOpts().Optimize) {
-OutlinedFun->removeFnAttr(llvm::Attribute::NoInline);
-OutlinedFun->removeFnAttr(llvm::Attribute::OptimizeNone);
-OutlinedFun->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
 
   return OutlinedFun;
 }
@@ -2102,6 +2092,14 @@
   // Force inline this outlined function at its call site.
   Fn->setLinkage(llvm::GlobalValue::InternalLinkage);
 
+  // Ensure we do not inline the function. This is trivially true for the ones
+  // passed to __kmpc_fork_call but the ones calles in serialized regions
+  // could be inlined. This is not a perfect but it is closer to the invariant
+  // we want, namely, every data environment starts with a new function.
+  // TODO: We should pass the if condition to the runtime function and do the
+  //   handling there. Much cleaner code.
+  cast(OutlinedFn)->addFnAttr(llvm::Attribute::NoInline);
+
   Address ZeroAddr = CGF.CreateDefaultAlignTempAlloca(CGF.Int32Ty,
   /*Name=*/".zero.addr");
   CGF.InitTempAlloca(ZeroAddr, CGF.Builder.getInt32(/*C*/ 0));
@@ -3134,11 +3132,6 @@
   "_omp_reduction_shuffle_and_reduce_func", &CGM.getModule());
   CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, CGFI);
   Fn->setDoesNotRecurse();
-  if (CGM.getLangOpts().Optimize) {
-Fn->removeFnAttr(llvm::Attribute::NoInline);
-Fn->removeFnAttr(llvm::Attribute::Op

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

2021-01-08 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

I have posted two questions to GitHub, mostly related to the guideline rule and 
how free the implementation could be: #1732 
 and #1733 
, I think I tagged you 
on both.

However, if we are going down the alias route, I think it should be the 
guideline version which is the alias, and this check should live under a 
different name. Where do you think should we put it? I think `bugprone-` is the 
best fit, and `readability-` is the second-best... The "avoid adjacent 
parameter of similar types" name could stay... or something like 
`...-easily-swappable-function-parameters`, that involves all the features 
here, with things like "default length 3" and such, "relatedness" heuristics 
turned on. Not sure what CVR-modelling's default should be... it finds less 
when "off", but too easily silences crucial issues (such as `memcpy(T*, const 
T*)`).


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

https://reviews.llvm.org/D69560

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


[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7783-7784
   "|%diff{casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with different sign">,
+  " converts between pointers to integer types that differ by"
+  " signed/unsigned/plain variation">,
   InGroup>;

I fee like `that differ by signed/unsigned/plain variation` is a bit hard for 
users to understand and maybe we want to spell it out a bit more explicitly. I 
took a stab at a more wordy diagnostic that I think is easier to understand, 
but if you have other ideas, I'm not tied to my wording. WDYT?

(Same suggestion applies below -- though we may want to switch to 
`ext_typecheck_convert_incompatible_pointer_sign.Text` below rather than 
spelling all this out manually.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

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


[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

2021-01-08 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

Thank you all for your input. Let me summarize.

**SUGGESTION 1**
Tweak the wording to clarify that it is Clang-specific. That's a good 
compromise IMO. It highlights that the described semantics apply to Clang only. 
And I'm not aware of any other way to achieve this right now (i.e. without 
intrusive refactoring).

**SUGGESTION 2:**
The description of `-I` in `-help` and `ClangCommandLineReference.rst` are made 
independent of each other (so that `-help` can be tweaked to suit both Clang 
and Flang).

`ClangCommandLineReference.rst` is basically generated from `Options.td`, so 
any modification to Options.td will affect the user-facing docs.

From docs/CMakeLists.txt:

  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)

From ClangOptionDocs.td:

  include "Options.td"

I don't see any straightforward solution here that would allow different 
descriptions for `-I` in `-help` and in the user-facing docs. Not without 
completely refactoring how ClangCommandLineReference.rst is produced.

**SUGGESTION 3**
Ideally, we'd have one help message for `-I` for Clang and another one for 
Flang (in `Options.td`). Unfortunately, AFAIK, there is no easy way to tweak 
the output of `-help`. The current API is quite inflexible in this respect. 
This was brought-up at some point last year [1][2]. From the limited feedback 
I'm guessing that that would also require a major refactor.

**SUGGESTION 4**
We add a new definition for `-I` for Flang, e.g. `def I_for_flang`.

This solves the problem, but adds yet another option definition. Note that 
`libclangDriver` creates only one global list of all options:

- 
https://github.com/llvm/llvm-project/blob/80dee7965dffdfb866afa9d74f3a4a97453708b2/clang/lib/Driver/DriverOptions.cpp#L42-L55
- 
https://github.com/llvm/llvm-project/blob/80dee7965dffdfb866afa9d74f3a4a97453708b2/clang/include/clang/Driver/Options.h#L43-L52

This means that both Flang and Clang will have access to `OPT_I` and 
`OPT_I_for_flang` (C++ options generated from Options.td). These can be hidden 
from the user, but will still be available. Also, this would mean the massive 
table of `libclangDriver` options keeps growing.

Is that how we should proceed moving forward when option names are identical 
for Clang and Flang, but the semantics are slightly different?

[1] https://lists.llvm.org/pipermail/llvm-dev/2020-July/143745.html
[2] https://lists.llvm.org/pipermail/cfe-dev/2020-October/066953.html

**Few final points**
I would really appreciate a compromise that does not require any major 
refactoring in Clang at this stage. 
We are ready to add `-I`to Flang and we have a bunch of patches almost ready 
that depend on it. 
I appreciate that this is just one option, but we are likely to experience more 
inconsistencies like this in the near future.

Also, for reference, output from  `gcc` and `gfortran`:

  -I Add  to the end of the main include path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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


[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

2021-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D69560#2487093 , @whisperity wrote:

> I have posted two questions to GitHub, mostly related to the guideline rule 
> and how free the implementation could be: #1732 
>  and #1733 
> , I think I tagged 
> you on both.
>
> However, if we are going down the alias route, I think it should be the 
> guideline version which is the alias, and this check should live under a 
> different name. Where do you think should we put it? I think `bugprone-` is 
> the best fit, and `readability-` is the second-best...

I think `bugprone` is where it should ideally live, but if we still think we 
have "too high" of a false positive rate (for however we decide we want to 
measure that), then `readability` is a good fallback. (I'm assuming more people 
enable `bugprone-*` than enable `readability-*`.)

> The "avoid adjacent parameter of similar types" name could stay... or 
> something like `...-easily-swappable-function-parameters`, that involves all 
> the features here, with things like "default length 3" and such, 
> "relatedness" heuristics turned on.

Hmm, how about `bugprone-easily-swappable-parameters`? We can drop the 
`function` from it because we probably also care about function-like things 
(like lambdas and blocks) and `parameters` should be sufficiently clear as to 
what's getting checked.

> Not sure what CVR-modelling's default should be... it finds less when "off", 
> but too easily silences crucial issues (such as `memcpy(T*, const T*)`).

My instinct is that if you accidentally swap a qualified parameter you'll get 
some other diagnostic about dropped qualifiers and so perhaps the default 
should be `off`, but perhaps there's something I'm not thinking of there.


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

https://reviews.llvm.org/D69560

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


[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

2021-01-08 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

In D69560#2487117 , @aaron.ballman 
wrote:

> In D69560#2487093 , @whisperity 
> wrote:
>
>> Not sure what CVR-modelling's default should be... it finds less when "off", 
>> but too easily silences crucial issues (such as `memcpy(T*, const T*)`).
>
> My instinct is that if you accidentally swap a qualified parameter you'll get 
> some other diagnostic about dropped qualifiers and so perhaps the default 
> should be `off`, but perhaps there's something I'm not thinking of there.

That only happens if at a call site the //argument// is qualified.

  void foo(const int& ir1, int& ir2);
  
  void test() {
int i1 = 8;
int i2 = 10;
  
foo(i2, i1); // No warning here.
  
const int ci = 12;
foo(i1, ci); // Error here.
  }

So while `const Something` and `Something` are distinct types conceptually (and 
as of now, according to I.24), an unqualified `Something` can still be passed 
in a swapped order at any given call site.
Will think about this.

I'll continue implementing some of the heuristics, re-run the measurements, and 
then I'll update the patch with the rename.


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

https://reviews.llvm.org/D69560

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


[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

2021-01-08 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

I'm guessing we're using the function boundary as a compiler barrier. That 
seems fragile in the face of improving cross-function optimisation.

What is the invariant we want around entry to a data environment, which happens 
to be met by a function boundary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315

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


[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

2021-01-08 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D94315#2487150 , @JonChesterfield 
wrote:

> I'm guessing we're using the function boundary as a compiler barrier. That 
> seems fragile in the face of improving cross-function optimisation.

Looks like applying inaccessiblemem_or_argmemonly attribute to the OpenMP 
functions is way too optimistic since we still can access this (inaccessible) 
memory using other OpenMP functions. Not sure about the semantics of this 
attribute, though.

> What is the invariant we want around entry to a data environment, which 
> happens to be met by a function boundary?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315

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


[PATCH] D78105: [CSInfo][ISEL] Call site info generation support for Mips

2021-01-08 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

Compiler crash reported in: https://bugs.llvm.org/show_bug.cgi?id=48695


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78105

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


[PATCH] D94201: [clang-format] Skip UTF8 Byte Order Mark while sorting includes

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek updated this revision to Diff 315424.
rjelonek added a comment.

rebase patch on master/main


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

https://reviews.llvm.org/D94201

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,42 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkMerge) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "\xEF\xBB\xBF#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"d.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 1));
+}
+
+TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkPreserve) {
+  Style.IncludeBlocks = Style.IBS_Preserve;
+  std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "\xEF\xBB\xBF#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2253,7 +2253,9 @@
   StringRef FileName,
   tooling::Replacements &Replaces,
   unsigned *Cursor) {
-  unsigned Prev = 0;
+  unsigned Prev = llvm::StringSwitch(Code)
+  .StartsWith("\xEF\xBB\xBF", 3) // UTF-8 BOM
+  .Default(0);
   unsigned SearchFrom = 0;
   llvm::Regex IncludeRegex(CppIncludeRegexPattern);
   SmallVector Matches;


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,42 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkMerge) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "\xEF\xBB\xBF#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"d.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 1));
+}
+
+TEST_F(SortIncludesTest, skipUTF8ByteOrderMarkPreserve) {
+  Style.IncludeBlocks = Style.IBS_Preserve;
+  std::string Code = "\xEF\xBB\xBF#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "\xEF\xBB\xBF#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2253,7 +2253,9 @@
   StringRef FileName,
   tooling::Replacements &Replaces,
   unsigned *Cursor) {
-  unsigned Prev = 0;
+  unsigned Prev = llvm::StringSwitch(Code)
+  .StartsWith("\xEF\xBB\xBF", 3) // UTF-8 BOM
+  .Default(0);
   uns

[PATCH] D94201: [clang-format] Skip UTF8 Byte Order Mark while sorting includes

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek added a comment.

@curdeius Can you commit this patch?


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

https://reviews.llvm.org/D94201

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


[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek updated this revision to Diff 315428.
rjelonek added a comment.

rebase patch on master/main


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

https://reviews.llvm.org/D94206

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -207,6 +207,27 @@
  "#include \n"
  "#include \n"
  "// clang-format on\n"));
+
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "// clang-format off\r\n"
+ "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "// clang-format on\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "// clang-format off\r\n"
+ "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "// clang-format on\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 1));
 }
 
 TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
@@ -879,6 +900,21 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, MergeLines) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"c.h\"\r\n"
+ "#include \"b\\\r\n"
+ ".h\"\r\n"
+ "#include \"a.h\"\r\n";
+
+  std::string Expected = "#include \"a.h\"\r\n"
+ "#include \"b\\\r\n"
+ ".h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "a.cpp", 1));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2289,7 +2289,8 @@
  Style.IncludeStyle.IncludeBlocks ==
  tooling::IncludeStyle::IBS_Regroup);
 
-if (!FormattingOff && !Line.endswith("\\")) {
+bool MergeWithNextLine = Trimmed.endswith("\\");
+if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
 StringRef IncludeName = Matches[2];
 int Category = Categories.getIncludePriority(
@@ -2307,10 +2308,12 @@
 IncludesInBlock.clear();
 FirstIncludeBlock = false;
   }
-  Prev = Pos + 1;
 }
 if (Pos == StringRef::npos || Pos + 1 == Code.size())
   break;
+
+if (!MergeWithNextLine)
+  Prev = Pos + 1;
 SearchFrom = Pos + 1;
   }
   if (!IncludesInBlock.empty()) {


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -207,6 +207,27 @@
  "#include \n"
  "#include \n"
  "// clang-format on\n"));
+
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "// clang-format off\r\n"
+ "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "// clang-format on\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "// clang-format off\r\n"
+ "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "// clang-format on\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 1));
 }
 
 TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
@@ -879,6 +900,21 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, MergeLines) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"c.h\"\r\n"
+ "#include \"b\\\r\n"
+ ".h\"\r\n"
+ "#include \"a.h\"\r\n";
+
+  std::string Expected = "#include \"a.h\"\r\n"
+ "#include \"b\\\r\n"
+ ".h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "a.cpp", 1));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Rafał Jelonek via Phabricator via cfe-commits
rjelonek updated this revision to Diff 315431.
rjelonek added a comment.

rebase patch on master/main


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

https://reviews.llvm.org/D94217

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -879,6 +879,45 @@
  "#include \"a.h\""));
 }
 
+TEST_F(SortIncludesTest, DoNotTreatPrecompiledHeadersAsFirstBlock) {
+  Style.IncludeBlocks = Style.IBS_Merge;
+  std::string Code = "#include \"d.h\"\r\n"
+ "#include \"b.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"c.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"e.h\"\r\n";
+
+  std::string Expected = "#include \"b.h\"\r\n"
+ "#include \"d.h\"\r\n"
+ "#pragma hdrstop\r\n"
+ "\r\n"
+ "#include \"e.h\"\r\n"
+ "#include \"a.h\"\r\n"
+ "#include \"c.h\"\r\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+
+  Code = "#include \"d.h\"\n"
+ "#include \"b.h\"\n"
+ "#pragma hdrstop( \"c:\\projects\\include\\myinc.pch\" )\n"
+ "\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"e.h\"\n";
+
+  Expected = "#include \"b.h\"\n"
+ "#include \"d.h\"\n"
+ "#pragma hdrstop(\"c:\\projects\\include\\myinc.pch\")\n"
+ "\n"
+ "#include \"e.h\"\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n";
+
+  EXPECT_EQ(Expected, sort(Code, "e.cpp", 2));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2305,7 +2305,10 @@
 sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
 Replaces, Cursor);
 IncludesInBlock.clear();
-FirstIncludeBlock = false;
+if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers
+  FirstIncludeBlock = true;
+else
+  FirstIncludeBlock = false;
   }
   Prev = Pos + 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-

[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

2021-01-08 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D94315#2487164 , @ABataev wrote:

> In D94315#2487150 , @JonChesterfield 
> wrote:
>
>> I'm guessing we're using the function boundary as a compiler barrier. That 
>> seems fragile in the face of improving cross-function optimisation.
>
> Looks like applying inaccessiblemem_or_argmemonly attribute to the OpenMP 
> functions is way too optimistic since we still can access this (inaccessible) 
> memory using other OpenMP functions. Not sure about the semantics of this 
> attribute, though.

I don't understand where `inaccessiblemem_or_argmemonly` is coming from. This 
prevents us from inlining the outlined parallel region.

>> What is the invariant we want around entry to a data environment, which 
>> happens to be met by a function boundary?

The data environment in one function is the same. So, changes that take affect 
only in a new data environment will not manifest in the function.
Take:

  a = get_some_ICV_fixed_per_data_env()
  unknown();
  b = get_some_ICV_fixed_per_data_env();

We want to ensure `a == b`.

---

The problem with PR48686 could actually be solved differently as well. However, 
the property I describe above is generally useful.
It holds, as far as I can tell, except when we do the `if(0)` "optimization". 
IMHO, the if condition should be passed to the runtime
and handled there. There is little to no point in exposing the conditional in 
user land and exposing even more runtime calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315

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


[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

2021-01-08 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D94315#2487242 , @jdoerfert wrote:

> In D94315#2487164 , @ABataev wrote:
>
>> In D94315#2487150 , 
>> @JonChesterfield wrote:
>>
>>> I'm guessing we're using the function boundary as a compiler barrier. That 
>>> seems fragile in the face of improving cross-function optimisation.
>>
>> Looks like applying inaccessiblemem_or_argmemonly attribute to the OpenMP 
>> functions is way too optimistic since we still can access this 
>> (inaccessible) memory using other OpenMP functions. Not sure about the 
>> semantics of this attribute, though.
>
> I don't understand where `inaccessiblemem_or_argmemonly` is coming from. This 
> prevents us from inlining the outlined parallel region.

__kmpc_serialized_parallel and __kmpc_end_serialized_parallel functions are 
marked with this attribute and I think OpenMPOpt optimizes read_only functions 
because of this. I.e. it does not consider __kmpc_serialized_parallel and 
__kmpc_end_serialized_paralle function calls as opimization barriers.

>>> What is the invariant we want around entry to a data environment, which 
>>> happens to be met by a function boundary?
>
> The data environment in one function is the same. So, changes that take 
> affect only in a new data environment will not manifest in the function.
> Take:
>
>   a = get_some_ICV_fixed_per_data_env()
>   unknown();
>   b = get_some_ICV_fixed_per_data_env();
>
> We want to ensure `a == b`.
>
> ---
>
> The problem with PR48686 could actually be solved differently as well. 
> However, the property I describe above is generally useful.
> It holds, as far as I can tell, except when we do the `if(0)` "optimization". 
> IMHO, the if condition should be passed to the runtime
> and handled there. There is little to no point in exposing the conditional in 
> user land and exposing even more runtime calls.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315

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


  1   2   >