[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Found today:

  if (something)
return; // some long comment
// that takes two lines.^

Expected: comment on last line is not re-indented.
Actual (comment re-indented):

  if (something)
return; // some long comment
  // that takes two lines.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

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

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197267.
hintonda added a comment.
Herald added subscribers: cfe-commits, thopre.
Herald added a project: clang.

- Fix dashes in error messages and in tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269

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

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

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

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

Szelethus reopened this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
Herald added a project: clang.

Reverted in rCTE348344 .


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54757



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

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

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

Szelethus commandeered this revision.
Szelethus added a reviewer: donat.nagy.
Szelethus added a comment.

I'll take a look at this.

In D54757#1319831 , @JonasToth wrote:

> I had to revert this patch because it broke (at least one) buildbot with an 
> assertion-failure 
> (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/40436/steps/test/logs/stdio)
>
> Could you please take a look at it? I could not reproduce locally though.


The logs are gone -- do you mind me commiting this again to retrieve the 
assertation?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54757



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

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

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197271.
hintonda added a comment.
Herald added subscribers: cfe-commits, thopre.
Herald added a project: clang.

- Fix test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270

Files:
  clang/test/Driver/clang-offload-bundler.c
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Support/check-default-options.txt
  llvm/test/tools/llvm-readobj/merged.test

Index: llvm/test/tools/llvm-readobj/merged.test
===
--- llvm/test/tools/llvm-readobj/merged.test
+++ llvm/test/tools/llvm-readobj/merged.test
@@ -10,4 +10,4 @@
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the --section-headers option: may only occur zero or one times!
Index: llvm/test/Support/check-default-options.txt
===
--- llvm/test/Support/check-default-options.txt
+++ llvm/test/Support/check-default-options.txt
@@ -1,18 +1,18 @@
-# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
-# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
-# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
-# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
-# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-objdump --help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj --help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen --help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report --help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump --help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
 # RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
 
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for --help
+# CHECK-OPT-RPT: -h  - Alias for --help
 # CHECK-DWARF:   -h  - Alias for -help
 
 # llvm-dwarfdump declares `-h` option and prints special help in that case,
 # which is weird, but makes for a good test, i.e., shows the default `-h`
 # wasn't used.
-# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
+# CHECK-DWARF-H-NOT: --help-list  - Display list of available options (--help-list-hidden for more)
Index: llvm/test/FileCheck/dump-input-enable.txt
===
--- llvm/test/FileCheck/dump-input-enable.txt
+++ llvm/test/FileCheck/dump-input-enable.txt
@@ -26,7 +26,7 @@
 ; RUN: not FileCheck -dump-input=foobar 2>&1 \
 ; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
 
-BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the -dump-input option: Cannot find option named 'foobar'!
+BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the --dump-input option: Cannot find option named 'foobar'!
 
 ;--
 ; Check -dump-input=help.
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -88,6 +88,22 @@
 
 //===--===//
 
+static StringRef ArgPrefix = "  -";
+static StringRef ArgPrefixLong = "  --";
+static StringRef ArgHelpPrefix = " - ";
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)
+return ArgPrefix.size() + ArgHelpPrefix.size();
+  return ArgPrefixLong.size() + ArgHelpPrefix.size();
+}
+
+static StringRef argPrefix(size_t len) {
+  if (len == 1)
+return ArgPrefix;
+  return ArgPrefixLong;
+}
+
 namespace {
 
 class CommandLineParser {
@@ -392,6 +408,8 @@
 GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+setMiscFlag(Grouping);
 }
 
 void Option::reset() {
@@ -1339,12 +1357,13 @@
 if (!Handler) {
   if (SinkOpts.empty()) {
 *Errs << ProgramName << ": Unknown command line argument '" << argv[i]
-  << "'.  Try: '" << argv[0] << " -help'\n";
+  << "'.  Try: '" << argv[0] << " --help'\n";
 
 if (NearestHandler) {
   // If we know a near match, report it as well.
-  *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString
- << "'?\n";
+  *Errs << ProgramName << ": Did you mean '"
+ 

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

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197272.
hintonda added a comment.

Minimize diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270

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


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


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

r359539 - [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Apr 30 01:47:56 2019
New Revision: 359539

URL: http://llvm.org/viewvc/llvm-project?rev=359539&view=rev
Log:
[analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

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

Similarly to D61106, the checker ran over an llvm_unreachable for vector types:

struct VectorSizeLong {
  VectorSizeLong() {}
  __attribute__((__vector_size__(16))) long x;
};

void __vector_size__LongTest() {
  VectorSizeLong v;
}
Since, according to my short research,

"The vector_size attribute is only applicable to integral and float scalars,
although arrays, pointers, and function return values are allowed in conjunction
with this construct."
[src: 
https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Vector-Extensions.html#Vector-Extensions]

vector types are safe to regard as primitive.

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


Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=359539&r1=359538&r2=359539&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
Tue Apr 30 01:47:56 2019
@@ -324,7 +324,8 @@ private:
 inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType() || T->isAtomicType();
+ T->isFunctionType() || T->isAtomicType() ||
+ T->isVectorType();
 }
 
 inline bool isDereferencableType(const QualType &T) {

Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=359539&r1=359538&r2=359539&view=diff
==
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Tue Apr 30 
01:47:56 2019
@@ -256,6 +256,29 @@ void fCharPointerTest() {
   CharPointerTest();
 }
 
+struct VectorSizePointer {
+  VectorSizePointer() {} // expected-warning{{1 uninitialized field}}
+  __attribute__((__vector_size__(8))) int *x; // expected-note{{uninitialized 
pointer 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+};
+
+void __vector_size__PointerTest() {
+  VectorSizePointer v;
+}
+
+struct VectorSizePointee {
+  using MyVectorType = __attribute__((__vector_size__(8))) int;
+  MyVectorType *x;
+
+  VectorSizePointee(decltype(x) x) : x(x) {}
+};
+
+void __vector_size__PointeeTest() {
+  VectorSizePointee::MyVectorType i;
+  // TODO: Report v.x's pointee.
+  VectorSizePointee v(&i);
+}
+
 struct CyclicPointerTest1 {
   int *ptr; // expected-note{{object references itself 'this->ptr'}}
   int dontGetFilteredByNonPedanticMode = 0;

Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp?rev=359539&r1=359538&r2=359539&view=diff
==
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp Tue Apr 30 01:47:56 
2019
@@ -1132,7 +1132,7 @@ void fCXX11MemberInitTest2() {
 }
 
 
//===--===//
-// _Atomic tests.
+// "Esoteric" primitive type tests.
 
//===--===//
 
 struct MyAtomicInt {
@@ -1142,6 +1142,17 @@ struct MyAtomicInt {
   MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
 };
 
-void entry() {
+void _AtomicTest() {
   MyAtomicInt b;
 }
+
+struct VectorSizeLong {
+  VectorSizeLong() {}
+  __attribute__((__vector_size__(16))) long x;
+};
+
+void __vector_size__LongTest() {
+  // TODO: Warn for v.x.
+  VectorSizeLong v;
+  v.x[0] = 0;
+}


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

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359539: [analyzer][UninitializedObjectChecker] PR41611: 
Regard vector types as primitive (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61246?vs=197209&id=197276#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61246

Files:
  
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp


Index: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
===
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
@@ -1132,7 +1132,7 @@
 }
 
 
//===--===//
-// _Atomic tests.
+// "Esoteric" primitive type tests.
 
//===--===//
 
 struct MyAtomicInt {
@@ -1142,6 +1142,17 @@
   MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
 };
 
-void entry() {
+void _AtomicTest() {
   MyAtomicInt b;
 }
+
+struct VectorSizeLong {
+  VectorSizeLong() {}
+  __attribute__((__vector_size__(16))) long x;
+};
+
+void __vector_size__LongTest() {
+  // TODO: Warn for v.x.
+  VectorSizeLong v;
+  v.x[0] = 0;
+}
Index: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -256,6 +256,29 @@
   CharPointerTest();
 }
 
+struct VectorSizePointer {
+  VectorSizePointer() {} // expected-warning{{1 uninitialized field}}
+  __attribute__((__vector_size__(8))) int *x; // expected-note{{uninitialized 
pointer 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+};
+
+void __vector_size__PointerTest() {
+  VectorSizePointer v;
+}
+
+struct VectorSizePointee {
+  using MyVectorType = __attribute__((__vector_size__(8))) int;
+  MyVectorType *x;
+
+  VectorSizePointee(decltype(x) x) : x(x) {}
+};
+
+void __vector_size__PointeeTest() {
+  VectorSizePointee::MyVectorType i;
+  // TODO: Report v.x's pointee.
+  VectorSizePointee v(&i);
+}
+
 struct CyclicPointerTest1 {
   int *ptr; // expected-note{{object references itself 'this->ptr'}}
   int dontGetFilteredByNonPedanticMode = 0;
Index: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -324,7 +324,8 @@
 inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType() || T->isAtomicType();
+ T->isFunctionType() || T->isAtomicType() ||
+ T->isVectorType();
 }
 
 inline bool isDereferencableType(const QualType &T) {


Index: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
===
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
@@ -1132,7 +1132,7 @@
 }
 
 //===--===//
-// _Atomic tests.
+// "Esoteric" primitive type tests.
 //===--===//
 
 struct MyAtomicInt {
@@ -1142,6 +1142,17 @@
   MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
 };
 
-void entry() {
+void _AtomicTest() {
   MyAtomicInt b;
 }
+
+struct VectorSizeLong {
+  VectorSizeLong() {}
+  __attribute__((__vector_size__(16))) long x;
+};
+
+void __vector_size__LongTest() {
+  // TODO: Warn for v.x.
+  VectorSizeLong v;
+  v.x[0] = 0;
+}
Index: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -256,6 +256,29 @@
   CharPointerTest();
 }
 
+struct VectorSizePointer {
+  VectorSizePointer() {} // expected-warning{{1 uninitialized field}}
+  __attribute__((__vector_size__(8))) int *x; // expected-note{{uninitialized pointer 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+};
+
+void __vector_size__PointerTest() {
+  VectorSizePointer v;
+}
+
+struct VectorSizePointee {
+  using MyVectorType = __attribute__((__vector_size__(8))) int;
+  MyVectorType *x;
+
+  Vecto

Re: r359361 - Revert Fix interactions between __builtin_constant_p and constexpr to match current trunk GCC.

[Eric Fiselier via cfe-commits]

Jorge,

Why did you revert this?

/Eric

On Sat, Apr 27, 2019 at 6:01 AM Roman Lebedev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Sat, Apr 27, 2019 at 3:29 AM Jorge Gorbe Moya via cfe-commits
>  wrote:
> >
> > Author: jgorbe
> > Date: Fri Apr 26 17:32:04 2019
> > New Revision: 359361
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=359361&view=rev
> > Log:
> > Revert Fix interactions between __builtin_constant_p and constexpr to
> match current trunk GCC.
> >
> > This reverts r359059 (git commit
> 0b098754b73f3b96d00ecb1c7605760b11c90298)
> It is common to specify the *reason* for the revert, so it is recorded
> in change log.
>
> > Removed:
> > cfe/trunk/test/SemaCXX/builtin-constant-p.cpp
> > Modified:
> > cfe/trunk/lib/AST/ExprConstant.cpp
> > cfe/trunk/lib/Sema/SemaChecking.cpp
> > cfe/trunk/test/SemaCXX/enable_if.cpp
> >
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359361&r1=359360&r2=359361&view=diff
> >
> ==
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Apr 26 17:32:04 2019
> > @@ -7801,33 +7801,19 @@ EvaluateBuiltinClassifyType(const CallEx
> >  }
> >
> >  /// EvaluateBuiltinConstantPForLValue - Determine the result of
> > -/// __builtin_constant_p when applied to the given pointer.
> > +/// __builtin_constant_p when applied to the given lvalue.
> >  ///
> > -/// A pointer is only "constant" if it is null (or a pointer cast to
> integer)
> > -/// or it points to the first character of a string literal.
> > -static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) {
> > -  APValue::LValueBase Base = LV.getLValueBase();
> > -  if (Base.isNull()) {
> > -// A null base is acceptable.
> > -return true;
> > -  } else if (const Expr *E = Base.dyn_cast()) {
> > -if (!isa(E))
> > -  return false;
> > -return LV.getLValueOffset().isZero();
> > -  } else {
> > -// Any other base is not constant enough for GCC.
> > -return false;
> > -  }
> > +/// An lvalue is only "constant" if it is a pointer or reference to the
> first
> > +/// character of a string literal.
> > +template
> > +static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) {
> > +  const Expr *E = LV.getLValueBase().template dyn_cast();
> > +  return E && isa(E) && LV.getLValueOffset().isZero();
> >  }
> >
> >  /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as
> similarly to
> >  /// GCC as we can manage.
> > -static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {
> > -  // Constant-folding is always enabled for the operand of
> __builtin_constant_p
> > -  // (even when the enclosing evaluation context otherwise requires a
> strict
> > -  // language-specific constant expression).
> > -  FoldConstant Fold(Info, true);
> > -
> > +static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
> >QualType ArgType = Arg->getType();
> >
> >// __builtin_constant_p always has one operand. The rules which gcc
> follows
> > @@ -7835,27 +7821,34 @@ static bool EvaluateBuiltinConstantP(Eva
> >//
> >//  - If the operand is of integral, floating, complex or enumeration
> type,
> >//and can be folded to a known value of that type, it returns 1.
> > -  //  - If the operand can be folded to a pointer to the first character
> > -  //of a string literal (or such a pointer cast to an integral type)
> > -  //or to a null pointer or an integer cast to a pointer, it
> returns 1.
> > +  //  - If the operand and can be folded to a pointer to the first
> character
> > +  //of a string literal (or such a pointer cast to an integral
> type), it
> > +  //returns 1.
> >//
> >// Otherwise, it returns 0.
> >//
> >// FIXME: GCC also intends to return 1 for literals of aggregate
> types, but
> >// its support for this does not currently work.
> > -  if (ArgType->isIntegralOrEnumerationType() ||
> ArgType->isFloatingType() ||
> > -  ArgType->isAnyComplexType() || ArgType->isPointerType() ||
> > -  ArgType->isNullPtrType()) {
> > -APValue V;
> > -if (!::EvaluateAsRValue(Info, Arg, V))
> > +  if (ArgType->isIntegralOrEnumerationType()) {
> > +Expr::EvalResult Result;
> > +if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
> >return false;
> >
> > -// For a pointer (possibly cast to integer), there are special
> rules.
> > +APValue &V = Result.Val;
> > +if (V.getKind() == APValue::Int)
> > +  return true;
> >  if (V.getKind() == APValue::LValue)
> >return EvaluateBuiltinConstantPForLValue(V);
> > -
> > -// Otherwise, any constant value is good enough.
> > -return V.getKind() != APValue::Uninitialized;
> > +  } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
> > +return Arg->isEvaluata

[PATCH] D61297: [clang-format] Fix bug that misses some function-like macro usages

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: klimek, djasper, sammccall, krasimir, MyDeveloperDay.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes PR41483 .


Repository:
  rC Clang

https://reviews.llvm.org/D61297

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith : up :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59413



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

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

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:95
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)

Any reason why not take a StringRef of the option to compute the prefix of? 
That would put the .size() gymnastic in one place instead of many.



Comment at: llvm/lib/Support/CommandLine.cpp:101
+
+static StringRef argPrefix(size_t len) {
+  if (len == 1)

Likewise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197278.
ilya-biryukov added a comment.

- Revamp TokenSource, make it more principled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Lex/TokenLexer.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Preprocessor.cpp

Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -867,20 +867,36 @@
   // We loop here until a lex function returns a token; this avoids recursion.
   bool ReturnedToken;
   bool IsNewToken = true;
+  TokenSource Source;
   do {
+Source = TokenSource();
+
 switch (CurLexerKind) {
 case CLK_Lexer:
+  Source.InDirective = CurLexer->ParsingPreprocessorDirective;
+  Source.IsMacroArg = InMacroArgs;
+  Source.InMacroArgPreExpansion = InMacroArgPreExpansion;
+
   ReturnedToken = CurLexer->Lex(Result);
   break;
 case CLK_TokenLexer:
+  Source.IsMacroArg = InMacroArgs;
+  Source.InMacroArgPreExpansion = InMacroArgPreExpansion;
+  Source.IsCached = !CurTokenLexer->isMacroExpansion();
+
   ReturnedToken = CurTokenLexer->Lex(Result);
   break;
 case CLK_CachingLexer:
+  Source.IsCached = true;
+
   CachingLex(Result, IsNewToken);
   ReturnedToken = true;
   break;
 case CLK_LexAfterModuleImport:
-  ReturnedToken = LexAfterModuleImport(Result);
+  Source.InDirective = true;
+
+  LexAfterModuleImport(Result);
+  ReturnedToken = true;
   break;
 }
   } while (!ReturnedToken);
@@ -937,6 +953,8 @@
 
   LastTokenWasAt = Result.is(tok::at);
   --LexLevel;
+  if (OnToken)
+OnToken(Result, Source);
 }
 
 /// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -463,6 +463,15 @@
  const MacroDefinition &M) {
   MacroInfo *MI = M.getMacroInfo();
 
+  // The macro-expanded identifiers are not seen by the Lex() method.
+  if (OnToken) {
+TokenSource S;
+S.InDirective = CurLexer && CurLexer->ParsingPreprocessorDirective;
+S.InMacroArgPreExpansion = InMacroArgPreExpansion;
+S.IsMacroName = true;
+OnToken(Identifier, S);
+  }
+
   // If this is a macro expansion in the "#if !defined(x)" line for the file,
   // then the macro could expand to different things in other contexts, we need
   // to disable the optimization in this case.
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -404,6 +404,12 @@
   setCodeCompletionReached();
   continue;
 }
+// This token is not reported to
+if (OnToken) {
+  TokenSource S;
+  S.InSkippedPPBranch = true;
+  OnToken(Tok, S);
+}
 
 // If this is the end of the buffer, we have an error.
 if (Tok.is(tok::eof)) {
@@ -883,6 +889,13 @@
   // Save the '#' token in case we need to return it later.
   Token SavedHash = Result;
 
+  // Lex() never sees the '#' token from directives, so report it here.
+  if (OnToken) {
+TokenSource S;
+S.InDirective = true;
+OnToken(Result, S);
+  }
+
   // Read the next token, the directive flavor.  This isn't expanded due to
   // C99 6.10.3p8.
   LexUnexpandedToken(Result);
Index: clang/include/clang/Lex/TokenLexer.h
===
--- clang/include/clang/Lex/TokenLexer.h
+++ clang/include/clang/Lex/TokenLexer.h
@@ -147,6 +147,10 @@
   /// preprocessor directive.
   bool isParsingPreprocessorDirective() const;
 
+  /// Returns true iff the TokenLexer is expanding a macro and not replaying a
+  /// stream of tokens.
+  bool isMacroExpansion() const { return Macro != nullptr; }
+
 private:
   void destroy();
 
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -33,6 +33,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -48,8 +49,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -114,6 +115,23 @@
   MU_Undef  = 2
 };
 
+/// Captures information about where the tokens come from. Used by the callback
+/// that records tokens.
+struct TokenSource {
+  /// A token is a name of a macro in a ma

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197279.
ilya-biryukov added a comment.

- Simplify collection of tokens
- Move dumping code to the bottom of the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,624 @@
+//===- TokensTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using llvm::ValueIs;
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Not;
+using ::testing::StartsWith;
+
+namespace {
+// Checks the passed ArrayRef has the same begin() and end() iterators as the
+// argument.
+MATCHER_P(SameRange, A, "") {
+  return A.begin() == arg.begin() && A.end() == arg.end();
+}
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+
+class TokenCollectorTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+return llvm::make_unique();
+  }
+
+private:
+  TokenBuffer &Result;
+  llvm::Optional Collector;
+};
+
+constexpr const char *FileName = "./input.cpp";
+FS->addFile(FileName, time_t(), llvm::MemoryBuffer::getMemBufferCopy(""));
+// Prepare to run a compiler.
+std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
+  FileName};
+auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+assert(CI);
+CI->getFrontendOpts().DisableFree = false;
+CI->getPreprocess

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

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/CommandLine.cpp:95
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)

thopre wrote:
> Any reason why not take a StringRef of the option to compute the prefix of? 
> That would put the .size() gymnastic in one place instead of many.
Actually, that's what I originally had, but since it was only a few places, I 
changed it to take the length.  Now that I see it's used all over, I'll 
probably switch it back as you suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269



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

[PATCH] D61136: [Analyzer] IteratorChecker - Ensure end()>=begin() and refactor begin and end symbol creation

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

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



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1929-1930
+
+  auto &SymMgr = State->getSymbolManager();
+  auto Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount, "end");
+  State = assumeNoOverflow(State, Sym, 4);

NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > This is a bit more `auto` than allowed by [[ 
> > > https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> > >  | our coding standards ]] (it pretty much disallows `auto` unless it's 
> > > some sort of `dyn_cast` (`getAs`) or an iterator.
> > I can add the type, of course. However, until now I understood and it is 
> > also in the coding standard that "other places where the type is already 
> > obvious from the context". For me it is obvious that `conjureSymbol()` 
> > returns a `SymbolRef`. Even more obvious is the `getSymbolManager()` 
> > returns a `SymbolManager`.
> [[ https://reviews.llvm.org/D33672?id=171506#inline-475812 | Here is a 
> discussion on this matter that i had recently. ]] It was hard enough to 
> convince people that the code below follows the coding standards:
> ```lang=c++
> auto DV = V.getAs();
> ```
> 
> Also, `conjuredSymbol()` in fact returns a `const SymbolConjured *`, which is 
> a more specialized type. This probably deserves at least a `const auto *`.
+1, I strongly disagree with the overuse of auto. I think project-wide things 
like the one you mentioned (`SVal::getAs`) are fine, because after googleing it 
once of twice it genuinely makes the code far more readable. However, where the 
scope of the type (like in many cases in this file) is very small, line breaks 
aren't a concern, or the function just simply isn't widely used (and 
`conjureSymbol` really isn't) we shouldn't use auto.

In general, I'd be just far more conservative: whenever in doubt, just don't 
use it. Whenever it's anything but super-super obvious, don't use it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61136



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

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

[Dávid Bolvanský via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/DiagnosticIDs.h:39
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 3600,
   DIAG_SIZE_ANALYSIS  =  100,

@rsmith  maybe this should be a separate patch? I hit this limit in this patch.


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

https://reviews.llvm.org/D61288



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:47
 
-using TextGenerator =
-std::function;
+// \c TextGenerator may fail, because it processes dynamically-bound match
+// results.  For example, a typo in the name of a bound node or a mismatch in

NIT: maybe shorten a bit, still capturing the essence? Something like the 
following should be enough:

```
Note that \p TextGenerator is allowed to fail, e.g. when trying to access a 
matched node that was not bound.
Allowing this to fail simplifies error handling for interactive tools like 
clang-query.
```



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:58
 
-/// Wraps a string as a TextGenerator.
+/// Wraps a string as a (trivially successful) TextGenerator.
 inline TextGenerator text(std::string M) {

Maybe drop `trivially successful`? Does not seem to be super-important.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:238
+  /// occurs in constructing a single \c AtomicChange then the change is still
+  /// passed to \c Consumer, but it's error will be set.  Note that clients are
+  /// responsible for handling the case that independent \c AtomicChanges

s/it's/its



Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:164
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())

Maybe follow a typical pattern for handling errors here (to avoid  `OrErr` 
suffixes and an extra `Err` variable)? I.e.
```
auto Replacement = Edit.Replacement(Result);
if (!Replacement)
  return Replacement.takeError();
T.Replacement = std::move(*Replacement);
```




Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:205
   if (auto Err = TransformationsOrErr.takeError()) {
-llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
- << "\n";
+AC.setError(llvm::toString(std::move(Err)));
+Consumer(AC);

This looks super-complicated.
Having `Error` in `AtomicChange` seems like a bad idea in the first place, why 
would we choose to use it here?

The following alternatives would encourage clients to handle errors properly:
1. accept an `Expected` in our callback,
2. provide a separate callback to consume errors.

WDYT about picking one of those two?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D61051: [analyzer] Treat functions without runtime branches as "small".

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

Szelethus added a comment.

I mean, better late then never, but LGTM :^)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61051



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

[clang-tools-extra] r359548 - Fix Wpedantic "default argument specified for lambda parameter" warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Apr 30 03:35:37 2019
New Revision: 359548

URL: http://llvm.org/viewvc/llvm-project?rev=359548&view=rev
Log:
Fix Wpedantic "default argument specified for lambda parameter" warning. NFCI.

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=359548&r1=359547&r2=359548&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Apr 30 03:35:37 2019
@@ -1463,7 +1463,7 @@ private:
 llvm::DenseMap BundleLookup;
 auto AddToBundles = [&](const CodeCompletionResult *SemaResult,
 const Symbol *IndexResult,
-const RawIdentifier *IdentifierResult = nullptr) {
+const RawIdentifier *IdentifierResult) {
   CompletionCandidate C;
   C.SemaResult = SemaResult;
   C.IndexResult = IndexResult;
@@ -1502,12 +1502,12 @@ private:
 };
 // Emit all Sema results, merging them with Index results if possible.
 for (auto &SemaResult : SemaResults)
-  AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult));
+  AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult), nullptr);
 // Now emit any Index-only results.
 for (const auto &IndexResult : IndexResults) {
   if (UsedIndexResults.count(&IndexResult))
 continue;
-  AddToBundles(/*SemaResult=*/nullptr, &IndexResult);
+  AddToBundles(/*SemaResult=*/nullptr, &IndexResult, nullptr);
 }
 // Emit identifier results.
 for (const auto &Ident : IdentifierResults)


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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Change logic to count references from only main files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -45,6 +45,9 @@
   return llvm::StringRef(arg.Definition.FileURI) == U;
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
+MATCHER_P(NumReferences, N, "") {
+  return arg.References == static_cast(N);
+}
 
 namespace clang {
 namespace clangd {
@@ -81,7 +84,7 @@
   FileSymbols FS;
   EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty());
 
-  FS.update("f1", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cc"));
+  FS.update("f1", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cc"), false);
   EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""),
   UnorderedElementsAre(QName("1"), QName("2"), QName("3")));
   EXPECT_THAT(getRefs(*FS.buildIndex(IndexType::Light), SymbolID("1")),
@@ -90,8 +93,8 @@
 
 TEST(FileSymbolsTest, Overlap) {
   FileSymbols FS;
-  FS.update("f1", numSlab(1, 3), nullptr);
-  FS.update("f2", numSlab(3, 5), nullptr);
+  FS.update("f1", numSlab(1, 3), nullptr, false);
+  FS.update("f2", numSlab(3, 5), nullptr, false);
   for (auto Type : {IndexType::Light, IndexType::Heavy})
 EXPECT_THAT(runFuzzyFind(*FS.buildIndex(Type), ""),
 UnorderedElementsAre(QName("1"), QName("2"), QName("3"),
@@ -110,8 +113,8 @@
   auto X2 = symbol("x");
   X2.Definition.FileURI = "file:///x2";
 
-  FS.update("f1", OneSymboSlab(X1), nullptr);
-  FS.update("f2", OneSymboSlab(X2), nullptr);
+  FS.update("f1", OneSymboSlab(X1), nullptr, false);
+  FS.update("f2", OneSymboSlab(X2), nullptr, false);
   for (auto Type : {IndexType::Light, IndexType::Heavy})
 EXPECT_THAT(
 runFuzzyFind(*FS.buildIndex(Type, DuplicateHandling::Merge), "x"),
@@ -123,14 +126,14 @@
   FileSymbols FS;
 
   SymbolID ID("1");
-  FS.update("f1", numSlab(1, 3), refSlab(ID, "f1.cc"));
+  FS.update("f1", numSlab(1, 3), refSlab(ID, "f1.cc"), false);
 
   auto Symbols = FS.buildIndex(IndexType::Light);
   EXPECT_THAT(runFuzzyFind(*Symbols, ""),
   UnorderedElementsAre(QName("1"), QName("2"), QName("3")));
   EXPECT_THAT(getRefs(*Symbols, ID), RefsAre({FileURI("f1.cc")}));
 
-  FS.update("f1", nullptr, nullptr);
+  FS.update("f1", nullptr, nullptr, false);
   auto Empty = FS.buildIndex(IndexType::Light);
   EXPECT_THAT(runFuzzyFind(*Empty, ""), IsEmpty());
   EXPECT_THAT(getRefs(*Empty, ID), ElementsAre());
@@ -366,6 +369,33 @@
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
+  FileSymbols FS;
+  FS.update("f1", numSlab(1, 3), nullptr, true);
+  FS.update("f2", numSlab(1, 3), nullptr, false);
+  EXPECT_THAT(
+  runFuzzyFind(*FS.buildIndex(IndexType::Light, DuplicateHandling::Merge),
+   ""),
+  UnorderedElementsAre(AllOf(QName("1"), NumReferences(0)),
+   AllOf(QName("2"), NumReferences(0)),
+   AllOf(QName("3"), NumReferences(0;
+}
+
+TEST(FileSymbolsTest, CountReferencesWithRefSlabs) {
+  FileSymbols FS;
+  FS.update("f1cpp", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cpp"), true);
+  FS.update("f1h", numSlab(1, 3), refSlab(SymbolID("1"), "f1.h"), false);
+  FS.update("f2cpp", numSlab(1, 3), refSlab(SymbolID("2"), "f2.cpp"), true);
+  FS.update("f2h", numSlab(1, 3), refSlab(SymbolID("2"), "f2.h"), false);
+  FS.update("f3cpp", numSlab(1, 3), refSlab(SymbolID("3"), "f3.cpp"), true);
+  FS.update("f3h", numSlab(1, 3), refSlab(SymbolID("3"), "f3.h"), false);
+  EXPECT_THAT(
+  runFuzzyFind(*FS.buildIndex(IndexType::Light, DuplicateHandling::Merge),
+   ""),
+  UnorderedElementsAre(AllOf(QName("1"), NumReferences(1)),
+   AllOf(QName("2"), NumReferences(1)),
+   AllOf(QName("3"), NumReferences(1;
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -32,6 +32,7 @@
   return !arg.IsTU && !arg.URI.empty() && arg.D

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Tyker via Phabricator via cfe-commits]

Tyker marked 2 inline comments as done.
Tyker added inline comments.



Comment at: clang/include/clang/AST/DeclBase.h:1539-1541
+uint64_t NumCtorInitializers : 64 - NumDeclContextBits -
+NumFunctionDeclBits -
+/*Other used bits in CXXConstructorDecl*/ 3;

rsmith wrote:
> I would prefer that we keep an explicit number here so that we can ensure 
> that this field has the range we desire.
couldn't we compute the value and static_assert on it. having to modify this 
each time we modify DeclContextBits or FunctionDeclBits is sad. and there isn't 
anything reminding us to do so in some cases.
what would be a reasonable minimum ?



Comment at: clang/lib/Sema/SemaInit.cpp:3817-3831
+if (AllowExplicit || !Conv->isExplicit()) {
   if (ConvTemplate)
-S.AddTemplateConversionCandidate(ConvTemplate, I.getPair(),
- ActingDC, Initializer, DestType,
- CandidateSet, AllowExplicit,
- /*AllowResultConversion*/false);
+S.AddTemplateConversionCandidate(
+ConvTemplate, I.getPair(), ActingDC, Initializer, DestType,
+CandidateSet, AllowExplicit, AllowExplicit,
+/*AllowResultConversion*/ false);
   else

rsmith wrote:
> We no longer pass `false` for `AllowExplicit` to `Add*Candidate` when 
> `CopyInitializing` is `true`. Is that an intentional change?
yes. i didn't found any cases in which AllowExplicit was false but 
CopyInitializing was true. so i assumed that relying only on AllowExplicit is 
better. the regression tests passes after the change.


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

https://reviews.llvm.org/D60934



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

r359550 - Fix gcc "-Wdangling-else" warnings. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Apr 30 03:57:02 2019
New Revision: 359550

URL: http://llvm.org/viewvc/llvm-project?rev=359550&view=rev
Log:
Fix gcc "-Wdangling-else" warnings. NFCI.

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=359550&r1=359549&r2=359550&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Apr 30 03:57:02 2019
@@ -4137,8 +4137,9 @@ struct RedeclChain : ASTImporterOptionSp
 auto *ToD = LastDeclMatcher().match(ToTU, getPattern());
 EXPECT_TRUE(ImportedD == ToD);
 EXPECT_FALSE(ToD->isThisDeclarationADefinition());
-if (auto *ToT = dyn_cast(ToD))
+if (auto *ToT = dyn_cast(ToD)) {
   EXPECT_TRUE(ToT->getTemplatedDecl());
+}
   }
 
   void TypedTest_DefinitionShouldBeImportedAsADefinition() {
@@ -4152,8 +4153,9 @@ struct RedeclChain : ASTImporterOptionSp
 EXPECT_EQ(DeclCounter().match(ToTU, getPattern()), 1u);
 auto *ToD = LastDeclMatcher().match(ToTU, getPattern());
 EXPECT_TRUE(ToD->isThisDeclarationADefinition());
-if (auto *ToT = dyn_cast(ToD))
+if (auto *ToT = dyn_cast(ToD)) {
   EXPECT_TRUE(ToT->getTemplatedDecl());
+}
   }
 
   void TypedTest_ImportPrototypeAfterImportedPrototype() {
@@ -4266,8 +4268,9 @@ struct RedeclChain : ASTImporterOptionSp
 auto *To0 = FirstDeclMatcher().match(ToTU, getPattern());
 EXPECT_TRUE(Imported0 == To0);
 EXPECT_TRUE(To0->isThisDeclarationADefinition());
-if (auto *ToT0 = dyn_cast(To0))
+if (auto *ToT0 = dyn_cast(To0)) {
   EXPECT_TRUE(ToT0->getTemplatedDecl());
+}
   }
 
   void TypedTest_ImportDefinitionThenPrototype() {


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

r359551 - Fix gcc "-Wdangling-else" warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Apr 30 03:57:37 2019
New Revision: 359551

URL: http://llvm.org/viewvc/llvm-project?rev=359551&view=rev
Log:
Fix gcc "-Wdangling-else" warning. NFCI.

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

Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=359551&r1=359550&r2=359551&view=diff
==
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Tue Apr 30 03:57:37 2019
@@ -217,8 +217,9 @@ TEST(LookupTest, replaceNestedClassName)
   // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
   // it's not visible at [0].
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
-if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old") {
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };
   Visitor.runOver(R"(
 // a.h


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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

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

Szelethus requested review of this revision.
Szelethus added a comment.

Similarly to D60925 , I plan to make a patch 
that will hide most the implementation options.


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

https://reviews.llvm.org/D57858



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

[PATCH] D61304: [OpenCL][PR41609] Deduce static data members to __global addr space

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, rjmccall.
Herald added subscribers: ebevhan, yaxunl.

Similarly to static variables in OpenCL, static class data members should be 
deduced to `__global` addr space.


https://reviews.llvm.org/D61304

Files:
  lib/Sema/SemaType.cpp
  test/SemaOpenCLCXX/address-space-deduction.cl


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -0,0 +1,12 @@
+//RUN: %clang_cc1 %s -cl-std=c++ -pedantic -ast-dump -verify
+
+//expected-no-diagnostics
+
+//CHECK: |-VarDecl  foo {{.*}} 'const __global int' constexpr cinit
+constexpr int foo = 0;
+
+class c {
+public:
+  //CHECK: `-VarDecl {{.*}} foo2 'const __global int' static constexpr cinit
+  static constexpr int foo2 = 0;
+};
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7308,8 +7308,10 @@
// otherwise it will fail some sema check.
   IsFuncReturnType || IsFuncType ||
   // Do not deduce addr space for member types of struct, except the 
pointee
-  // type of a pointer member type.
-  (D.getContext() == DeclaratorContext::MemberContext && !IsPointee) ||
+  // type of a pointer member type or static data members.
+  (D.getContext() == DeclaratorContext::MemberContext &&
+   (!IsPointee &&
+D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static)) ||
   // Do not deduce addr space for types used to define a typedef and the
   // typedef itself, except the pointee type of a pointer type which is 
used
   // to define the typedef.


Index: test/SemaOpenCLCXX/address-space-deduction.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-deduction.cl
@@ -0,0 +1,12 @@
+//RUN: %clang_cc1 %s -cl-std=c++ -pedantic -ast-dump -verify
+
+//expected-no-diagnostics
+
+//CHECK: |-VarDecl  foo {{.*}} 'const __global int' constexpr cinit
+constexpr int foo = 0;
+
+class c {
+public:
+  //CHECK: `-VarDecl {{.*}} foo2 'const __global int' static constexpr cinit
+  static constexpr int foo2 = 0;
+};
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7308,8 +7308,10 @@
// otherwise it will fail some sema check.
   IsFuncReturnType || IsFuncType ||
   // Do not deduce addr space for member types of struct, except the pointee
-  // type of a pointer member type.
-  (D.getContext() == DeclaratorContext::MemberContext && !IsPointee) ||
+  // type of a pointer member type or static data members.
+  (D.getContext() == DeclaratorContext::MemberContext &&
+   (!IsPointee &&
+D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static)) ||
   // Do not deduce addr space for types used to define a typedef and the
   // typedef itself, except the pointee type of a pointer type which is used
   // to define the typedef.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Seems reasonable for OpenCL kernels. You might want to add an AST dump test to 
check that the visibility is being set correctly in case it's being printed in 
AST.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61274



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

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: include/clang/Basic/DiagnosticIDs.h:39
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 3600,
   DIAG_SIZE_ANALYSIS  =  100,

xbolva00 wrote:
> @rsmith  maybe this should be a separate patch? I hit this limit in this 
> patch.
Is there any downside to just bumping it to, say, 4000 ?


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

https://reviews.llvm.org/D61288



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 2 inline comments as done.
ymandel added inline comments.



Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:205
   if (auto Err = TransformationsOrErr.takeError()) {
-llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
- << "\n";
+AC.setError(llvm::toString(std::move(Err)));
+Consumer(AC);

ilya-biryukov wrote:
> This looks super-complicated.
> Having `Error` in `AtomicChange` seems like a bad idea in the first place, 
> why would we choose to use it here?
> 
> The following alternatives would encourage clients to handle errors properly:
> 1. accept an `Expected` in our callback,
> 2. provide a separate callback to consume errors.
> 
> WDYT about picking one of those two?
Agreed! I was using `setError` on the assumption that it was the "standard" way 
to express errors.  Given that it seems to be totally ignored otherwise, let's 
go with option 1. I'll update the revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

r359558 - Fix inconsistency in calculating DIAG_START_ values.

[Russell Gallop via cfe-commits]

Author: russell_gallop
Date: Tue Apr 30 05:53:19 2019
New Revision: 359558

URL: http://llvm.org/viewvc/llvm-project?rev=359558&view=rev
Log:
Fix inconsistency in calculating DIAG_START_ values.

This was introduced at r313975. As DIAG_SIZE_CROSSTU and
DIAG_SIZE_COMMENT are both 100 this should be NFC.

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticIDs.h

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=359558&r1=359557&r2=359558&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Tue Apr 30 05:53:19 2019
@@ -50,8 +50,8 @@ namespace clang {
   DIAG_START_PARSE = DIAG_START_LEX   + DIAG_SIZE_LEX,
   DIAG_START_AST   = DIAG_START_PARSE + DIAG_SIZE_PARSE,
   DIAG_START_COMMENT   = DIAG_START_AST   + DIAG_SIZE_AST,
-  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_CROSSTU,
-  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_COMMENT,
+  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_COMMENT,
+  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_CROSSTU,
   DIAG_START_ANALYSIS  = DIAG_START_SEMA  + DIAG_SIZE_SEMA,
   DIAG_START_REFACTORING   = DIAG_START_ANALYSIS  + DIAG_SIZE_ANALYSIS,
   DIAG_UPPER_LIMIT = DIAG_START_REFACTORING   + 
DIAG_SIZE_REFACTORING


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

[PATCH] D61264: Fix inconsistency in calculating DIAG_START values

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359558: Fix inconsistency in calculating DIAG_START_ values. 
(authored by russell_gallop, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61264?vs=197080&id=197304#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61264

Files:
  cfe/trunk/include/clang/Basic/DiagnosticIDs.h


Index: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
===
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h
@@ -50,8 +50,8 @@
   DIAG_START_PARSE = DIAG_START_LEX   + DIAG_SIZE_LEX,
   DIAG_START_AST   = DIAG_START_PARSE + DIAG_SIZE_PARSE,
   DIAG_START_COMMENT   = DIAG_START_AST   + DIAG_SIZE_AST,
-  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_CROSSTU,
-  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_COMMENT,
+  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_COMMENT,
+  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_CROSSTU,
   DIAG_START_ANALYSIS  = DIAG_START_SEMA  + DIAG_SIZE_SEMA,
   DIAG_START_REFACTORING   = DIAG_START_ANALYSIS  + DIAG_SIZE_ANALYSIS,
   DIAG_UPPER_LIMIT = DIAG_START_REFACTORING   + 
DIAG_SIZE_REFACTORING


Index: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
===
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h
@@ -50,8 +50,8 @@
   DIAG_START_PARSE = DIAG_START_LEX   + DIAG_SIZE_LEX,
   DIAG_START_AST   = DIAG_START_PARSE + DIAG_SIZE_PARSE,
   DIAG_START_COMMENT   = DIAG_START_AST   + DIAG_SIZE_AST,
-  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_CROSSTU,
-  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_COMMENT,
+  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_COMMENT,
+  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_CROSSTU,
   DIAG_START_ANALYSIS  = DIAG_START_SEMA  + DIAG_SIZE_SEMA,
   DIAG_START_REFACTORING   = DIAG_START_ANALYSIS  + DIAG_SIZE_ANALYSIS,
   DIAG_UPPER_LIMIT = DIAG_START_REFACTORING   + DIAG_SIZE_REFACTORING
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61309: [clang] Add no-warn support for Wa

[Brian Cain via Phabricator via cfe-commits]

bcain created this revision.
bcain added reviewers: pcc, kparzysz, sidneym.
bcain added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61309

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/as-no-warnings.c
  clang/tools/driver/cc1as_main.cpp

Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -131,6 +131,7 @@
   unsigned RelaxAll : 1;
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
+  unsigned NoWarn : 1;
   unsigned IncrementalLinkerCompatible : 1;
   unsigned EmbedBitcode : 1;
 
@@ -156,6 +157,7 @@
 RelaxAll = 0;
 NoExecStack = 0;
 FatalWarnings = 0;
+NoWarn = 0;
 IncrementalLinkerCompatible = 0;
 DwarfVersion = 0;
 EmbedBitcode = 0;
@@ -285,6 +287,7 @@
   Opts.RelaxAll = Args.hasArg(OPT_mrelax_all);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
+  Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
   Opts.RelocationModel = Args.getLastArgValue(OPT_mrelocation_model, "pic");
   Opts.TargetABI = Args.getLastArgValue(OPT_target_abi);
   Opts.IncrementalLinkerCompatible =
@@ -375,6 +378,8 @@
   std::unique_ptr MOFI(new MCObjectFileInfo());
 
   MCTargetOptions MCOptions;
+  MCOptions.MCNoWarn = Opts.NoWarn;
+  MCOptions.MCFatalWarnings = Opts.FatalWarnings;
   MCContext Ctx(MAI.get(), MRI.get(), MOFI.get(), &SrcMgr, &MCOptions);
 
   bool PIC = false;
Index: clang/test/Driver/as-no-warnings.c
===
--- /dev/null
+++ clang/test/Driver/as-no-warnings.c
@@ -0,0 +1,14 @@
+// RUN: %clang -### %s -c -o tmp.o -target i686-pc-linux-gnu -fno-integrated-as -Wa,--no-warn 2>&1 | FileCheck -check-prefix=CHECK-NOIAS %s
+// RUN: %clang -### %s -c -o tmp.o -target i686-pc-linux-gnu -integrated-as -Wa,--no-warn 2>&1 | FileCheck %s
+// RUN: %clang %s -c -o %t.o -target i686-pc-linux-gnu -integrated-as -Wa,--no-warn 2>&1 | FileCheck -allow-empty --check-prefix=CHECK-AS-NOWARN %s
+// RUN: %clang %s -c -o %t.o -target i686-pc-linux-gnu -fno-integrated-as -Wa,--no-warn 2>&1 | FileCheck -allow-empty --check-prefix=CHECK-AS-NOWARN %s
+// RUN: not %clang %s -c -o %t.o -target i686-pc-linux-gnu -integrated-as -Wa,--fatal-warnings 2>&1 | FileCheck --check-prefix=CHECK-AS-FATAL %s
+// RUN: not %clang %s -c -o %t.o -target i686-pc-linux-gnu -fno-integrated-as -Wa,--fatal-warnings 2>&1 | FileCheck --check-prefix=CHECK-AS-FATAL %s
+
+// CHECK: "-cc1" {{.*}} "-massembler-no-warn"
+// CHECK-NOIAS: "--no-warn"
+// CHECK-AS-NOWARN-NOT: warning:
+// CHECK-AS-FATAL-NOT: warning:
+// CHECK-AS-FATAL: error
+
+__asm(".warning");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -856,6 +856,7 @@
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
+  Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
   Opts.EnableSegmentedStacks = Args.hasArg(OPT_split_stacks);
   Opts.RelaxAll = Args.hasArg(OPT_mrelax_all);
   Opts.IncrementalLinkerCompatible =
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -130,7 +130,7 @@
   const Driver &D = HTC.getDriver();
   ArgStringList CmdArgs;
 
-  CmdArgs.push_back("-march=hexagon");
+  CmdArgs.push_back("-arch=hexagon");
 
   RenderExtraToolArgs(JA, CmdArgs);
 
@@ -148,6 +148,12 @@
 CmdArgs.push_back("-fsyntax-only");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mhexagon_hvx,
+   options::OPT_mno_hexagon_hvx)) {
+if (A->getOption().matches(options::OPT_mhexagon_hvx))
+  CmdArgs.push_back("-mhvx");
+  }
+
   if (auto G = toolchains::HexagonToolChain::getSmallDataThreshold(Args)) {
 CmdArgs.push_back(Args.MakeArgString("-gpsize=" + Twine(G.getValue(;
   }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2145,6 +2145,8 @@
 // Do nothing, this is the default and we don't support anything else.
   } else if (Value == "-L") {
 CmdArgs.push_back("-msave-temp-labels");
+  } else if (Value == "--no-warn") {
+CmdArgs.push_back("-massembler-no-warn");

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Addresses comments, including error handling style and signature of 
ChangeConsumer.

Updates testing code to use new ChangeConsumer signature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -10,6 +10,8 @@
 
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,6 +20,8 @@
 using namespace ast_matchers;
 
 namespace {
+using ::testing::IsEmpty;
+
 constexpr char KHeaderContents[] = R"cc(
   struct string {
 string(const char*);
@@ -84,26 +88,43 @@
 Factory->create(), Code, std::vector(), "input.cc",
 "clang-tool", std::make_shared(),
 FileContents)) {
+  llvm::errs() << "Running tool failed.\n";
+  return None;
+}
+if (ErrorCount != 0) {
+  llvm::errs() << "Generating changes failed.\n";
   return None;
 }
-auto ChangedCodeOrErr =
+auto ChangedCode =
 applyAtomicChanges("input.cc", Code, Changes, ApplyChangesSpec());
-if (auto Err = ChangedCodeOrErr.takeError()) {
-  llvm::errs() << "Change failed: " << llvm::toString(std::move(Err))
-   << "\n";
+if (!ChangedCode) {
+  llvm::errs() << "Applying changes failed: "
+   << llvm::toString(ChangedCode.takeError()) << "\n";
   return None;
 }
-return *ChangedCodeOrErr;
+return *ChangedCode;
+  }
+
+  Transformer::ChangeConsumer consumer() {
+return [this](Expected C) {
+  if (C) {
+Changes.push_back(std::move(*C));
+  } else {
+consumeError(C.takeError());
+++ErrorCount;
+  }
+};
   }
 
   void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
-Transformer T(std::move(Rule),
-  [this](const AtomicChange &C) { Changes.push_back(C); });
+Transformer T(std::move(Rule), consumer());
 T.registerMatchers(&MatchFinder);
 compareSnippets(Expected, rewrite(Input));
   }
 
   clang::ast_matchers::MatchFinder MatchFinder;
+  // Records whether any errors occurred in individual changes.
+  int ErrorCount = 0;
   AtomicChanges Changes;
 
 private:
@@ -356,6 +377,23 @@
 // Negative tests (where we expect no transformation to occur).
 //
 
+// Tests for a conflict in edits from a single match for a rule.
+TEST_F(TransformerTest, TextGeneratorFailure) {
+  std::string Input = "int conflictOneRule() { return 3 + 7; }";
+  // Try to change the whole binary-operator expression AND one its operands:
+  StringRef O = "O";
+  auto AlwaysFail = [](const ast_matchers::MatchFinder::MatchResult &)
+  -> llvm::Expected {
+return llvm::createStringError(llvm::errc::invalid_argument, "ERROR");
+  };
+  Transformer T(makeRule(binaryOperator().bind(O), change(O, AlwaysFail)),
+consumer());
+  T.registerMatchers(&MatchFinder);
+  EXPECT_FALSE(rewrite(Input));
+  EXPECT_THAT(Changes, IsEmpty());
+  EXPECT_EQ(ErrorCount, 1);
+}
+
 // Tests for a conflict in edits from a single match for a rule.
 TEST_F(TransformerTest, OverlappingEditsInRule) {
   std::string Input = "int conflictOneRule() { return 3 + 7; }";
@@ -364,13 +402,11 @@
   Transformer T(
   makeRule(binaryOperator(hasLHS(expr().bind(L))).bind(O),
{change(O, "DELETE_OP"), change(L, "DELETE_LHS")}),
-  [this](const AtomicChange &C) { Changes.push_back(C); });
+  consumer());
   T.registerMatchers(&MatchFinder);
-  // The rewrite process fails...
-  EXPECT_TRUE(rewrite(Input));
-  // ... but one AtomicChange was consumed:
-  ASSERT_EQ(Changes.size(), 1u);
-  EXPECT_TRUE(Changes[0].hasError());
+  EXPECT_FALSE(rewrite(Input));
+  EXPECT_THAT(Changes, IsEmpty());
+  EXPECT_EQ(ErrorCount, 1);
 }
 
 // Tests for a conflict in edits across multiple matches (of the same rule).
@@ -379,27 +415,27 @@
   // Try to change the whole binary-operator expression AND one its operands:
   StringRef E = "E";
   Transformer T(makeRule(expr().bind(E), change(E, "DELETE_EXPR")),
-[this](const AtomicChange &C) { Changes.push_back(C); });
+consumer());
   T.registerMatchers(&MatchFinder);
   // The rewrite process fails because the changes conflict with each other...
   EXPECT_FALSE(rewrite(Input));
-  // ... but all changes are (individually) fine:
-  ASSERT_EQ(Changes.size(), 2u);
-  EXPECT_FALSE(Changes[0].hasError());

[PATCH] D61130: [llvm-mc] Add reportWarning() to MCContext

[Brian Cain via Phabricator via cfe-commits]

bcain updated this revision to Diff 197317.
bcain changed the repository for this revision from rL LLVM to rG LLVM Github 
Monorepo.
bcain added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add context, add clang update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61130

Files:
  clang/tools/driver/cc1as_main.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCChecker.cpp
  llvm/test/MC/Hexagon/nowarn.s
  llvm/tools/llvm-mc/llvm-mc.cpp

Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm/tools/llvm-mc/llvm-mc.cpp
+++ llvm/tools/llvm-mc/llvm-mc.cpp
@@ -279,7 +279,7 @@
 static int AssembleInput(const char *ProgName, const Target *TheTarget,
  SourceMgr &SrcMgr, MCContext &Ctx, MCStreamer &Str,
  MCAsmInfo &MAI, MCSubtargetInfo &STI,
- MCInstrInfo &MCII, MCTargetOptions &MCOptions) {
+ MCInstrInfo &MCII, MCTargetOptions const &MCOptions) {
   std::unique_ptr Parser(
   createMCAsmParser(SrcMgr, Ctx, Str, MAI));
   std::unique_ptr TAP(
@@ -316,7 +316,7 @@
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);
 
   cl::ParseCommandLineOptions(argc, argv, "llvm machine code playground\n");
-  MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
+  const MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
   setDwarfDebugFlags(argc, argv);
 
   setDwarfDebugProducer();
@@ -368,7 +368,7 @@
   // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
   // MCObjectFileInfo needs a MCContext reference in order to initialize itself.
   MCObjectFileInfo MOFI;
-  MCContext Ctx(MAI.get(), MRI.get(), &MOFI, &SrcMgr);
+  MCContext Ctx(MAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions);
   MOFI.InitMCObjectFileInfo(TheTriple, PIC, Ctx, LargeCodeModel);
 
   if (SaveTempLabels)
Index: llvm/test/MC/Hexagon/nowarn.s
===
--- /dev/null
+++ llvm/test/MC/Hexagon/nowarn.s
@@ -0,0 +1,19 @@
+# RUN: llvm-mc -arch=hexagon -mhvx --filetype=asm %s -o - 2>&1 | FileCheck %s
+# RUN: llvm-mc --no-warn -arch=hexagon -mhvx --filetype=obj %s -o - | llvm-objdump -d - | FileCheck --check-prefix=CHECK-NOWARN %s
+# RUN: not llvm-mc --fatal-warnings -arch=hexagon -mhvx --filetype=asm %s 2>&1 | FileCheck --check-prefix=CHECK-FATAL-WARN %s
+
+	.text
+.warning
+
+{
+  v7.tmp = vmem(r28 + #3)
+  v7:6.w = vadd(v17:16.w, v17:16.w)
+  v17:16.uw = vunpack(v8.uh)
+}
+
+# CHECK-NOWARN-NOT: warning
+# CHECK-FATAL-WARN-NOT: warning
+# CHECK-FATAL-WARN: error
+# CHECK-FATAL-WARN: error
+# CHECK: warning:
+# CHECK: warning:
Index: llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCChecker.cpp
===
--- llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCChecker.cpp
+++ llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCChecker.cpp
@@ -727,9 +727,6 @@
 }
 
 void HexagonMCChecker::reportWarning(Twine const &Msg) {
-  if (ReportErrors) {
-auto SM = Context.getSourceManager();
-if (SM)
-  SM->PrintMessage(MCB.getLoc(), SourceMgr::DK_Warning, Msg);
-  }
+  if (ReportErrors)
+Context.reportWarning(MCB.getLoc(), Msg);
 }
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -56,11 +56,11 @@
 
 MCContext::MCContext(const MCAsmInfo *mai, const MCRegisterInfo *mri,
  const MCObjectFileInfo *mofi, const SourceMgr *mgr,
- bool DoAutoReset)
+ MCTargetOptions const *TargetOpts, bool DoAutoReset)
 : SrcMgr(mgr), InlineSrcMgr(nullptr), MAI(mai), MRI(mri), MOFI(mofi),
   Symbols(Allocator), UsedNames(Allocator),
   CurrentDwarfLoc(0, 0, 0, DWARF2_FLAG_IS_STMT, 0, 0),
-  AutoReset(DoAutoReset) {
+  AutoReset(DoAutoReset), TargetOptions(TargetOpts) {
   SecureLogFile = AsSecureLogFileName;
 
   if (SrcMgr && SrcMgr->getNumBuffers())
@@ -652,6 +652,21 @@
 report_fatal_error(Msg, false);
 }
 
+void MCContext::reportWarning(SMLoc Loc, const Twine &Msg) {
+  if (TargetOptions && TargetOptions->MCNoWarn)
+return;
+  if (TargetOptions && TargetOptions->MCFatalWarnings)
+reportError(Loc, Msg);
+  else {
+// If we have a source manager use it. Otherwise, try using the inline
+// source manager.
+if (SrcMgr)
+  SrcMgr->PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
+else if (InlineSrcMgr)
+  InlineSrcMgr->PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
+  }
+}
+
 void MCContext::reportFatalError(SMLoc Loc, const Twine &Msg) {
   reportError(Loc, Msg);
 
Index: llvm/lib/CodeGen/MachineModuleInfo.cpp
=

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:164
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())

ilya-biryukov wrote:
> Maybe follow a typical pattern for handling errors here (to avoid  `OrErr` 
> suffixes and an extra `Err` variable)? I.e.
> ```
> auto Replacement = Edit.Replacement(Result);
> if (!Replacement)
>   return Replacement.takeError();
> T.Replacement = std::move(*Replacement);
> ```
> 
Here and elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[clang-tools-extra] r359564 - [clangd] gen_std uses multiprocessing pool to be fast. While here, log ambiguous symbols. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Apr 30 07:21:10 2019
New Revision: 359564

URL: http://llvm.org/viewvc/llvm-project?rev=359564&view=rev
Log:
[clangd] gen_std uses multiprocessing pool to be fast. While here, log 
ambiguous symbols. NFC

Modified:
clang-tools-extra/trunk/clangd/include-mapping/gen_std.py

Modified: clang-tools-extra/trunk/clangd/include-mapping/gen_std.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/include-mapping/gen_std.py?rev=359564&r1=359563&r2=359564&view=diff
==
--- clang-tools-extra/trunk/clangd/include-mapping/gen_std.py (original)
+++ clang-tools-extra/trunk/clangd/include-mapping/gen_std.py Tue Apr 30 
07:21:10 2019
@@ -31,8 +31,11 @@ Usage:
 from bs4 import BeautifulSoup
 
 import argparse
+import collections
 import datetime
+import multiprocessing
 import os
+import signal
 import sys
 
 STDGEN_CODE_PREFIX = """\
@@ -101,7 +104,12 @@ class Symbol:
 self.headers = headers
 
 
-def GetSymbols(root_dir, index_page_name, namespace):
+def ReadSymbolPage(path, name):
+  with open(path) as f:
+return ParseSymbolPage(f.read())
+
+
+def GetSymbols(pool, root_dir, index_page_name, namespace):
   """Get all symbols listed in the index page. All symbols should be in the
   given namespace.
 
@@ -114,24 +122,22 @@ def GetSymbols(root_dir, index_page_name
   #  contains the defined header.
   #   2. Parse the symbol page to get the defined header.
   index_page_path = os.path.join(root_dir, index_page_name)
-  symbols = []
   with open(index_page_path, "r") as f:
-# A map from symbol name to a set of headers.
-symbol_headers = {}
+# Read each symbol page in parallel.
+results = [] # (symbol_name, promise of [header...])
 for symbol_name, symbol_page_path in ParseIndexPage(f.read()):
-  with open(os.path.join(root_dir, symbol_page_path), "r") as f:
-headers = ParseSymbolPage(f.read())
-  if not headers:
-sys.stderr.write("No header found for symbol %s at %s\n" % 
(symbol_name,
-  symbol_page_path))
-continue
-
-  if symbol_name not in symbol_headers:
-symbol_headers[symbol_name] = set()
-  symbol_headers[symbol_name].update(headers)
+  path = os.path.join(root_dir, symbol_page_path)
+  results.append((symbol_name,
+  pool.apply_async(ReadSymbolPage, (path, symbol_name
+
+# Build map from symbol name to a set of headers.
+symbol_headers = collections.defaultdict(set)
+for symbol_name, lazy_headers in results:
+  symbol_headers[symbol_name].update(lazy_headers.get())
 
-for name, headers in sorted(symbol_headers.items(), key=lambda t : t[0]):
-  symbols.append(Symbol(name, namespace, list(headers)))
+  symbols = []
+  for name, headers in sorted(symbol_headers.items(), key=lambda t : t[0]):
+symbols.append(Symbol(name, namespace, list(headers)))
   return symbols
 
 
@@ -169,8 +175,16 @@ def main():
   ]
 
   symbols = []
-  for root_dir, page_name, namespace in parse_pages:
-symbols.extend(GetSymbols(root_dir, page_name, namespace))
+  # Run many workers to process individual symbol pages under the symbol index.
+  # Don't allow workers to capture Ctrl-C.
+  pool = multiprocessing.Pool(
+  initializer=lambda: signal.signal(signal.SIGINT, signal.SIG_IGN))
+  try:
+for root_dir, page_name, namespace in parse_pages:
+  symbols.extend(GetSymbols(pool, root_dir, page_name, namespace))
+  finally:
+pool.terminate()
+pool.join()
 
   # We don't have version information from the unzipped offline HTML files.
   # so we use the modified time of the symbol_index.html as the version.
@@ -179,12 +193,16 @@ def main():
 os.stat(index_page_path).st_mtime).strftime('%Y-%m-%d')
   print STDGEN_CODE_PREFIX % cppreference_modified_date
   for symbol in symbols:
-if len(symbol.headers) > 1:
+if len(symbol.headers) == 1:
+  # SYMBOL(unqualified_name, namespace, header)
+  print "SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
+symbol.headers[0])
+elif len(symbol.headers) == 0:
+  sys.stderr.write("No header found for symbol %s\n" % symbol.name)
+else:
   # FIXME: support symbols with multiple headers (e.g. std::move).
-  continue
-# SYMBOL(unqualified_name, namespace, header)
-print "SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
-  symbol.headers[0])
+  sys.stderr.write("Ambiguous header for symbol %s: %s\n" % (
+  symbol.name, ', '.join(symbol.headers)))
 
 
 if __name__ == '__main__':


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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I'm confused by some of the decisions made in this change, see below.




Comment at: clang-tools-extra/trunk/clangd/test/lit.cfg.in:29
+  config.available_features.add('clangd-xpc-support')
+

trailing newline



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+@LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.

Every other LLVM project puts the lit.cfg.in for the unit tests in 
foo/test/Unit/lit.cfg.in. Any reason why clangd is different?

Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file 
lit.cfg.py.in since it helps Windows. Can you rename the lit.cfg(.in) files you 
added to have .py in them please?

Also, why did you merge the site.cfg.py.in file with the non-generated non-site 
file? This too is different from all other LLVM projects.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:11
+# Point the dynamic loader at dynamic libraries in 'lib'.
+# XXX: it seems every project has a copy of this logic. Move it somewhere.
+import platform

Did you mean to commit this XXX?



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:23
+
+

Lots of trailing newlines.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

[Sam McCall via Phabricator via cfe-commits]

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

Previously we were just jumping from the symbol index to the symbol page, and
grabbing all the headers mentioned there. But the page often lists multiple
symbols, and so we got false positives and thus ambiguities (which were 
dropped).

Now we look at which declarations are for the symbol we want, and prefer headers
listed above that symbol. If there are none, we fall back to the old behavior.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61316

Files:
  clangd/StdSymbolMap.inc
  clangd/include-mapping/gen_std.py

Index: clangd/include-mapping/gen_std.py
===
--- clangd/include-mapping/gen_std.py
+++ clangd/include-mapping/gen_std.py
@@ -35,6 +35,7 @@
 import datetime
 import multiprocessing
 import os
+import re
 import signal
 import sys
 
@@ -50,7 +51,13 @@
 //===--===//
 """
 
-def ParseSymbolPage(symbol_page_html):
+def HasClass(tag, *classes):
+  for c in tag.get('class', []):
+if c in classes:
+  return True
+  return False
+
+def ParseSymbolPage(symbol_page_html, symbol_name):
   """Parse symbol page and retrieve the include header defined in this page.
   The symbol page provides header for the symbol, specifically in
   "Defined in header " section. An example:
@@ -61,17 +68,38 @@
 
   Returns a list of headers.
   """
-  headers = []
+  headers = set()
+  all_headers = set()
 
   soup = BeautifulSoup(symbol_page_html, "html.parser")
-  #  "Defined in header " are defined in  or
-  #  .
-  for header_tr in soup.select('tr.t-dcl-header,tr.t-dsc-header'):
-if "Defined in header " in header_tr.text:
-  # The interesting header content (e.g. ) is wrapped in .
-  for header_code in header_tr.find_all("code"):
-headers.append(header_code.text)
-  return headers
+  # Rows in table are like:
+  #   Defined in header   .t-dsc-header
+  #   Defined in header   .t-dsc-header
+  #   decl1.t-dcl
+  #   Defined in header   .t-dsc-header
+  #   decl2.t-dcl
+  for table in soup.select('table.t-dcl-begin, table.t-dsc-begin'):
+current_headers = []
+was_decl = False
+for row in table.select('tr'):
+  if HasClass(row, 't-dcl', 't-dsc'):
+was_decl = True
+# Declaration is in the first cell.
+text = row.find('td').text
+# Decl may not be for the symbol name we're looking for.
+if not re.search("\\b%s\\b" % symbol_name, text):
+  continue
+headers.update(current_headers)
+  elif HasClass(row, 't-dsc-header'):
+if was_decl:
+  current_headers = []
+was_decl = False
+# The interesting header content (e.g. ) is wrapped in .
+for header_code in row.find_all("code"):
+  current_headers.append(header_code.text)
+  all_headers.add(header_code.text)
+  # If the symbol was never named, consider all named headers.
+  return headers or all_headers
 
 
 def ParseIndexPage(index_page_html):
@@ -106,7 +134,7 @@
 
 def ReadSymbolPage(path, name):
   with open(path) as f:
-return ParseSymbolPage(f.read())
+return ParseSymbolPage(f.read(), name)
 
 
 def GetSymbols(pool, root_dir, index_page_name, namespace):
Index: clangd/StdSymbolMap.inc
===
--- clangd/StdSymbolMap.inc
+++ clangd/StdSymbolMap.inc
@@ -134,12 +134,15 @@
 SYMBOL(bad_variant_access, std::, )
 SYMBOL(bad_weak_ptr, std::, )
 SYMBOL(basic_common_reference, std::, )
+SYMBOL(basic_filebuf, std::, )
 SYMBOL(basic_fstream, std::, )
 SYMBOL(basic_ifstream, std::, )
 SYMBOL(basic_ios, std::, )
 SYMBOL(basic_iostream, std::, )
+SYMBOL(basic_istream, std::, )
 SYMBOL(basic_istringstream, std::, )
 SYMBOL(basic_ofstream, std::, )
+SYMBOL(basic_ostream, std::, )
 SYMBOL(basic_ostringstream, std::, )
 SYMBOL(basic_osyncstream, std::, )
 SYMBOL(basic_regex, std::, )
@@ -193,6 +196,7 @@
 SYMBOL(codecvt, std::, )
 SYMBOL(codecvt_base, std::, )
 SYMBOL(codecvt_byname, std::, )
+SYMBOL(codecvt_mode, std::, )
 SYMBOL(codecvt_utf16, std::, )
 SYMBOL(codecvt_utf8, std::, )
 SYMBOL(codecvt_utf8_utf16, std::, )
@@ -245,6 +249,7 @@
 SYMBOL(declare_reachable, std::, )
 SYMBOL(declval, std::, )
 SYMBOL(default_delete, std::, )
+SYMBOL(default_random_engine, std::, )
 SYMBOL(default_searcher, std::, )
 SYMBOL(defaultfloat, std::, )
 SYMBOL(defer_lock, std::, )
@@ -264,8 +269,10 @@
 SYMBOL(disjunction, std::, )
 SYMBOL(disjunction_v, std::, )
 SYMBOL(distance, std::, )
+SYMBOL(div_t, std::, )
 SYMBOL(divides, std::, )
 SYMBOL(domain_error, std::, )
+SYMBOL(double_t, std::, )
 SYMBOL(dynamic_extent, std::, )
 SYMBOL(dynamic_pointer_cast, std::, )
 SYMBOL(emit_on_flush, std::, )
@@ 

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+@LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.

thakis wrote:
> Every other LLVM project puts the lit.cfg.in for the unit tests in 
> foo/test/Unit/lit.cfg.in. Any reason why clangd is different?
> 
> Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file 
> lit.cfg.py.in since it helps Windows. Can you rename the lit.cfg(.in) files 
> you added to have .py in them please?
> 
> Also, why did you merge the site.cfg.py.in file with the non-generated 
> non-site file? This too is different from all other LLVM projects.
One consequence of these differences (except things being harder to understand) 
is that running `bin/llvm-lit ../llvm-project/clang-tools-extra/clangd/test` 
doesn't do anything, and running a single test that way doesn't work either. 
This works fine for individual llvm, clang, lld, clang-tools-extra tests and it 
used to work for clangd tests.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/trunk/clangd/test/CMakeLists.txt:5
+  # No tests for these, but we should still make sure they build.
+  clangd-indexer
+  dexp

Since you're touching this anyway, there's now one test that calls 
clang-indexer iirc.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

Friendly ping for comments. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+@LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.

thakis wrote:
> thakis wrote:
> > Every other LLVM project puts the lit.cfg.in for the unit tests in 
> > foo/test/Unit/lit.cfg.in. Any reason why clangd is different?
> > 
> > Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file 
> > lit.cfg.py.in since it helps Windows. Can you rename the lit.cfg(.in) files 
> > you added to have .py in them please?
> > 
> > Also, why did you merge the site.cfg.py.in file with the non-generated 
> > non-site file? This too is different from all other LLVM projects.
> One consequence of these differences (except things being harder to 
> understand) is that running `bin/llvm-lit 
> ../llvm-project/clang-tools-extra/clangd/test` doesn't do anything, and 
> running a single test that way doesn't work either. This works fine for 
> individual llvm, clang, lld, clang-tools-extra tests and it used to work for 
> clangd tests.
I know little about lit configurations but not being able to run a single test 
(`$build/bin/llvm-lit clangd/test/path/to/some/test`) seems a big downside..

(offtopic: compiler-rt tests have two modes: i386 and x86_64. I wonder if 
llvm-lit can run a test in the i386 mode (if it defaults to x86_64))


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: rjmccall, ebevhan.

This is a straw-man idea for solving binding references with address spaces to 
temporaries. If the logic below makes sense I will apply it elsewhere in 
similar checks.

Arbitrary address space references can't be bound to temporaries. The reference 
binding logic should check that the address space of temporary can be 
implicitly converted to address space in reference when temporary 
materialization is performed.

Example:

  _AS1 float gf;
  ...
  _AS1 const int& ref = gf; //this can only be accepted if implicit conversion 
from _AS1 to LangAS::Default is allowed

This review depends on: https://reviews.llvm.org/D58060


https://reviews.llvm.org/D61318

Files:
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Initialization.h
  lib/Sema/SemaInit.cpp
  test/SemaOpenCLCXX/address-space-references.cl

Index: test/SemaOpenCLCXX/address-space-references.cl
===
--- test/SemaOpenCLCXX/address-space-references.cl
+++ test/SemaOpenCLCXX/address-space-references.cl
@@ -9,3 +9,7 @@
 void foo() {
   bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
+
+__global const int &f(__global float &ref) {
+  return ref; // expected-error{{reference of type 'const __global int &' cannot bind to a temporary object because of address space mismatch}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -3336,6 +3336,7 @@
   case FK_NonConstLValueReferenceBindingToVectorElement:
   case FK_NonConstLValueReferenceBindingToUnrelated:
   case FK_RValueReferenceBindingToLValue:
+  case FK_ReferenceAddrspaceMismatchTemporary:
   case FK_ReferenceInitDropsQualifiers:
   case FK_ReferenceInitFailed:
   case FK_ConversionFailed:
@@ -4831,9 +4832,15 @@
 
   Sequence.AddReferenceBindingStep(cv1T1IgnoreAS, /*bindingTemporary=*/true);
 
-  if (T1Quals.hasAddressSpace())
+  if (T1Quals.hasAddressSpace()) {
+if (!T1Quals.isAddressSpaceSupersetOf(cv1T1IgnoreAS.getQualifiers())) {
+  Sequence.SetFailed(
+  InitializationSequence::FK_ReferenceAddrspaceMismatchTemporary);
+  return;
+}
 Sequence.AddQualificationConversionStep(cv1T1, isLValueRef ? VK_LValue
: VK_XValue);
+  }
 }
 
 /// Attempt character array initialization from a string literal
@@ -8497,6 +8504,11 @@
   << Args[0]->getSourceRange();
 break;
 
+  case FK_ReferenceAddrspaceMismatchTemporary:
+S.Diag(Kind.getLocation(), diag::err_reference_bind_temporary_addrspace)
+<< DestType << Args[0]->getSourceRange();
+break;
+
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = OnlyArg->getType();
 QualType NonRefType = DestType.getNonReferenceType();
@@ -8832,6 +8844,10 @@
   OS << "reference initialization drops qualifiers";
   break;
 
+case FK_ReferenceAddrspaceMismatchTemporary:
+  OS << "reference with mismatching address space bound to temporary";
+  break;
+
 case FK_ReferenceInitFailed:
   OS << "reference initialization failed";
   break;
Index: include/clang/Sema/Initialization.h
===
--- include/clang/Sema/Initialization.h
+++ include/clang/Sema/Initialization.h
@@ -1010,6 +1010,9 @@
 /// Reference binding drops qualifiers.
 FK_ReferenceInitDropsQualifiers,
 
+/// Reference with mismatching address space binding to temporary.
+FK_ReferenceAddrspaceMismatchTemporary,
+
 /// Reference binding failed.
 FK_ReferenceInitFailed,
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1840,6 +1840,9 @@
   "reference %diff{to %select{type|incomplete type}1 $ could not bind to an "
   "%select{rvalue|lvalue}2 of type $|could not bind to %select{rvalue|lvalue}2 of "
   "incompatible type}0,3">;
+def err_reference_bind_temporary_addrspace : Error<
+  "reference of type %0 cannot bind to a temporary object because of "
+  "address space mismatch">;
 def err_reference_bind_init_list : Error<
   "reference to type %0 cannot bind to an initializer list">;
 def err_init_list_bad_dest_type : Error<
Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@
   DIAG_SIZE_AST   =  150,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 3600,
   DIAG_SIZE

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 197350.
sammccall added a comment.

fix std::chrono regression


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61316

Files:
  clangd/StdSymbolMap.inc
  clangd/include-mapping/gen_std.py

Index: clangd/include-mapping/gen_std.py
===
--- clangd/include-mapping/gen_std.py
+++ clangd/include-mapping/gen_std.py
@@ -35,6 +35,7 @@
 import datetime
 import multiprocessing
 import os
+import re
 import signal
 import sys
 
@@ -50,7 +51,13 @@
 //===--===//
 """
 
-def ParseSymbolPage(symbol_page_html):
+def HasClass(tag, *classes):
+  for c in tag.get('class', []):
+if c in classes:
+  return True
+  return False
+
+def ParseSymbolPage(symbol_page_html, symbol_name):
   """Parse symbol page and retrieve the include header defined in this page.
   The symbol page provides header for the symbol, specifically in
   "Defined in header " section. An example:
@@ -61,17 +68,41 @@
 
   Returns a list of headers.
   """
-  headers = []
+  headers = set()
+  all_headers = set()
 
   soup = BeautifulSoup(symbol_page_html, "html.parser")
-  #  "Defined in header " are defined in  or
-  #  .
-  for header_tr in soup.select('tr.t-dcl-header,tr.t-dsc-header'):
-if "Defined in header " in header_tr.text:
-  # The interesting header content (e.g. ) is wrapped in .
-  for header_code in header_tr.find_all("code"):
-headers.append(header_code.text)
-  return headers
+  # Rows in table are like:
+  #   Defined in header   .t-dsc-header
+  #   Defined in header   .t-dsc-header
+  #   decl1.t-dcl
+  #   Defined in header   .t-dsc-header
+  #   decl2.t-dcl
+  for table in soup.select('table.t-dcl-begin, table.t-dsc-begin'):
+current_headers = []
+was_decl = False
+for row in table.select('tr'):
+  if HasClass(row, 't-dcl', 't-dsc'):
+was_decl = True
+# Declaration is in the first cell.
+text = row.find('td').text
+# Decl may not be for the symbol name we're looking for.
+if not re.search("\\b%s\\b" % symbol_name, text):
+  continue
+headers.update(current_headers)
+  elif HasClass(row, 't-dsc-header'):
+if was_decl:
+  current_headers = []
+was_decl = False
+# There are also .t-dsc-header for "defined in namespace".
+if not "Defined in header " in row.text:
+  continue
+# The interesting header content (e.g. ) is wrapped in .
+for header_code in row.find_all("code"):
+  current_headers.append(header_code.text)
+  all_headers.add(header_code.text)
+  # If the symbol was never named, consider all named headers.
+  return headers or all_headers
 
 
 def ParseIndexPage(index_page_html):
@@ -106,7 +137,7 @@
 
 def ReadSymbolPage(path, name):
   with open(path) as f:
-return ParseSymbolPage(f.read())
+return ParseSymbolPage(f.read(), name)
 
 
 def GetSymbols(pool, root_dir, index_page_name, namespace):
Index: clangd/StdSymbolMap.inc
===
--- clangd/StdSymbolMap.inc
+++ clangd/StdSymbolMap.inc
@@ -134,12 +134,15 @@
 SYMBOL(bad_variant_access, std::, )
 SYMBOL(bad_weak_ptr, std::, )
 SYMBOL(basic_common_reference, std::, )
+SYMBOL(basic_filebuf, std::, )
 SYMBOL(basic_fstream, std::, )
 SYMBOL(basic_ifstream, std::, )
 SYMBOL(basic_ios, std::, )
 SYMBOL(basic_iostream, std::, )
+SYMBOL(basic_istream, std::, )
 SYMBOL(basic_istringstream, std::, )
 SYMBOL(basic_ofstream, std::, )
+SYMBOL(basic_ostream, std::, )
 SYMBOL(basic_ostringstream, std::, )
 SYMBOL(basic_osyncstream, std::, )
 SYMBOL(basic_regex, std::, )
@@ -193,6 +196,7 @@
 SYMBOL(codecvt, std::, )
 SYMBOL(codecvt_base, std::, )
 SYMBOL(codecvt_byname, std::, )
+SYMBOL(codecvt_mode, std::, )
 SYMBOL(codecvt_utf16, std::, )
 SYMBOL(codecvt_utf8, std::, )
 SYMBOL(codecvt_utf8_utf16, std::, )
@@ -245,6 +249,7 @@
 SYMBOL(declare_reachable, std::, )
 SYMBOL(declval, std::, )
 SYMBOL(default_delete, std::, )
+SYMBOL(default_random_engine, std::, )
 SYMBOL(default_searcher, std::, )
 SYMBOL(defaultfloat, std::, )
 SYMBOL(defer_lock, std::, )
@@ -264,8 +269,10 @@
 SYMBOL(disjunction, std::, )
 SYMBOL(disjunction_v, std::, )
 SYMBOL(distance, std::, )
+SYMBOL(div_t, std::, )
 SYMBOL(divides, std::, )
 SYMBOL(domain_error, std::, )
+SYMBOL(double_t, std::, )
 SYMBOL(dynamic_extent, std::, )
 SYMBOL(dynamic_pointer_cast, std::, )
 SYMBOL(emit_on_flush, std::, )
@@ -298,6 +305,7 @@
 SYMBOL(extent, std::, )
 SYMBOL(extent_v, std::, )
 SYMBOL(extreme_value_distribution, std::, )
+SYMBOL(fabs, std::, )
 SYMBOL(false_type, std::, )
 SYMBOL(fclose, std::, )
 SYMBOL(fdim, std::, )
@@ -323,6 +331,7 @@
 SYMBOL(fgets, std::, )
 SYMBOL(fgetwc, std

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Thanks! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne abandoned this revision.
ldionne added a comment.

I decided to use a different approach and first refactor the header search 
logic from CC1 to the driver. A patch should come eventually when I've tested 
it properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61032



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D59168#1480801 , @phosek wrote:

> In D59168#1480428 , @jdenny wrote:
>
> > It seems we have roughly the same situation for their ABIs.  That is, for 
> > .so files, someone noted that libc++ and libunwind have always used the 
> > same version, .1, and openmp doesn't use a version (except in Debian 
> > packages).  In other words, for each of these three, a change in ABI 
> > compatibility has never been indicated.  For openmp only, it's apparently 
> > assumed a change never will need to be indicated (except in Debian 
> > packages).
>
>
> They already use `.2` for ABI v2 which is currently considered unstable but 
> some project like Fuchsia already use. There's also an ongoing discussion to 
> stabilize v2 
> .


Thanks, I wasn't aware.  I still see only .1 in my build, so apparently a 
different config is needed.




Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

I naively assumed libunwind would have its own directory so it could be 
selected independently.  Does this mean any library for C++ goes in c++?


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

https://reviews.llvm.org/D59168



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

[PATCH] D61319: [PR41674] [OpenCL] Fix initialisation of this via pointer

[Kévin Petit via Phabricator via cfe-commits]

kpet created this revision.
kpet added reviewers: Anastasia, mikael.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.

When the expression used to initialise this has a pointer type,
check the address space of the pointee type instead of the pointer
type to decide whether an address space cast is required.
It is the pointee type that carries the address space qualifier.


Repository:
  rC Clang

https://reviews.llvm.org/D61319

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaOpenCLCXX/this-initialisation-from-pointer.cl


Index: test/SemaOpenCLCXX/this-initialisation-from-pointer.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/this-initialisation-from-pointer.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -cl-std=c++ -triple spir -fsyntax-only -verify -ast-dump %s 
| FileCheck %s
+// expected-no-diagnostics
+
+struct foo {
+void init() {
+m_data = 1;
+}
+private:
+int m_data;
+};
+
+kernel void test(global struct foo* afoo) {
+afoo->init();
+// CHECK: CXXMemberCallExpr {{.*}}  'void'
+// CHECK-NEXT: MemberExpr {{.*}}  '' ->init {{.*}}
+// CHECK-NEXT: ImplicitCastExpr {{.*}}  '__generic foo *' 

+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5278,12 +5278,12 @@
   }
 
   if (!Context.hasSameType(From->getType(), DestType)) {
-if (From->getType().getAddressSpace() != DestType.getAddressSpace())
-  From = ImpCastExprToType(From, DestType, CK_AddressSpaceConversion,
- From->getValueKind()).get();
+CastKind CK;
+if (FromRecordType.getAddressSpace() != DestType.getAddressSpace())
+  CK = CK_AddressSpaceConversion;
 else
-  From = ImpCastExprToType(From, DestType, CK_NoOp,
- From->getValueKind()).get();
+  CK = CK_NoOp;
+From = ImpCastExprToType(From, DestType, CK, From->getValueKind()).get();
   }
   return From;
 }


Index: test/SemaOpenCLCXX/this-initialisation-from-pointer.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/this-initialisation-from-pointer.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -cl-std=c++ -triple spir -fsyntax-only -verify -ast-dump %s | FileCheck %s
+// expected-no-diagnostics
+
+struct foo {
+void init() {
+m_data = 1;
+}
+private:
+int m_data;
+};
+
+kernel void test(global struct foo* afoo) {
+afoo->init();
+// CHECK: CXXMemberCallExpr {{.*}}  'void'
+// CHECK-NEXT: MemberExpr {{.*}}  '' ->init {{.*}}
+// CHECK-NEXT: ImplicitCastExpr {{.*}}  '__generic foo *' 
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5278,12 +5278,12 @@
   }
 
   if (!Context.hasSameType(From->getType(), DestType)) {
-if (From->getType().getAddressSpace() != DestType.getAddressSpace())
-  From = ImpCastExprToType(From, DestType, CK_AddressSpaceConversion,
- From->getValueKind()).get();
+CastKind CK;
+if (FromRecordType.getAddressSpace() != DestType.getAddressSpace())
+  CK = CK_AddressSpaceConversion;
 else
-  From = ImpCastExprToType(From, DestType, CK_NoOp,
- From->getValueKind()).get();
+  CK = CK_NoOp;
+From = ImpCastExprToType(From, DestType, CK, From->getValueKind()).get();
   }
   return From;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@rjmccall or @ebevhan do you have any more feedback for this patch?

Btw, it has now dependency with https://reviews.llvm.org/D61318


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

https://reviews.llvm.org/D58060



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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D60907#1483660 , @jdoerfert wrote:

> In D60907#1483615 , @hfinkel wrote:
>
> > In D60907#1479370 , @gtbercea 
> > wrote:
> >
> > > In D60907#1479142 , @hfinkel 
> > > wrote:
> > >
> > > > In D60907#1479118 , @gtbercea 
> > > > wrote:
> > > >
> > > > > Ping @hfinkel @tra
> > > >
> > > >
> > > > The last two comments in D47849  
> > > > indicated exploration of a different approach, and one which still 
> > > > seems superior to this one. Can you please comment on why you're now 
> > > > pursuing this approach instead?
> > >
> > >
> > > ...
> > >
> > > Hal, as far as I can tell, this solution is similar to yours but with a 
> > > slightly different implementation. If there are particular aspects about 
> > > this patch you would like to discuss/give feedback on please let me know.
> >
> >
> > The solution I suggested had the advantages of:
> >
> > 1. Being able to directly reuse the code in 
> > `__clang_cuda_device_functions.h`. On the other hand, using this solution 
> > we need to implement a wrapper function for every math function. When 
> > `__clang_cuda_device_functions.h` is updated, we need to update the OpenMP 
> > wrapper as well.
>
>
> I'd even go as far as to argue that `__clang_cuda_device_functions.h` should 
> include the internal math.h wrapper to get all math functions. See also the 
> next comment.
>
> > 2. Providing access to wrappers for other CUDA intrinsics in a natural way 
> > (e.g., rnorm3d) [it looks a bit nicer to provide a host version of rnorm3d 
> > than __nv_rnorm3d in user code].
>
> @hfinkel 
>  I don't see why you want to mix CUDA intrinsics with math.h overloads.


What I had in mind was matching non-standard functions in a standard way. For 
example, let's just say that I have a CUDA kernel that uses the rnorm3d 
function, or I otherwise have a function that I'd like to write in OpenMP that 
will make good use of this CUDA function (because it happens to have an 
efficient device implementation). This is a function that CUDA provides, in the 
global namespace, although it's not standard.

Then I can do something like this (depending on how we setup the 
implementation):

  double rnorm3d(double a,  double b, double c) {
return sqrt(a*a + b*b + c*c);
  }
  
  ...
  
  #pragma omp target
  {
double a = ..., b = ..., c = ...;
double r = rnorm3d(a, b, c)
  }

and, if we use the CUDA math headers for CUDA math-function support, than this 
might "just work." To be clear, I can see an argument for having this work 
being a bad idea ;) -- but it has the advantage of providing a way to take 
advantage of system-specific functions while still writing completely-portable 
code.

>   I added a rough outline of how I imagined the internal math.h header to 
> look like as a comment in D47849. Could you elaborate how that differs from 
> what you imagine and how the other intrinsics come in?

That looks like what I had in mind (including `__clang_cuda_device_functions.h` 
to get the device functions.)

> 
> 
>> 3. Being similar to the "declare variant" functionality from OpenMP 5, and 
>> thus, I suspect, closer to the solution we'll eventually be able to apply in 
>> a standard way to all targets.
> 
> I can see this.
> 
>>> This solution is following Alexey's suggestions. This solution allows the 
>>> optimization of math calls if they apply (example: pow(x,2) => x*x ) which 
>>> was one of the issues in the previous solution I implemented.
>> 
>> So we're also missing that optimization for CUDA code when compiling with 
>> Clang? Isn't this also something that, regardless, should be fixed?
> 
> Maybe through a general built-in recognition and lowering into target 
> specific implementations/intrinsics late again?

I suspect that we need to match the intrinsics and perform the optimizations in 
LLVM at that level in order to get the optimizations for CUDA.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+@LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.

MaskRay wrote:
> thakis wrote:
> > thakis wrote:
> > > Every other LLVM project puts the lit.cfg.in for the unit tests in 
> > > foo/test/Unit/lit.cfg.in. Any reason why clangd is different?
> > > 
> > > Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this 
> > > file lit.cfg.py.in since it helps Windows. Can you rename the 
> > > lit.cfg(.in) files you added to have .py in them please?
> > > 
> > > Also, why did you merge the site.cfg.py.in file with the non-generated 
> > > non-site file? This too is different from all other LLVM projects.
> > One consequence of these differences (except things being harder to 
> > understand) is that running `bin/llvm-lit 
> > ../llvm-project/clang-tools-extra/clangd/test` doesn't do anything, and 
> > running a single test that way doesn't work either. This works fine for 
> > individual llvm, clang, lld, clang-tools-extra tests and it used to work 
> > for clangd tests.
> I know little about lit configurations but not being able to run a single 
> test (`$build/bin/llvm-lit clangd/test/path/to/some/test`) seems a big 
> downside..
> 
> (offtopic: compiler-rt tests have two modes: i386 and x86_64. I wonder if 
> llvm-lit can run a test in the i386 mode (if it defaults to x86_64))
> Every other LLVM project puts the lit.cfg.in for the unit tests in 
> foo/test/Unit/lit.cfg.in. Any reason why clangd is different? ... Also, why 
> did you merge the site.cfg.py.in 

Most of these changes were motivated by reducing the number of moving parts, 
which inhibit understanding. It's likely to be harder to understand if you're 
knowledgeable about the way things are done in LLVM. My reasoning in weighing 
simplicity over matching LLVM precisely:
 - the overlap in active contributors appears fairly small (maybe I'm missing 
other ways people interact with the code)
 - average understanding of the fairly elaborate LLVM setup seems fairly low, I 
would describe the median response to e.g. the `Unit` directory as "baffled".

> Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file 
> lit.cfg.py.in since it helps Windows

Can you elaborate, or point to some docs?

> One consequence of these differences (except things being harder to 
> understand) is that running `bin/llvm-lit 
> ../llvm-project/clang-tools-extra/clangd/test` doesn't do anything

Indeed. This works from the build directory though, which is where we typically 
run tests.
It also works for `.../unittests`, which is (IMO) much more obvious than 
`.../test/Unit`.

> and running a single test that way doesn't work either
AFAICS it never did for unit tests? lit tests are not a significant fraction of 
our tests.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:11
+# Point the dynamic loader at dynamic libraries in 'lib'.
+# XXX: it seems every project has a copy of this logic. Move it somewhere.
+import platform

thakis wrote:
> Did you mean to commit this XXX?
Yes. Probably should have been spelled FIXME.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:23
+
+

thakis wrote:
> Lots of trailing newlines.
Are these actually confusing, or significant in some way?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

r359574 - [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Tue Apr 30 09:48:33 2019
New Revision: 359574

URL: http://llvm.org/viewvc/llvm-project?rev=359574&view=rev
Log:
[LibTooling] Change Transformer's TextGenerator to a partial function.

Summary:
Changes the signature of the TextGenerator std::function to return an 
Expected
instead of std::string to allow for (non-fatal) failures.  Previously, we
expected that any failures would be expressed with assertions. However, that's
unfriendly to running the code in servers or other places that don't want their
library calls to crash the program.

Correspondingly, updates Transformer's handling of failures in TextGenerators
and the signature of `ChangeConsumer`.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=359574&r1=359573&r2=359574&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Tue Apr 30 
09:48:33 2019
@@ -44,12 +44,16 @@ enum class NodePart {
   Name,
 };
 
-using TextGenerator =
-std::function;
+// Note that \p TextGenerator is allowed to fail, e.g. when trying to access a
+// matched node that was not bound.  Allowing this to fail simplifies error
+// handling for interactive tools like clang-query.
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
 
 /// Wraps a string as a TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+ -> Expected { return M; };
 }
 
 // Description of a source-code edit, expressed in terms of an AST node.
@@ -222,11 +226,13 @@ translateEdits(const ast_matchers::Match
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
   using ChangeConsumer =
-  std::function;
+  std::function Change)>;
 
-  /// \param Consumer Receives each successful rewrite as an \c AtomicChange.
-  /// Note that clients are responsible for handling the case that independent
-  /// \c AtomicChanges conflict with each other.
+  /// \param Consumer Receives each rewrite or error.  Will not necessarily be
+  /// called for each match; for example, if the rewrite is not applicable
+  /// because of macros, but doesn't fail.  Note that clients are responsible
+  /// for handling the case that independent \c AtomicChanges conflict with 
each
+  /// other.
   Transformer(RewriteRule Rule, ChangeConsumer Consumer)
   : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {}
 

Modified: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp?rev=359574&r1=359573&r2=359574&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp Tue Apr 30 09:48:33 2019
@@ -153,16 +153,19 @@ tooling::translateEdits(const MatchResul
 auto It = NodesMap.find(Edit.Target);
 assert(It != NodesMap.end() && "Edit target must be bound in the match.");
 
-Expected RangeOrErr = getTargetRange(
+Expected Range = getTargetRange(
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
-if (auto Err = RangeOrErr.takeError())
-  return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+if (!Range)
+  return Range.takeError();
+if (Range->isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range->getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto Replacement = Edit.Replacement(Result);
+if (!Replacement)
+  return Replacement.takeError();
+Transformation T;
+T.Range = *Range;
+T.Replacement = std::move(*Replacement);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
@@ -194,14 +197,13 @@ void Transformer::run(const MatchResult
   Root->second.getSourceRange().getBegin());
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
-  auto TransformationsOrErr = translateEdits(Result, Rule.Edits);
-  if (auto Err = TransformationsOrErr.takeError()) {
-llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
- << "\n";
+  auto Tra

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ymandel marked an inline comment as done.
Closed by commit rL359574: [LibTooling] Change Transformer's TextGenerator 
to a partial function. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61015?vs=197316&id=197361#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61015

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

Index: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,16 @@
   Name,
 };
 
-using TextGenerator =
-std::function;
+// Note that \p TextGenerator is allowed to fail, e.g. when trying to access a
+// matched node that was not bound.  Allowing this to fail simplifies error
+// handling for interactive tools like clang-query.
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
 
 /// Wraps a string as a TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+ -> Expected { return M; };
 }
 
 // Description of a source-code edit, expressed in terms of an AST node.
@@ -222,11 +226,13 @@
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
   using ChangeConsumer =
-  std::function;
+  std::function Change)>;
 
-  /// \param Consumer Receives each successful rewrite as an \c AtomicChange.
-  /// Note that clients are responsible for handling the case that independent
-  /// \c AtomicChanges conflict with each other.
+  /// \param Consumer Receives each rewrite or error.  Will not necessarily be
+  /// called for each match; for example, if the rewrite is not applicable
+  /// because of macros, but doesn't fail.  Note that clients are responsible
+  /// for handling the case that independent \c AtomicChanges conflict with each
+  /// other.
   Transformer(RewriteRule Rule, ChangeConsumer Consumer)
   : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {}
 
Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -153,16 +153,19 @@
 auto It = NodesMap.find(Edit.Target);
 assert(It != NodesMap.end() && "Edit target must be bound in the match.");
 
-Expected RangeOrErr = getTargetRange(
+Expected Range = getTargetRange(
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
-if (auto Err = RangeOrErr.takeError())
-  return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+if (!Range)
+  return Range.takeError();
+if (Range->isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range->getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto Replacement = Edit.Replacement(Result);
+if (!Replacement)
+  return Replacement.takeError();
+Transformation T;
+T.Range = *Range;
+T.Replacement = std::move(*Replacement);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
@@ -194,14 +197,13 @@
   Root->second.getSourceRange().getBegin());
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
-  auto TransformationsOrErr = translateEdits(Result, Rule.Edits);
-  if (auto Err = TransformationsOrErr.takeError()) {
-llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
- << "\n";
+  auto Transformations = translateEdits(Result, Rule.Edits);
+  if (!Transformations) {
+Consumer(Transformations.takeError());
 return;
   }
-  auto &Transformations = *TransformationsOrErr;
-  if (Transformations.empty()) {
+
+  if (Transformations->empty()) {
 // No rewrite applied (but no error encountered either).
 RootLoc.print(llvm::errs() << "note: skipping match at loc ",
   *Result.SourceManager);
@@ -209,14 +211,14 @@
 return;
   }
 
-  // Convert the result to an AtomicChange.
+  // Record the results in the AtomicChange.
   AtomicChange AC(*Result.SourceManager, RootLoc);
-  for (const auto &T : Transformations) {
+  for (const auto &T : *Transformations) {
 if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
-  AC.setError(

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+@LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.

sammccall wrote:
> MaskRay wrote:
> > thakis wrote:
> > > thakis wrote:
> > > > Every other LLVM project puts the lit.cfg.in for the unit tests in 
> > > > foo/test/Unit/lit.cfg.in. Any reason why clangd is different?
> > > > 
> > > > Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this 
> > > > file lit.cfg.py.in since it helps Windows. Can you rename the 
> > > > lit.cfg(.in) files you added to have .py in them please?
> > > > 
> > > > Also, why did you merge the site.cfg.py.in file with the non-generated 
> > > > non-site file? This too is different from all other LLVM projects.
> > > One consequence of these differences (except things being harder to 
> > > understand) is that running `bin/llvm-lit 
> > > ../llvm-project/clang-tools-extra/clangd/test` doesn't do anything, and 
> > > running a single test that way doesn't work either. This works fine for 
> > > individual llvm, clang, lld, clang-tools-extra tests and it used to work 
> > > for clangd tests.
> > I know little about lit configurations but not being able to run a single 
> > test (`$build/bin/llvm-lit clangd/test/path/to/some/test`) seems a big 
> > downside..
> > 
> > (offtopic: compiler-rt tests have two modes: i386 and x86_64. I wonder if 
> > llvm-lit can run a test in the i386 mode (if it defaults to x86_64))
> > Every other LLVM project puts the lit.cfg.in for the unit tests in 
> > foo/test/Unit/lit.cfg.in. Any reason why clangd is different? ... Also, why 
> > did you merge the site.cfg.py.in 
> 
> Most of these changes were motivated by reducing the number of moving parts, 
> which inhibit understanding. It's likely to be harder to understand if you're 
> knowledgeable about the way things are done in LLVM. My reasoning in weighing 
> simplicity over matching LLVM precisely:
>  - the overlap in active contributors appears fairly small (maybe I'm missing 
> other ways people interact with the code)
>  - average understanding of the fairly elaborate LLVM setup seems fairly low, 
> I would describe the median response to e.g. the `Unit` directory as 
> "baffled".
> 
> > Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file 
> > lit.cfg.py.in since it helps Windows
> 
> Can you elaborate, or point to some docs?
> 
> > One consequence of these differences (except things being harder to 
> > understand) is that running `bin/llvm-lit 
> > ../llvm-project/clang-tools-extra/clangd/test` doesn't do anything
> 
> Indeed. This works from the build directory though, which is where we 
> typically run tests.
> It also works for `.../unittests`, which is (IMO) much more obvious than 
> `.../test/Unit`.
> 
> > and running a single test that way doesn't work either
> AFAICS it never did for unit tests? lit tests are not a significant fraction 
> of our tests.
> motivated by reducing the number of moving parts

That's a great motivation, and I agree the current state is hard to understand. 
But if every LLVM subsubproject does its own lit setup that they feel is 
simpler, the resulting whole will be even harder to understand than what we 
have now. Are you planning on changing the rest of c-t-e, clang, lld, llvm to 
use this new lit setup as well? If not, clangd should use the standard setup 
imho.

> Can you elaborate, or point to some docs?

It's just that Windows uses file extensions for file type associations heavily. 
If the file is called foo.py, you can double-click it in explorer and an editor 
will appear. See revision history of all the lit.(site.)cfg.py files in c-t-e, 
clang, llvm, lld for Zach's patches to move things to this world.

> AFAICS it never did for unit tests?

Yes, this only works for lit tests. LLVM in general tries to do most of its 
testing via lit tests.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

r359578 - [LibTooling] Fix broken test after r359574.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Tue Apr 30 10:24:36 2019
New Revision: 359578

URL: http://llvm.org/viewvc/llvm-project?rev=359578&view=rev
Log:
[LibTooling] Fix broken test after r359574.

r359574 changed the way that failures are reported, which broke the test 
TransformerTest.NodePartNameDeclRefFailure which detects a faiure.

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

Modified: cfe/trunk/unittests/Tooling/TransformerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/TransformerTest.cpp?rev=359578&r1=359577&r2=359578&view=diff
==
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp Tue Apr 30 10:24:36 2019
@@ -251,9 +251,11 @@ TEST_F(TransformerTest, NodePartNameDecl
   )cc";
 
   StringRef Ref = "ref";
-  testRule(makeRule(declRefExpr(to(functionDecl())).bind(Ref),
-change(Ref, NodePart::Name, "good")),
-   Input, Input);
+  Transformer T(makeRule(declRefExpr(to(functionDecl())).bind(Ref),
+ change(Ref, NodePart::Name, "good")),
+consumer());
+  T.registerMatchers(&MatchFinder);
+  EXPECT_FALSE(rewrite(Input));
 }
 
 TEST_F(TransformerTest, NodePartMember) {


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

[PATCH] D61324: Make check-clang depend on the clang-check binary always

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.
Herald added subscribers: llvm-commits, mgorny.
Herald added a project: LLVM.

check-clang (the target that runs all clang tests) used to
only depend on clang-check (a binary like clang-tidy,
clang-refactor, etc) if the static analyzer is enabled.
However, several lit tests call clang-check unconditionally,
so always depend on it.

Fixes a "could not find clang-check" lit warning in clean builds with
the static analyzer disabled.

Also sort the deps in the CMake file and put just one dep on each line.


https://reviews.llvm.org/D61324

Files:
  clang/test/CMakeLists.txt
  llvm/utils/gn/secondary/clang/test/BUILD.gn


Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -108,6 +108,7 @@
 ":lit_unit_site_cfg",
 "//clang/lib/Headers",
 "//clang/tools/c-index-test",
+"//clang/tools/clang-check",
 "//clang/tools/clang-diff",
 "//clang/tools/clang-format",
 "//clang/tools/clang-import-test",
@@ -147,7 +148,6 @@
   }
   if (clang_enable_static_analyzer) {
 deps += [
-  "//clang/tools/clang-check",
   "//clang/tools/clang-extdef-mapping",
 ]
   }
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -46,21 +46,23 @@
 endif ()
 
 list(APPEND CLANG_TEST_DEPS
-  clang clang-resource-headers
+  c-index-test
+  clang
+  clang-resource-headers
+  clang-check
   clang-format
-  c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
   clang-import-test
   clang-rename
   clang-refactor
   clang-diff
+  diagtool
   hmaptool
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)
   list(APPEND CLANG_TEST_DEPS
-clang-check
 clang-extdef-mapping
 )
 endif()


Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -108,6 +108,7 @@
 ":lit_unit_site_cfg",
 "//clang/lib/Headers",
 "//clang/tools/c-index-test",
+"//clang/tools/clang-check",
 "//clang/tools/clang-diff",
 "//clang/tools/clang-format",
 "//clang/tools/clang-import-test",
@@ -147,7 +148,6 @@
   }
   if (clang_enable_static_analyzer) {
 deps += [
-  "//clang/tools/clang-check",
   "//clang/tools/clang-extdef-mapping",
 ]
   }
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -46,21 +46,23 @@
 endif ()
 
 list(APPEND CLANG_TEST_DEPS
-  clang clang-resource-headers
+  c-index-test
+  clang
+  clang-resource-headers
+  clang-check
   clang-format
-  c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
   clang-import-test
   clang-rename
   clang-refactor
   clang-diff
+  diagtool
   hmaptool
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)
   list(APPEND CLANG_TEST_DEPS
-clang-check
 clang-extdef-mapping
 )
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

+1 to Hal's comments.

@jdoerfert :

> I'd even go as far as to argue that __clang_cuda_device_functions.h should 
> include the internal math.h wrapper to get all math functions. See also the 
> next comment.

I'd argue other way around -- include __clang_cuda_device_functions.h from 
math.h and do not preinclude anything.
If the user does not include math.h, it should not have its namespace polluted 
by some random stuff. NVCC did this, but that's one of the most annoying 
'features' we have to be compatible with for the sake of keeping existing 
nvcc-compilable CUDA code happy.

If users do include math.h, it should do the right thing, for both sides of the 
compilation.
IMO It's math.h that should be triggering pulling device functions in.




Comment at: lib/Driver/ToolChains/Clang.cpp:1157-1159
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP) &&
+  getToolChain().getTriple().isNVPTX())
+getToolChain().AddMathDeviceFunctions(Args, CmdArgs);

This functionality is openMP-specific, but the function name 
`AddMathDeviceFunctions()` is not. I'd rather keep OpenMP specialization down 
where it can be easily seen. Could this check be pushed down into 
CudaInstallationDetector::AddMathDeviceFunctions() ?

Another potential problem here is that this file will be pre-included only for 
the device. It will potentially result in more observable semantic differences 
between host and device compilations. I don't know if it matters for OpenMP, 
though.

IMO intercepting math.h and providing device-specific overloads *in addition* 
to the regular math.h functions would be a better approach.

Another problem with pre-included files is that sometimes users may 
intentionally need to *not* include them.
For CUDA we have `-nocudainc` flag. Your change, at least, will need something 
similar, IMO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks a test:  
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio

  [--] 1 test from TransformerTest
  [ RUN  ] TransformerTest.NodePartNameDeclRefFailure
  
/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/unittests/Tooling/TransformerTest.cpp:66:
 Failure
  Value of: MaybeActual
Actual: false
  Expected: true
  Rewrite failed. Expecting: 
  struct Y {
int operator*();
  };
  int neutral(int x) {
Y y;
int (Y::*ptr)() = &Y::operator*;
return *y + x;
  }

  [  FAILED  ] TransformerTest.NodePartNameDeclRefFailure (83 ms)
  [--] 1 test from TransformerTest (83 ms total)

Can you take a look?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61015



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2033
+
+  void setExplicitSpecifier(ExplicitSpecInfo ESI);
+

Tyker wrote:
> Tyker wrote:
> > rsmith wrote:
> > > Generally we don't want to have setters in the AST; the AST is intended 
> > > to be immutable after creation. Is this necessary?
> > this is used in 2 cases:
> > - for deserialization.
> > - for in `Sema::ActOnFunctionDeclarator` to make every declaration of the 
> > same function have the same explicit specifier as the canonical one. there 
> > is probably a better way to do this but i didn't find how.
> > 
> > the second use case will need to be changed because the constructor's 
> > explicit specifier will be tail-allocated so the explicit specifier from 
> > the canonical declaration will need to be recovered before construction to 
> > allocate storage. 
> > 
> > how can we find the canonical declaration of a function before it is 
> > constructed ?
> i found a solution around that issue.
> now setExplicitSpecifier is only used for deserialization.
as it is only used by deserialization and ASTDeclReader is friend to all class 
with a setExplicitSpecifier.
I made setExplicitSpecifier private.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6174
 CXXConversionDecl)) {
-  return Node.isExplicit();
+  return Node.getExplicitSpecifier().isSpecified();
 }

rsmith wrote:
> This will match `explicit(false)` constructors. I think 
> `getExplicitSpecifier().isExplicit()` would be better, but please also add a 
> FIXME here: it's not clear whether this should match a dependent 
> `explicit()`.
shouldn't this be able to match with deduction guides too ?



Comment at: clang/lib/Sema/SemaInit.cpp:9361
 // Only consider converting constructors.
-if (GD->isExplicit())
+if (!GD->isMaybeNotExplicit())
   continue;

rsmith wrote:
> Tyker wrote:
> > rsmith wrote:
> > > We need to substitute into the deduction guide first to detect whether it 
> > > forms a "converting constructor", and that will need to be done inside 
> > > `AddTemplateOverloadCandidate`.
> > similarly as the previous if. this check removes deduction guide that are 
> > already resolve to be explicit when we are in a context that doesn't allow 
> > explicit.
> > every time the explicitness was checked before my change i replaced it by a 
> > check that removes already resolved explicit specifiers.
> Unlike in the previous case, we do //not// yet pass an `AllowExplicit` flag 
> into `AddTemplateOverloadCandidate` here, so this will incorrectly allow 
> dependent //explicit-specifier//s that evaluate to `true` in 
> copy-initialization contexts.
the default value for `AllowExplicit` is false. so the conversion will be 
removed in `AddTemplateOverloadCandidate`.


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

https://reviews.llvm.org/D60934



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61015#1484669 , @thakis wrote:

> This breaks a test:  
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio
>
>   [--] 1 test from TransformerTest
>   [ RUN  ] TransformerTest.NodePartNameDeclRefFailure
>   
> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/unittests/Tooling/TransformerTest.cpp:66:
>  Failure
>   Value of: MaybeActual
> Actual: false
>   Expected: true
>   Rewrite failed. Expecting: 
>   struct Y {
> int operator*();
>   };
>   int neutral(int x) {
> Y y;
> int (Y::*ptr)() = &Y::operator*;
> return *y + x;
>   }
>
>   [  FAILED  ] TransformerTest.NodePartNameDeclRefFailure (83 ms)
>   [--] 1 test from TransformerTest (83 ms total)
>
>
> Can you take a look?


Fixed in r359578.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61015



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> Does llvm-elfabi consume your proposed Schema format? Has it landed yet?

No, I just proposed it and explained my reasoning. If you wanted to add this 
format to TextAPI that would be acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.
Herald added a project: LLVM.

I re-landed this to get access to the msan output, since the link above has 
long expired. Here's the new link: 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/31821/steps/check-clang%20msan/logs/stdio

Sadly, the bot output isn't super useful:

  Command Output (stderr):
  --
  
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/CodeGen/thinlto_backend.ll:10:18:
 error: CHECK-WARNING: expected string not found in input
  ; CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only 
allowed with '-x ir'
   ^
  :1:1: note: scanning from here
  ==7832==WARNING: MemorySanitizer: use-of-uninitialized-value
  ^
  
  --

I'll see if I can figure out how to do an msan build of llvm.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

jdenny wrote:
> I naively assumed libunwind would have its own directory so it could be 
> selected independently.  Does this mean any library for C++ goes in c++?
We could do that, but I'm not sure if it's worth doing without a clear use 
case? Do you know of any client that would want to consume libunwind 
independently of other C++ libraries? I'm aware of Rust but they build 
libunwind independently so there's no need for this.


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

https://reviews.llvm.org/D59168



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D59168#1484467 , @jdenny wrote:

> In D59168#1480801 , @phosek wrote:
>
> > In D59168#1480428 , @jdenny wrote:
> >
> > > It seems we have roughly the same situation for their ABIs.  That is, for 
> > > .so files, someone noted that libc++ and libunwind have always used the 
> > > same version, .1, and openmp doesn't use a version (except in Debian 
> > > packages).  In other words, for each of these three, a change in ABI 
> > > compatibility has never been indicated.  For openmp only, it's apparently 
> > > assumed a change never will need to be indicated (except in Debian 
> > > packages).
> >
> >
> > They already use `.2` for ABI v2 which is currently considered unstable but 
> > some project like Fuchsia already use. There's also an ongoing discussion 
> > to stabilize v2 
> > .
>
>
> Thanks, I wasn't aware.  I still see only .1 in my build, so apparently a 
> different config is needed.


It's the `LIBCXX_ABI_VERSION` CMake option, see 
https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121


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

https://reviews.llvm.org/D59168



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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D60907#1484529 , @hfinkel wrote:

> In D60907#1483660 , @jdoerfert wrote:
>
> > In D60907#1483615 , @hfinkel wrote:
> >
> > > In D60907#1479370 , @gtbercea 
> > > wrote:
> > >
> > > > In D60907#1479142 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D60907#1479118 , 
> > > > > @gtbercea wrote:
> > > > >
> > > > > > Ping @hfinkel @tra
> > > > >
> > > > >
> > > > > The last two comments in D47849  
> > > > > indicated exploration of a different approach, and one which still 
> > > > > seems superior to this one. Can you please comment on why you're now 
> > > > > pursuing this approach instead?
> > > >
> > > >
> > > > ...
> > > >
> > > > Hal, as far as I can tell, this solution is similar to yours but with a 
> > > > slightly different implementation. If there are particular aspects 
> > > > about this patch you would like to discuss/give feedback on please let 
> > > > me know.
> > >
> > >
> > > The solution I suggested had the advantages of:
> > >
> > > 1. Being able to directly reuse the code in 
> > > `__clang_cuda_device_functions.h`. On the other hand, using this solution 
> > > we need to implement a wrapper function for every math function. When 
> > > `__clang_cuda_device_functions.h` is updated, we need to update the 
> > > OpenMP wrapper as well.
> >
> >
> > I'd even go as far as to argue that `__clang_cuda_device_functions.h` 
> > should include the internal math.h wrapper to get all math functions. See 
> > also the next comment.
> >
> > > 2. Providing access to wrappers for other CUDA intrinsics in a natural 
> > > way (e.g., rnorm3d) [it looks a bit nicer to provide a host version of 
> > > rnorm3d than __nv_rnorm3d in user code].
> >
> > @hfinkel 
> >  I don't see why you want to mix CUDA intrinsics with math.h overloads.
>
>
> What I had in mind was matching non-standard functions in a standard way. For 
> example, let's just say that I have a CUDA kernel that uses the rnorm3d 
> function, or I otherwise have a function that I'd like to write in OpenMP 
> that will make good use of this CUDA function (because it happens to have an 
> efficient device implementation). This is a function that CUDA provides, in 
> the global namespace, although it's not standard.
>
> Then I can do something like this (depending on how we setup the 
> implementation):
>
>   double rnorm3d(double a,  double b, double c) {
> return sqrt(a*a + b*b + c*c);
>   }
>   
>   ...
>   
>   #pragma omp target
>   {
> double a = ..., b = ..., c = ...;
> double r = rnorm3d(a, b, c)
>   }
>   
>
> and, if we use the CUDA math headers for CUDA math-function support, than 
> this might "just work." To be clear, I can see an argument for having this 
> work being a bad idea ;) -- but it has the advantage of providing a way to 
> take advantage of system-specific functions while still writing 
> completely-portable code.


Matching `rnorm3d` and replacing it with some nvvm "intrinsic" is something I 
wouldn't like to see happening if `math.h` was included and not if it was not. 
As you say, in Cuda that is not how it works either. I'm in favor of reusing 
the built-in recognition mechanism:
That is, if the target is nvptx, the name is rnorm3d, we match that name and 
use the appropriate intrinsic, as we do others already for other targets.

>>   I added a rough outline of how I imagined the internal math.h header to 
>> look like as a comment in D47849. Could you elaborate how that differs from 
>> what you imagine and how the other intrinsics come in?
> 
> That looks like what I had in mind (including 
> `__clang_cuda_device_functions.h` to get the device functions.)
> 
>> 
>> 
>>> 3. Being similar to the "declare variant" functionality from OpenMP 5, and 
>>> thus, I suspect, closer to the solution we'll eventually be able to apply 
>>> in a standard way to all targets.
>> 
>> I can see this.
>> 
 This solution is following Alexey's suggestions. This solution allows the 
 optimization of math calls if they apply (example: pow(x,2) => x*x ) which 
 was one of the issues in the previous solution I implemented.
>>> 
>>> So we're also missing that optimization for CUDA code when compiling with 
>>> Clang? Isn't this also something that, regardless, should be fixed?
>> 
>> Maybe through a general built-in recognition and lowering into target 
>> specific implementations/intrinsics late again?
> 
> I suspect that we need to match the intrinsics and perform the optimizations 
> in LLVM at that level in order to get the optimizations for CUDA.

That seems reasonable to me. We could also match other intrinsics, e.g., 
`rnorm3d`, here as well, both by name but also by the computation pattern.

In D60907#148464

r359594 - AMDGPU: Enable _Float16

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 30 11:35:37 2019
New Revision: 359594

URL: http://llvm.org/viewvc/llvm-project?rev=359594&view=rev
Log:
AMDGPU: Enable _Float16

Added:
cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=359594&r1=359593&r2=359594&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Apr 30 11:35:37 2019
@@ -252,6 +252,9 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const
  !isAMDGCN(Triple));
   UseAddrSpaceMapMangling = true;
 
+  HasLegalHalfType = true;
+  HasFloat16 = true;
+
   // Set pointer width and alignment for target address space 0.
   PointerWidth = PointerAlign = DataLayout->getPointerSizeInBits();
   if (getMaxPointerWidth() == 64) {

Added: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp?rev=359594&view=auto
==
--- cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Tue Apr 30 11:35:37 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx701 -S -o - %s | 
FileCheck %s -check-prefix=NOF16
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx803 -S -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -S -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 -S -o - %s | 
FileCheck %s
+void f() {
+  _Float16 x, y, z;
+  // CHECK: v_add_f16_e64
+  // NOF16: v_add_f32_e64
+  z = x + y;
+  // CHECK: v_sub_f16_e64
+  // NOF16: v_sub_f32_e64
+  z = x - y;
+  // CHECK: v_mul_f16_e64
+  // NOF16: v_mul_f32_e64
+  z = x * y;
+  // CHECK: v_div_fixup_f16
+  // NOF16: v_div_fixup_f32
+  z = x / y;
+}


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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D38061#1484486 , @dblaikie wrote:

> Seems like the right thing would be for the DWARF code that wants a rendered 
> type name to pass its own printing policy, rather than changing some 
> relatively global one.
>
> (though also I have my doubts about the whole approach - macro expansion can 
> change the line number as well as the column number, so only suppressing 
> column number would be insufficient - and this also reduces the usability for 
> users (because the file/line/column of an anonymous type is a useful 
> debugging aid). Seems to me like this should be opt-in if it's supported at 
> all - though ideally build systems would use -frewrite-includes rather than 
> full preprocessing, and then macros and lines/columns would be preserved, I 
> think)


Line number is ok because preprocessor also inserts #linemarker, and later on 
these markers are used to recover the original line number before macro 
expansion. Column is problematic, as there's nothing like a column marker (if 
possible at all) so there's no way to see through the column change from macro 
expansion. We tried -frewrite-includes too, but it has its own issues - without 
macro expansion, it bloats the file size that needs to send to distcc remote 
machine significantly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Seems like the right thing would be for the DWARF code that wants a rendered 
type name to pass its own printing policy, rather than changing some relatively 
global one.

(though also I have my doubts about the whole approach - macro expansion can 
change the line number as well as the column number, so only suppressing column 
number would be insufficient - and this also reduces the usability for users 
(because the file/line/column of an anonymous type is a useful debugging aid). 
Seems to me like this should be opt-in if it's supported at all - though 
ideally build systems would use -frewrite-includes rather than full 
preprocessing, and then macros and lines/columns would be preserved, I think)


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D60907#1484756 , @jdoerfert wrote:

> I actually don't want to preinclude anything and my arguments are (mostly) 
> for the OpenMP offloading code path not necessarily Cuda.
>  Maybe to clarify, what I want is:
>
> 1. Make sure the `clang/Headers/math.h` is found first if `math.h` is 
> included.
> 2. Use a scheme similar to the one described 
> https://reviews.llvm.org/D47849#1483653 in `clang/Headers/math.h`
> 3. Only add `math.h` function overloads in our `math.h`. **<- This is 
> debatable**


Agreed.

> 4. Include `clang/Headers/math.h` from `__clang_cuda_device_functions.h` to 
> avoid duplication of math function declarations.

This is not needed for CUDA. `math.h` is included early on in 
`__clang_cuda_runtime_wrapper.h` (via ``), so by the time 
`__clang_cuda_device_functions.h` is included, math.h has already been included 
one way or another -- either in step 3 above, or directly by the 
__clang_cuda_runtime_wrapper.h


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D59168#1484754 , @phosek wrote:

> It's the `LIBCXX_ABI_VERSION` CMake option, see 
> https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121


Thanks.




Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

phosek wrote:
> jdenny wrote:
> > I naively assumed libunwind would have its own directory so it could be 
> > selected independently.  Does this mean any library for C++ goes in c++?
> We could do that, but I'm not sure if it's worth doing without a clear use 
> case? Do you know of any client that would want to consume libunwind 
> independently of other C++ libraries? I'm aware of Rust but they build 
> libunwind independently so there's no need for this.
I'm afraid I have no idea.  I'm not close enough to these libraries to be an 
adequate reviewer by myself.  Another reviewer should probably take a look at 
the new version of the patch.


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

https://reviews.llvm.org/D59168



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

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

vitalybuka told me that this should repro things locally:

  mkdir /tmp/bot
  cd /tmp/bot
  svn checkout https://llvm.org/svn/llvm-project/zorg
  BUILDBOT_CLOBBER= BUILDBOT_REVISION=YOUR_REV 
zorg/trunk/zorg/buildbot/builders/sanitizers/buildbot_fast.sh 2>&1 | tee 
buildlog.txt

I'm now running that and waiting for it.

In the meantime, 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/31821/steps/check-llvm%20asan/logs/stdio
 has a possibly better error:

  [ RUN  ] Option.FindNearest
  =
  ==64567==ERROR: AddressSanitizer: stack-use-after-scope on address 
0x7f488a118db1 at pc 0x00577ebb bp 0x7ffcd3a9ba90 sp 0x7ffcd3a9ba88
  READ of size 1 at 0x7f488a118db1 thread T0
  #0 0x577eba in unsigned int 
llvm::ComputeEditDistance(llvm::ArrayRef, llvm::ArrayRef, 
bool, unsigned int) 
/b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/edit_distance.h:81:25
  #1 0x56a016 in llvm::opt::OptTable::findNearest(llvm::StringRef, 
std::__1::basic_string, 
std::__1::allocator >&, unsigned int, unsigned int, unsigned int) const 
/b/sanitizer-x86_64-linux-fast/build/llvm/lib/Option/OptTable.cpp:301:21
  #2 0x54d0fa in Option_FindNearest_Test::TestBody() 
/b/sanitizer-x86_64-linux-fast/build/llvm/unittests/Option/OptionParsingTest.cpp:279:3
  #3 0x5f81a1 in HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
  #4 0x5f81a1 in testing::Test::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
  #5 0x5fa758 in testing::TestInfo::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
  #6 0x5fbd14 in testing::TestCase::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
  #7 0x619834 in testing::internal::UnitTestImpl::RunAllTests() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
  #8 0x6189af in 
HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
  #9 0x6189af in testing::UnitTest::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
  #10 0x5ded86 in RUN_ALL_TESTS 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
  #11 0x5ded86 in main 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
  #12 0x7f488d38a2e0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
  #13 0x42a3a9 in _start 
(/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/unittests/Option/OptionTests+0x42a3a9)
  
  Address 0x7f488a118db1 is located in stack of thread T0 at offset 433 in frame
  #0 0x56935f in llvm::opt::OptTable::findNearest(llvm::StringRef, 
std::__1::basic_string, 
std::__1::allocator >&, unsigned int, unsigned int, unsigned int) const 
/b/sanitizer-x86_64-linux-fast/build/llvm/lib/Option/OptTable.cpp:251


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



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

r359598 - Add requires amdgpu-registered-target for amdgpu-float16.cpp

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 30 12:06:15 2019
New Revision: 359598

URL: http://llvm.org/viewvc/llvm-project?rev=359598&view=rev
Log:
Add requires amdgpu-registered-target for amdgpu-float16.cpp

Modified:
cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp

Modified: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp?rev=359598&r1=359597&r2=359598&view=diff
==
--- cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Tue Apr 30 12:06:15 2019
@@ -1,3 +1,4 @@
+// REQUIRES: amdgpu-registered-target
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx701 -S -o - %s | 
FileCheck %s -check-prefix=NOF16
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx803 -S -o - %s | 
FileCheck %s
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -S -o - %s | 
FileCheck %s


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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the insightful explanation, Craig. As far as I understand, 
implementing intrinsics with builtins is possible but it is more complex and 
wasn't providing enough value to prefer it over inline assembly. If that is 
correct, I'd like to revive the abandoned implementation. Does it sound 
reasonable? And do I need to do something special for testing? Because I don't 
have access to corresponding hardware and unable to test the intrinsics on the 
actual hardware.


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

https://reviews.llvm.org/D44387



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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Your understanding is correct. As far as testing I think the existing testing 
in the original patches is sufficient.

I'm not sure I understand how a target specific intrinsic that only works on 
x86 in the bitcode is substantially better than inline assembly.   Do you plan 
to also change the cpuid intrinsics in cpuid.h that are also implemented in 
inline assembly?


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

https://reviews.llvm.org/D44387



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

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: shafik, teemperor, jingham, clayborg, a_sidorin.
Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus, 
dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added projects: clang, LLDB.

With LLDB we use localUncachedLookup(), however, that fails to find
Decls when a transparent context is involved and the given DC has
external lexical storage.  The solution is to use noload_lookup, which
works well with transparent contexts.  But, we cannot use only the
noload_lookup since the slow case of localUncachedLookup is still needed
in some other cases.

These other cases are handled in ASTImporterLookupTable, but we cannot
use that with LLDB since that traverses through the AST which initiates
the load of external decls again via DC::decls().

We must avoid loading external decls during the import becuase
ExternalASTSource is implemented with ASTImporter, so external loads
during import results in uncontrolled and faulty import.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61333

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Utility/Logging.h
  lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
  lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
  lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
  lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Symbol/ClangASTImporter.cpp
  lldb/source/Utility/Logging.cpp

Index: lldb/source/Utility/Logging.cpp
===
--- lldb/source/Utility/Logging.cpp
+++ lldb/source/Utility/Logging.cpp
@@ -17,6 +17,7 @@
 
 static constexpr Log::Category g_categories[] = {
   {{"api"}, {"log API calls and return values"}, LIBLLDB_LOG_API},
+  {{"ast"}, {"log AST"}, LIBLLDB_LOG_AST},
   {{"break"}, {"log breakpoints"}, LIBLLDB_LOG_BREAKPOINTS},
   {{"commands"}, {"log command argument parsing"}, LIBLLDB_LOG_COMMANDS},
   {{"comm"}, {"log communication activities"}, LIBLLDB_LOG_COMMUNICATION},
Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -918,6 +918,28 @@
   if (clang::TagDecl *to_tag = dyn_cast(to)) {
 if (clang::TagDecl *from_tag = dyn_cast(from)) {
   to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
+
+  Log *log_ast(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_AST));
+  if (log_ast) {
+std::string name_string;
+if (NamedDecl *from_named_decl = dyn_cast(from)) {
+  llvm::raw_string_ostream name_stream(name_string);
+  from_named_decl->printName(name_stream);
+  name_stream.flush();
+}
+log_ast->Printf(" [ClangASTImporter][TUDecl: %p] Imported "
+"(%sDecl*)%p, named %s (from "
+"(Decl*)%p)",
+static_cast(to->getTranslationUnitDecl()),
+from->getDeclKindName(), static_cast(to),
+name_string.c_str(), static_cast(from));
+
+// Log the AST of the TU.
+std::string ast_string;
+llvm::raw_string_ostream ast_stream(ast_string);
+to->getTranslationUnitDecl()->dump(ast_stream);
+log_ast->Printf("%s", ast_string.c_str());
+  }
 }
   }
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,18 +637,6 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
-
-  DeclContext *decl_context_non_const =
-  const_cast(decl_context);
-
-  if (copied_decl->getDeclContext() != decl_context) {
-if (copied_decl->getDeclContext()->containsDecl(copied_decl))
-  copied_decl->getDeclContext()->removeDecl(copied_decl);
-copied_decl->setDeclContext(decl_context_non_const);
-  }
-
-  if (!decl_context_non_const->containsDecl(copied_decl))
-decl_context_non_const->addDeclInternal(copied_decl);
 }
   }
 
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
@@ -5,11 +5,11 @@
 typedef struct {
 int a;
 int b;
-} FILE;
+} MYFILE;
 
 int main()
 {
-FILE *myFile = malloc(sizeof(FILE));
+MYFILE *myFile = malloc(sizeof(MYFILE));
 
 myFile->a = 5;
 myFile->b = 9;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
===

r359603 - [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Apr 30 12:35:14 2019
New Revision: 359603

URL: http://llvm.org/viewvc/llvm-project?rev=359603&view=rev
Log:
[Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

When compiler-rt is selected as the runtime library for Linux targets
use its crtbegin.o/crtend.o implemenetation rather than platform one
if available.

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

Added:
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o

cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-x86_64.o
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-i386.o
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-x86_64.o
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=359603&r1=359602&r2=359603&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Apr 30 12:35:14 2019
@@ -21,6 +21,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Path.h"
@@ -453,20 +454,29 @@ void tools::gnutools::Linker::ConstructJ
 
 if (IsIAMCU)
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
-else {
-  const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+else if (HasCRTBeginEndFiles) {
+  std::string P;
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {
+std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+   ToolChain::FT_Object);
+if (ToolChain.getVFS().exists(crtbegin))
+  P = crtbegin;
+  }
+  if (P.empty()) {
+const char *crtbegin;
+if (Args.hasArg(options::OPT_static))
+  crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
+else if (Args.hasArg(options::OPT_shared))
+  crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
+else if (IsPIE || IsStaticPIE)
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
+else
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
+P = ToolChain.GetFilePath(crtbegin);
+  }
+  CmdArgs.push_back(Args.MakeArgString(P));
+ }
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -560,16 +570,27 @@ void tools::gnutools::Linker::ConstructJ
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
-  const char *crtend;
-  if (Args.hasArg(options::OPT_shared))
-crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
-  else if (IsPIE || IsStaticPIE)
-crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
-  else
-crtend = isAndroid ? "crtend_android.o" : "crtend.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtend)));
+  if (HasCRTBeginEndFiles) {
+std::string P;
+if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+!isAndroid) {
+  std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+   ToolChain::FT_Object);
+  if (ToolChain.getVFS().exists(crtend))
+P = crtend;
+}
+if (P.empty()) {
+  const char *crtend;
+  if (Args.hasArg(options::OPT_shared))
+crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
+  else if (IsPIE || IsStaticPIE)
+crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
+  else
+crtend = isAndroid ? "crtend_android.o" : "crtend.o";
+  P = ToolChain.GetFilePath(crtend);
+}
+CmdArgs.push_back(Args.MakeArgString(P));
+  }
   if (!isAndroid)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
 }

Added: 
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o
URL: 
http://llvm.or

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c:8
 int b;
-} FILE;
+} MYFILE;
 

In TestCmodules.py we have `import Darwin` and then `expr *fopen("/dev/zero", 
"w")`. This imports the name `FILE` from the Darwin module. Then when we want 
`expr *myFile`, the two different definition of the `FILE` structs collides.
This happens because now the lookup finds the existing definition for the 
`FILE` in the TU associated with the expression evaluator, and that comes from 
the Darwin module.



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:640
   }
-
-  DeclContext *decl_context_non_const =

We must remove the below addDeclInternal call, because that may be called from 
another
addDeclInternal:
```
6  clang::CXXConstructorDecl::isDefaultConstructor() const + 87
7  clang::CXXRecordDecl::addedMember(clang::Decl*) + 803
8  clang::DeclContext::addHiddenDecl(clang::Decl*) + 341
9  clang::DeclContext::addDeclInternal(clang::Decl*) + 29
10 lldb_private::ClangASTSource::FindExternalLexicalDecls(clang::DeclContext 
const*, llvm::function_ref, 
llvm::SmallVectorImpl&) + 3917
11 
lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalLexicalDecls(clang::DeclContext
 const*, llvm::function_ref, 
llvm::SmallVectorImpl&) + 102
12 clang::ExternalASTSource::FindExternalLexicalDecls(clang::DeclContext 
const*, llvm::SmallVectorImpl&) + 101
13 clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const + 269
14 clang::DeclContext::buildLookup() + 336
15 clang::DeclContext::makeDeclVisibleInContextWithFlags(clang::NamedDecl*, 
bool, bool) + 403
16 clang::DeclContext::addDeclInternal(clang::Decl*) + 92
17 clang::ASTNodeImporter::VisitFieldDecl(clang::FieldDecl*) + 3851
```
... and at the second call of addDeclInternal we may add an incomplete Decl 
(CXXConstructorDecl above).
We must always avoid redundant work in lldb which is already handled in 
Clang::ASTImporter.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359603: [Driver] Support compiler-rt crtbegin.o/crtend.o for 
Linux (authored by phosek, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59264?vs=193733&id=197414#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59264

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-x86_64.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-i386.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-x86_64.o
  cfe/trunk/test/Driver/linux-ld.c

Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -21,6 +21,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Path.h"
@@ -453,20 +454,29 @@
 
 if (IsIAMCU)
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
-else {
-  const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+else if (HasCRTBeginEndFiles) {
+  std::string P;
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {
+std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+   ToolChain::FT_Object);
+if (ToolChain.getVFS().exists(crtbegin))
+  P = crtbegin;
+  }
+  if (P.empty()) {
+const char *crtbegin;
+if (Args.hasArg(options::OPT_static))
+  crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
+else if (Args.hasArg(options::OPT_shared))
+  crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
+else if (IsPIE || IsStaticPIE)
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
+else
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
+P = ToolChain.GetFilePath(crtbegin);
+  }
+  CmdArgs.push_back(Args.MakeArgString(P));
+	  }
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -560,16 +570,27 @@
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
-  const char *crtend;
-  if (Args.hasArg(options::OPT_shared))
-crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
-  else if (IsPIE || IsStaticPIE)
-crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
-  else
-crtend = isAndroid ? "crtend_android.o" : "crtend.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtend)));
+  if (HasCRTBeginEndFiles) {
+std::string P;
+if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+!isAndroid) {
+  std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+   ToolChain::FT_Object);
+  if (ToolChain.getVFS().exists(crtend))
+P = crtend;
+}
+if (P.empty()) {
+  const char *crtend;
+  if (Args.hasArg(options::OPT_shared))
+crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
+  else if (IsPIE || IsStaticPIE)
+crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
+  else
+crtend = isAndroid ? "crtend_android.o" : "crtend.o";
+  P = ToolChain.GetFilePath(crtend);
+}
+CmdArgs.push_back(Args.MakeArgString(P));
+  }
   if (!isAndroid)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
 }
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -2,7 +2,7 @@
 // sysroot to make these tests independent of the host system.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197415.
leonardchan marked 17 inline comments as done.
leonardchan removed a reviewer: martong.
Herald added a reviewer: martong.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Frontend/macro_defined_type.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaObjC/externally-retained.m
  clang/test/SemaObjC/gc-attributes.m
  clang/test/SemaObjC/mrc-weak.m
  clang/test/SemaObjCXX/gc-attributes.mm
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1614,6 +1614,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaObjCXX/gc-attributes.mm
===
--- clang/test/SemaObjCXX/gc-attributes.mm
+++ clang/test/SemaObjCXX/gc-attributes.mm
@@ -3,7 +3,7 @@
 @interface A
 @end
 
-void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
+void f0(__strong A **); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
 
 void test_f0() {
   A *a;
@@ -12,7 +12,7 @@
   f0(&a2); // expected-error{{no matching function}}
 }
 
-void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
+void f1(__weak A **); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
 
 void test_f1() {
   A *a;
Index: clang/test/SemaObjC/mrc-weak.m
===
--- clang/test/SemaObjC/mrc-weak.m
+++ clang/test/SemaObjC/mrc-weak.m
@@ -62,6 +62,6 @@
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id *)value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 
Index: clang/test/SemaObjC/gc-attributes.m
===
--- clang/test/SemaObjC/gc-attributes.m
+++ clang/test/SemaObjC/gc-attributes.m
@@ -9,7 +9,7 @@
   A *a;
   static __weak A *a2;
   f0(&a);
-  f0(&a2); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+  f0(&a2); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}}
 }
 
 void f1(__weak A**); // expected-note{{passing argument to parameter here}}
@@ -18,7 +18,7 @@
   A *a;
   __strong A *a2;
   f1(&a);
-  f1(&a2); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+  f1(&a2); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}}
 }
 
 // These qualifiers should silently expand to nothing in GC mode.
Index: clang/test/SemaObjC/externally-retained.m
===
--- clang/test/SemaObjC/externally-retained.m
+++ clang/test/SemaObjC/externally-retained.m
@@ -68,6 +68,12 @@
   second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
 };
 
+void (^blk2)(ObjCTy *, ObjCTy *) =
+^(__stron

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:257-262
+  bool AttrStartIsInMacro =
+  (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+   StartLoc, SrcMgr, PP.getLangOpts()));
+  bool AttrEndIsInMacro =
+  (EndLoc.isMacroID() &&
+   Lexer::isAtEndOfMacroExpansion(EndLoc, SrcMgr, PP.getLangOpts()));

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > I think these checks need to be moved to the last loop of 
> > > `FindLocsWithCommonFileID`. Otherwise for a case like:
> > > 
> > > ```
> > > #define THING \
> > >   int *p = nullptr;
> > >   AS1 int *q = nullptr;
> > > 
> > > THING
> > > ```
> > > 
> > > ... `FindLocsWithCommonFileID` will pick out the `THING` macro and return 
> > > the range from the `int` token to the second semicolon, and the checks 
> > > here will fail. Instead, we should pick out the inner `AS1` expansion, 
> > > because it's the outermost macro expansion that starts with `StartLoc` 
> > > and ends with `EndLoc`.
> > Moved, although this doesn't appear to fix this case. On closer inspection, 
> > when generating the vector of starting locations, the expansion location 
> > for `AS1` doesn't seem to be returned by `getExpansionLocStart`. It goes 
> > straight from the location of the `__attribute__` behind `AS1` to `THING` 
> > and skips `AS1`. I found this out by just dumping `StartLoc` on every 
> > iteration.
> > 
> > The only way I can generate the location for `AS1` in `THING` is by also 
> > watching for the spelling location of each expansion, but this SourceLoc is 
> > not a macro ID and would not fulfill these last 2 checks. Is this intended? 
> > If it's a bug I don't mind addressing it if you point me in the right 
> > direction to start.
> Right, sorry, mistake on my part. To deal with this, you need to call 
> `isAt{Start,End}OfMacroExpansion` while collecting `StartLocs` and `EndLocs`, 
> and stop once you hit a location where they return `false`. (That would mean 
> that you never step from the location of `AS1` out to the location of 
> `THING`.)
Hmm. So I still run into the problem of none of the locations being at the 
start or end of the macro expansion. In your example, I only find 2 source 
locations at all. Let's say I have:

```
1 #define AS1 __attribute__((address_space(1)))
2
3 int main() {
4 #define THING \
5   int *p = 0; \
6   AS1 int *q = p;
7
8   THING;
9 }
```

I only see the following expansion source locations:

```
/usr/local/google/home/leonardchan/misc/test.c:8:3 

/usr/local/google/home/leonardchan/misc/test.c:8:3 

```

And it seems none of them return true for `isAtStartOfMacroExpansion` since the 
`__attribute__` keyword isn't the first token of `THING`. I imagine stopping 
early would work if the 2nd expansion location instead referred to `AS1` (on 
line 6 col 3), but it doesn't seem to be the case here.

I can also see similar results for other examples where the macro is nested but 
does not encapsulate the whole macro:

```
#define THING2 int AS1 *q2 = p2;
int *p2;
THING2;  // Error doesn't print AS1
```

For our case, it seems like the correct thing to do is to get the spelling 
location and default to it if the macro itself doesn't contain the whole 
attribute. I updated and renamed this function to account for this and we can 
now see `AS1` printed. Also added test cases for this.

For cases like `#define AS2 AS1`, `AS1` is still printed, but this is intended 
since `AS1` is more immediate of an expansion than `AS2`.



Comment at: clang/lib/Sema/SemaType.cpp:6967-6972
+  QualType R = T.IgnoreParens().IgnoreMacroDefinitions();
   while (const AttributedType *AT = dyn_cast(R)) {
 if (AT->isCallingConv())
   return true;
-R = AT->getModifiedType().IgnoreParens();
+R = AT->getModifiedType().IgnoreParens().IgnoreMacroDefinitions();
   }

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > Instead of calling `IgnoreParens()` and `IgnoreMacroDefinitions` here, I 
> > > think we should just be ignoring any type sugar by using 
> > > `getAs()`:
> > > 
> > > ```
> > > bool Sema::hasExplicitCallingConv(QualType T) {
> > >   while (const auto *AT = T->getAs()) {
> > > if (AT->isCallingConv())
> > >   return true;
> > > T = AT->getModifiedType();
> > >   }
> > >   return false;
> > > }
> > > ```
> > > 
> > > (Note also the change from passing `T` by reference -- and then never 
> > > storing to it in the function -- to passing it by value.)
> > When using only `getAs`, the following 3 tests fail:
> > 
> > Clang :: CodeGenCXX/mangle-ms.cpp
> > Clang :: SemaCXX/calling-conv-compat.cpp
> > Clang :: SemaCXX/decl-microsoft-call-conv.cpp
> > 
> > since using `getAs` would involved removing a `TypeDefType` for each of 
> > these. Looking at the comment above this method's declaration, part of the 
> > intent is to retain the typede

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

phosek marked an inline comment as not done.
phosek added a subscriber: rsmith.
phosek added inline comments.



Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

jdenny wrote:
> phosek wrote:
> > jdenny wrote:
> > > I naively assumed libunwind would have its own directory so it could be 
> > > selected independently.  Does this mean any library for C++ goes in c++?
> > We could do that, but I'm not sure if it's worth doing without a clear use 
> > case? Do you know of any client that would want to consume libunwind 
> > independently of other C++ libraries? I'm aware of Rust but they build 
> > libunwind independently so there's no need for this.
> I'm afraid I have no idea.  I'm not close enough to these libraries to be an 
> adequate reviewer by myself.  Another reviewer should probably take a look at 
> the new version of the patch.
@EricWF @rsmith is this something you have an opinion on (or can you recommend 
someone who might)?


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

https://reviews.llvm.org/D59168



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

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think I figured it out. r359604 should fix the bug that caused this to be 
reverted twice.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197416.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Frontend/macro_defined_type.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaObjC/externally-retained.m
  clang/test/SemaObjC/gc-attributes.m
  clang/test/SemaObjC/mrc-weak.m
  clang/test/SemaObjCXX/gc-attributes.mm
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1614,6 +1614,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaObjCXX/gc-attributes.mm
===
--- clang/test/SemaObjCXX/gc-attributes.mm
+++ clang/test/SemaObjCXX/gc-attributes.mm
@@ -3,7 +3,7 @@
 @interface A
 @end
 
-void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
+void f0(__strong A **); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
 
 void test_f0() {
   A *a;
@@ -12,7 +12,7 @@
   f0(&a2); // expected-error{{no matching function}}
 }
 
-void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
+void f1(__weak A **); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
 
 void test_f1() {
   A *a;
Index: clang/test/SemaObjC/mrc-weak.m
===
--- clang/test/SemaObjC/mrc-weak.m
+++ clang/test/SemaObjC/mrc-weak.m
@@ -62,6 +62,6 @@
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id *)value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 
Index: clang/test/SemaObjC/gc-attributes.m
===
--- clang/test/SemaObjC/gc-attributes.m
+++ clang/test/SemaObjC/gc-attributes.m
@@ -9,7 +9,7 @@
   A *a;
   static __weak A *a2;
   f0(&a);
-  f0(&a2); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+  f0(&a2); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}}
 }
 
 void f1(__weak A**); // expected-note{{passing argument to parameter here}}
@@ -18,7 +18,7 @@
   A *a;
   __strong A *a2;
   f1(&a);
-  f1(&a2); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+  f1(&a2); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}}
 }
 
 // These qualifiers should silently expand to nothing in GC mode.
Index: clang/test/SemaObjC/externally-retained.m
===
--- clang/test/SemaObjC/externally-retained.m
+++ clang/test/SemaObjC/externally-retained.m
@@ -68,6 +68,12 @@
   second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
 };
 
+void (^blk2)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, ObjCTy *second) __attribute__((objc_externally_retained)) {
+  first = 0;
+  second = 0; // expected-erro

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
Herald added a project: clang.

This revision adds a new kind of rewrite rule, `CompositeRewriteRule`, which
composes multiple subrules into a new rule that allows ordered-choice among its
subrules. With this feature, users can write the rules that appear later in the
list of subrules knowing that previous rules' patterns *have not matched*,
freeing them from reasoning about those cases in the current pattern.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61335

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -116,7 +116,8 @@
 };
   }
 
-  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+  template 
+  void testRule(R Rule, StringRef Input, StringRef Expected) {
 Transformer T(std::move(Rule), consumer());
 T.registerMatchers(&MatchFinder);
 compareSnippets(Expected, rewrite(Input));
@@ -375,6 +376,92 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, OrderedRuleUnrelated) {
+  StringRef Flag = "flag";
+  RewriteRule FlagRule = makeRule(
+  cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
+hasName("proto::ProtoCommandLineFlag"
+   .bind(Flag)),
+unless(callee(cxxMethodDecl(hasName("GetProto"),
+  change(Flag, "PROTO"));
+
+  std::string Input = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = flag.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = PROTO.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return REPLACED; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSize(), FlagRule}), Input, Expected);
+}
+
+// Version of ruleStrlenSizeAny that inserts a method with a different name than
+// ruleStrlenSize, so we can tell their effect apart.
+RewriteRule ruleStrlenSizeDistinct() {
+  StringRef S;
+  return makeRule(
+  callExpr(callee(functionDecl(hasName("strlen"))),
+   hasArgument(0, cxxMemberCallExpr(
+  on(expr().bind(S)),
+  callee(cxxMethodDecl(hasName("c_str")),
+  change("DISTINCT"));
+}
+
+TEST_F(TransformerTest, OrderedRuleRelated) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return REPLACED; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSize(), ruleStrlenSizeDistinct()}), Input,
+   Expected);
+}
+
+// Change the order of the rules to get a different result.
+TEST_F(TransformerTest, OrderedRuleRelatedSwapped) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return DISTINCT; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSizeDistinct(), ruleStrlenSize()}), Input,
+   Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -28,6 +28,7 @@
 using namespace tooling;
 
 using ast_matchers::MatchFinder;
+using ast_matchers::internal::DynTypedMatcher;
 using ast_type_traits::ASTNodeKind;
 using ast_type_traits::DynTypedNode;
 using llvm::Error;
@@ -171,7 +172,7 @@
   return Transformations;
 }
 
-RewriteRule tooling::makeRule(ast_matchers::internal::DynTypedMatcher M,
+RewriteRule tooling::makeRule(DynTypedMatcher M,
   SmallVector Edits) {
   M.setAllowBind(true);
   // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
@@ -181,6 +182,80 @@
 
 constexpr llvm::StringLiteral RewriteRule::RootId;
 
+// Determines whether A is higher than B in the class hierarchy.
+static bool isHigher(ASTNo

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D38061#1484568 , @wenlei wrote:

> In D38061#1484486 , @dblaikie wrote:
>
> > Seems like the right thing would be for the DWARF code that wants a 
> > rendered type name to pass its own printing policy, rather than changing 
> > some relatively global one.
> >
> > (though also I have my doubts about the whole approach - macro expansion 
> > can change the line number as well as the column number, so only 
> > suppressing column number would be insufficient - and this also reduces the 
> > usability for users (because the file/line/column of an anonymous type is a 
> > useful debugging aid). Seems to me like this should be opt-in if it's 
> > supported at all - though ideally build systems would use 
> > -frewrite-includes rather than full preprocessing, and then macros and 
> > lines/columns would be preserved, I think)
>
>
> Line number is ok because preprocessor also inserts #linemarker, and later on 
> these markers are used to recover the original line number before macro 
> expansion. Column is problematic, as there's nothing like a column marker (if 
> possible at all) so there's no way to see through the column change from 
> macro expansion. We tried -frewrite-includes too, but it has its own issues - 
> without macro expansion, it bloats the file size that needs to send to distcc 
> remote machine significantly.


Do you have some data on that growth/size comparison & its overall performance 
impact on the build? I'd be curious.

But, yeah, fair point about the line markers - I'd thought a multiline macro 
would break that, but seems not (it gets stamped out by the preprocessor all on 
one line anyway).

Still - figure this should be opt-in and done at the debug info level, not by 
changing a global printing policy. (& I'd still suggest using 
-frewrite-includes because Clang changes its diagnostic behavior based on the 
presence of macros to reduce diagnostic false positives, so you'll be degrading 
the usability in other ways by compiling fully preprocessed code)


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 197421.
vsapsai added a comment.

- Add a test case with `-disable-free`.

Thanks for the suggestion, Erik.


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

https://reviews.llvm.org/D60848

Files:
  clang/lib/Parse/ParseExpr.cpp
  clang/test/SemaCXX/typo-correction.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- /dev/null
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+
+@class Dictionary;
+
+@interface Test
+@end
+@implementation Test
+// rdar://problem/47403222
+- (void)rdar47403222:(Dictionary *)opts {
+  [self undeclaredMethod:undeclaredArg];
+  // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  opts[(__bridge id)undeclaredKey] = 0;
+  // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
+}
+@end
Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -678,7 +678,7 @@
 struct a0is0 {};
 struct b0is0 {};
 int g() {
-  0 [ // expected-error {{subscripted value is not an array}}
+  0 [
   sizeof(c0is0)]; // expected-error {{use of undeclared identifier}}
 };
 }
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -1582,7 +1582,9 @@
 
   SourceLocation RLoc = Tok.getLocation();
 
-  ExprResult OrigLHS = LHS;
+  LHS = Actions.CorrectDelayedTyposInExpr(LHS);
+  Idx = Actions.CorrectDelayedTyposInExpr(Idx);
+  Length = Actions.CorrectDelayedTyposInExpr(Length);
   if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
   Tok.is(tok::r_square)) {
 if (ColonLoc.isValid()) {
@@ -1594,12 +1596,6 @@
 }
   } else {
 LHS = ExprError();
-  }
-  if (LHS.isInvalid()) {
-(void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
-(void)Actions.CorrectDelayedTyposInExpr(Idx);
-(void)Actions.CorrectDelayedTyposInExpr(Length);
-LHS = ExprError();
 Idx = ExprError();
   }
 


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- /dev/null
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+
+@class Dictionary;
+
+@interface Test
+@end
+@implementation Test
+// rdar://problem/47403222
+- (void)rdar47403222:(Dictionary *)opts {
+  [self undeclaredMethod:undeclaredArg];
+  // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  opts[(__bridge id)undeclaredKey] = 0;
+  // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
+}
+@end
Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -678,7 +678,7 @@
 struct a0is0 {};
 struct b0is0 {};
 int g() {
-  0 [ // expected-error {{subscripted value is not an array}}
+  0 [
   sizeof(c0is0)]; // expected-error {{use of undeclared identifier}}
 };
 }
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -1582,7 +1582,9 @@
 
   SourceLocation RLoc = Tok.getLocation();
 
-  ExprResult OrigLHS = LHS;
+  LHS = Actions.CorrectDelayedTyposInExpr(LHS);
+  Idx = Actions.CorrectDelayedTyposInExpr(Idx);
+  Length = Actions.CorrectDelayedTyposInExpr(Length);
   if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
   Tok.is(tok::r_square)) {
 if (ColonLoc.isValid()) {
@@ -1594,12 +1596,6 @@
 }
   } else {
 LHS = ExprError();
-  }
-  if (LHS.isInvalid()) {
-(void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
-(void)Actions.CorrectDelayedTyposInExpr(Idx);
-(void)Actions.CorrectDelayedTyposInExpr(Length);
-LHS = ExprError();
 Idx = ExprError();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

IIRC, C says *members* are initialized as if they were in static storage, which 
might mean that their interior padding should be zeroed, but I don't think says 
anything about the padding in the enclosing aggregate.  But I think 
zero-initializing padding is probably the right thing to do in general under 
this flag.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM, I think this makes sense.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: dschuff, sbc100, aheejin.
Herald added a subscriber: jgravelle-google.
Herald added a project: clang.

Similar to https://reviews.llvm.org/D61334, update clang tests to use the 
"wasm32-wasi" triple, removing the "-musl" environment and omitting the 
"-unknown" vendor.


Repository:
  rC Clang

https://reviews.llvm.org/D61338

Files:
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9577,10 +9577,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-unknown \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY32,WEBASSEMBLY-WASI %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
 //
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -24,17 +24,17 @@
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi-musl/c++/v1" "-internal-isystem" 
"/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi-musl" 
"-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" 
"-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" 
"/foo/include"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -24,20 +24,20 @@
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %cl

[PATCH] D60930: [codeview] Fix symbol names for dynamic initializers and atexit stubs

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In Swift we basically shove *everything* through our corresponding 
linkage-entity structure, so I'm not opposed to the basic idea.

That said, it's unfortunate that this needs to be raised to the AST level.  
Also, you have really been contributing to this project far too long to still 
need to be told to add comments to the new types and functions you're adding. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60930



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D61280#1485073 , @rjmccall wrote:

> IIRC, C says *members* are initialized as if they were in static storage, 
> which might mean that their interior padding should be zeroed, but I don't 
> think says anything about the padding in the enclosing aggregate.  But I 
> think zero-initializing padding is probably the right thing to do in general 
> under this flag.


Quoth the Standard:

C17 6.7.9 Initialization ❡21
If there are fewer initializers in a brace-enclosed list than there are 
elements or members of an aggregate, or fewer characters in a string literal 
used to initialize an array of known size than there are elements in the array, 
the remainder of the aggregate shall be initialized implicitly the same as 
objects that have static storage duration.

C17 6.7.9 Initialization ❡10
If an object that has automatic storage duration is not initialized explicitly, 
its value is indeterminate. If an object that has static or thread storage 
duration is not initialized explicitly, then:

- if it has pointer type, it is initialized to a null pointer;
- if it has arithmetic type, it is initialized to (positive or unsigned) zero;
- if it is an aggregate, every member is initialized (recursively) according to 
these rules, and and padding is initialized to zero bits;
- if it is a union, the first named member is initialized (recessively) 
according to these rules, and any padding is initialized to zero bits;

🙂


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think the implication is supposed to be that padding is 
zero-initialized or not depending on where in the aggregate it appears, but it 
doesn't really matter, I don't think we're arguing about the goal of this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61304: [OpenCL][PR41609] Deduce static data members to __global addr space

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Presumably a similar rule would apply to thread-locals if you supported them.


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

https://reviews.llvm.org/D61304



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D61280#1485115 , @rjmccall wrote:

> I don't think the implication is supposed to be that padding is 
> zero-initialized or not depending on where in the aggregate it appears, but 
> it doesn't really matter, I don't think we're arguing about the goal of this 
> patch.


The way I read it, C guarantees that an aggregate's padding is zero-initialized 
when you use brace-enclosed list *unless* you've mentioned all the elements / 
members of the aggregate. Fail to mention one → padding must be zero. Mention 
all of them → padding isn't guaranteed.

But then *reading* padding is another story. What triggered this change is C 
code that used `memcmp` on a struct with padding, and was expecting padding to 
be set. Not great code, but AFAICT technically correct.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  So it sounds like this should either be a device-only rule, with no 
warning in mixed-mode languages like CUDA, or we should take a different 
approach.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61274



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

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

rsmith added a comment.

I'd like to understand more about the intended use cases of this functionality. 
What information do clients want?




Comment at: clang/lib/Lex/Preprocessor.cpp:870-900
+  TokenSource Source;
   do {
+Source = TokenSource();
+
 switch (CurLexerKind) {
 case CLK_Lexer:
+  Source.InDirective = CurLexer->ParsingPreprocessorDirective;

This is a lot of extra stuff to be doing in the main `Lex` loop. Adding one 
(perfectly-predicted) branch on `OnToken` seems like it should be fine, but 
this seems like a bit more added complexity than I'd prefer. I'd like some 
performance measurements of `-cc1 -Eonly` to see if this makes any real 
difference.



Comment at: clang/lib/Lex/Preprocessor.cpp:896
 case CLK_LexAfterModuleImport:
-  ReturnedToken = LexAfterModuleImport(Result);
+  Source.InDirective = true;
+

This isn't a directive; these are normal tokens.



Comment at: clang/lib/Lex/Preprocessor.cpp:956-957
   --LexLevel;
+  if (OnToken)
+OnToken(Result, Source);
 }

This seems like it's going to be extremely hard to use. If you just want the 
expanded token stream, then removing all the other calls to `OnToken` and only 
calling it here when `LexLevel == 0` will give you that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885



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

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

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

I'm not sure what the IsSizeGreaterThan128 check is doing here - if the return 
type is over 128 bits, then it will be indirectly returned in X8 with this 
check, which is not always what we want (e.g. in 
https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the value 
returned in X1). Another check of hasMicrosoftABIRestrictions might be OK, but 
that's also not quite right due to the size check. 


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

https://reviews.llvm.org/D60349



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

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

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 197436.

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

https://reviews.llvm.org/D60349

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

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

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

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

rsmith added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:257-262
+  bool AttrStartIsInMacro =
+  (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+   StartLoc, SrcMgr, PP.getLangOpts()));
+  bool AttrEndIsInMacro =
+  (EndLoc.isMacroID() &&
+   Lexer::isAtEndOfMacroExpansion(EndLoc, SrcMgr, PP.getLangOpts()));

leonardchan wrote:
> rsmith wrote:
> > leonardchan wrote:
> > > rsmith wrote:
> > > > I think these checks need to be moved to the last loop of 
> > > > `FindLocsWithCommonFileID`. Otherwise for a case like:
> > > > 
> > > > ```
> > > > #define THING \
> > > >   int *p = nullptr;
> > > >   AS1 int *q = nullptr;
> > > > 
> > > > THING
> > > > ```
> > > > 
> > > > ... `FindLocsWithCommonFileID` will pick out the `THING` macro and 
> > > > return the range from the `int` token to the second semicolon, and the 
> > > > checks here will fail. Instead, we should pick out the inner `AS1` 
> > > > expansion, because it's the outermost macro expansion that starts with 
> > > > `StartLoc` and ends with `EndLoc`.
> > > Moved, although this doesn't appear to fix this case. On closer 
> > > inspection, when generating the vector of starting locations, the 
> > > expansion location for `AS1` doesn't seem to be returned by 
> > > `getExpansionLocStart`. It goes straight from the location of the 
> > > `__attribute__` behind `AS1` to `THING` and skips `AS1`. I found this out 
> > > by just dumping `StartLoc` on every iteration.
> > > 
> > > The only way I can generate the location for `AS1` in `THING` is by also 
> > > watching for the spelling location of each expansion, but this SourceLoc 
> > > is not a macro ID and would not fulfill these last 2 checks. Is this 
> > > intended? If it's a bug I don't mind addressing it if you point me in the 
> > > right direction to start.
> > Right, sorry, mistake on my part. To deal with this, you need to call 
> > `isAt{Start,End}OfMacroExpansion` while collecting `StartLocs` and 
> > `EndLocs`, and stop once you hit a location where they return `false`. 
> > (That would mean that you never step from the location of `AS1` out to the 
> > location of `THING`.)
> Hmm. So I still run into the problem of none of the locations being at the 
> start or end of the macro expansion. In your example, I only find 2 source 
> locations at all. Let's say I have:
> 
> ```
> 1 #define AS1 __attribute__((address_space(1)))
> 2
> 3 int main() {
> 4 #define THING \
> 5   int *p = 0; \
> 6   AS1 int *q = p;
> 7
> 8   THING;
> 9 }
> ```
> 
> I only see the following expansion source locations:
> 
> ```
> /usr/local/google/home/leonardchan/misc/test.c:8:3 
> 
> /usr/local/google/home/leonardchan/misc/test.c:8:3 
> 
> ```
> 
> And it seems none of them return true for `isAtStartOfMacroExpansion` since 
> the `__attribute__` keyword isn't the first token of `THING`. I imagine 
> stopping early would work if the 2nd expansion location instead referred to 
> `AS1` (on line 6 col 3), but it doesn't seem to be the case here.
> 
> I can also see similar results for other examples where the macro is nested 
> but does not encapsulate the whole macro:
> 
> ```
> #define THING2 int AS1 *q2 = p2;
> int *p2;
> THING2;  // Error doesn't print AS1
> ```
> 
> For our case, it seems like the correct thing to do is to get the spelling 
> location and default to it if the macro itself doesn't contain the whole 
> attribute. I updated and renamed this function to account for this and we can 
> now see `AS1` printed. Also added test cases for this.
> 
> For cases like `#define AS2 AS1`, `AS1` is still printed, but this is 
> intended since `AS1` is more immediate of an expansion than `AS2`.
I'd like to get some (correct) form of this landed, so I wonder if we can do 
something much simpler for now. Can we just check that the first token of the 
attribute-specifier `isAtStartOfMacroExpansion` and the last token 
`isAtEndOfMacroExpansion` and that they're from the same expansion of the same 
(object-like) macro? That won't find the outermost such macro, nor will it deal 
with cases where `__attribute__` or the last `)` was itself generated by a 
macro, but it should not have any false positives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329



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