[PATCH] D147846: [clangd] Hover: resolve forwarding parameters for CalleeArgInfo

[Nathan Ridge via Phabricator via cfe-commits]

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

Makes sense, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147846

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

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:986
+  const FunctionDecl *FD = nullptr;
+  llvm::SmallVector Args;
+

tom-anders wrote:
> Unfortunately, while CallExpr and CXXConstructExpr basically have the same 
> API for getting Args, they're not related by a common base class.
> 
> Is there a more elegant solution than temporarily storing the Args in a 
> SmallVector here?
You can use `ArrayRef` instead of `SmallVector` and avoid copying the arguments 
(compare 
[process_call](https://searchfox.org/llvm/rev/b34ca0851a5209a10c0ca285c000a18073677891/clang-tools-extra/clangd/InlayHints.cpp#239,265)
 in `InlayHintVisitor`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147847

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:1220
+/// \endcode
+bool AfterCSharpProperty;
   };

HazardyKnusperkeks wrote:
> Please sort. :)
Are we sure we want THIS to be alphabetic, as this changes the initializer 
order, if someone is using the format() library in downstream code this could 
subtly break them?

```
Expanded.BraceWrapping = {/*AfterCaseLabel=*/false,
/*AfterClass=*/false,
/*AfterControlStatement=*/FormatStyle::BWACS_Never,
/*AfterEnum=*/false,
/*AfterFunction=*/false,
/*AfterNamespace=*/false,
/*AfterObjCDeclaration=*/false,
/*AfterStruct=*/false,
/*AfterUnion=*/false,
/*AfterExternBlock=*/false,
/*BeforeCatch=*/false,
/*BeforeElse=*/false,
/*BeforeLambdaBody=*/false,
/*BeforeWhile=*/false,
/*IndentBraces=*/false,
/*SplitEmptyFunction=*/true,
/*SplitEmptyRecord=*/true,
/*SplitEmptyNamespace=*/true,
/*AfterCSharpProperty=*/false};
```


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

https://reviews.llvm.org/D148467

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

[PATCH] D148318: [clang-tidy] Add `performance-avoid-endl` check

[André Schackier via Phabricator via cfe-commits]

AMS21 updated this revision to Diff 514118.
AMS21 added a comment.

Remove std::ends from the check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148318

Files:
  clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.cpp
  clang-tools-extra/clang-tidy/performance/AvoidEndlCheck.h
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance/avoid-endl.rst
  clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp
@@ -0,0 +1,157 @@
+// RUN: %check_clang_tidy %s performance-avoid-endl %t
+
+namespace std {
+  template 
+  class basic_ostream {
+public:
+template 
+basic_ostream& operator<<(T);
+basic_ostream& operator<<(basic_ostream& (*)(basic_ostream&));
+  };
+
+  template 
+  class basic_iostream : public basic_ostream {};
+
+  using ostream = basic_ostream;
+  using wostream = basic_ostream;
+
+  using iostream = basic_iostream;
+  using wiostream = basic_iostream;
+
+  ostream cout;
+  wostream wcout;
+
+  ostream cerr;
+  wostream wcerr;
+
+  ostream clog;
+  wostream wclog;
+
+  template
+  basic_ostream& endl(basic_ostream&);
+} // namespace std
+
+void good() {
+  std::cout << "Hello" << '\n';
+  std::cout << "World\n";
+
+  std::wcout << "Hello" << '\n';
+  std::wcout << "World\n";
+
+  std::cerr << "Hello" << '\n';
+  std::cerr << "World\n";
+
+  std::wcerr << "Hello" << '\n';
+  std::wcerr << "World\n";
+
+  std::clog << "Hello" << '\n';
+  std::clog << "World\n";
+
+  std::wclog << "Hello" << '\n';
+  std::wclog << "World\n";
+}
+
+void bad() {
+  std::cout << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::cout << "World" << '\n';
+  std::wcout << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wcout << "World" << '\n';
+  std::cerr << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::cerr << "World" << '\n';
+  std::wcerr << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wcerr << "World" << '\n';
+  std::clog << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::clog << "World" << '\n';
+  std::wclog << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wclog << "World" << '\n';
+}
+
+void bad_single_argument() {
+  std::cout << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::cout << '\n';
+  std::wcout << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wcout << '\n';
+  std::cerr << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::cerr << '\n';
+  std::wcerr << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wcerr << '\n';
+  std::clog << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::clog << '\n';
+  std::wclog << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::wclog << '\n';
+}
+
+void bad_multiple() {
+  std::cout << "Hello" << std::endl << "World" << std::endl;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-MESSAGES: :[[@LINE-2]]:51: warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]
+  // CHECK-FIXES: std::cout << "Hello" << '\n' << "World" << '\n';
+  std::wcout << "Hello" << std::endl << "World" << std:

[PATCH] D148318: [clang-tidy] Add `performance-avoid-endl` check

[André Schackier via Phabricator via cfe-commits]

AMS21 added a comment.

I've also notices that we don't handle this case

  std::endl(std::cout);

Although a rather unusual thing to use, its still valid and has the same 
problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148318

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

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Ping (:


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

https://reviews.llvm.org/D140722

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

[clang] 82fdd5b - [clang][NFC] Make parameters to NoteOverloadCandidate const

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-04-17T09:27:51+02:00
New Revision: 82fdd5b5123ee8528267a5bed1c443a30f3f93d7

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

LOG: [clang][NFC] Make parameters to NoteOverloadCandidate const

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 33d6c40f157fe..c12660081e7c4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4076,7 +4076,7 @@ class Sema final {
 
   // Emit as a 'note' the specific overload candidate
   void NoteOverloadCandidate(
-  NamedDecl *Found, FunctionDecl *Fn,
+  const NamedDecl *Found, const FunctionDecl *Fn,
   OverloadCandidateRewriteKind RewriteKind = 
OverloadCandidateRewriteKind(),
   QualType DestType = QualType(), bool TakingAddress = false);
 

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index ab4300518ecf1..5a8544e3739fe 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10385,7 +10385,8 @@ enum OverloadCandidateSelect {
 };
 
 static std::pair
-ClassifyOverloadCandidate(Sema &S, NamedDecl *Found, FunctionDecl *Fn,
+ClassifyOverloadCandidate(Sema &S, const NamedDecl *Found,
+  const FunctionDecl *Fn,
   OverloadCandidateRewriteKind CRK,
   std::string &Description) {
 
@@ -10409,7 +10410,7 @@ ClassifyOverloadCandidate(Sema &S, NamedDecl *Found, 
FunctionDecl *Fn,
 if (CRK & CRK_Reversed)
   return oc_reversed_binary_operator;
 
-if (CXXConstructorDecl *Ctor = dyn_cast(Fn)) {
+if (const auto *Ctor = dyn_cast(Fn)) {
   if (!Ctor->isImplicit()) {
 if (isa(Found))
   return oc_inherited_constructor;
@@ -10428,7 +10429,7 @@ ClassifyOverloadCandidate(Sema &S, NamedDecl *Found, 
FunctionDecl *Fn,
   return oc_implicit_copy_constructor;
 }
 
-if (CXXMethodDecl *Meth = dyn_cast(Fn)) {
+if (const auto *Meth = dyn_cast(Fn)) {
   // This actually gets spelled 'candidate function' for now, but
   // it doesn't hurt to split it out.
   if (!Meth->isImplicit())
@@ -10450,10 +10451,10 @@ ClassifyOverloadCandidate(Sema &S, NamedDecl *Found, 
FunctionDecl *Fn,
   return std::make_pair(Kind, Select);
 }
 
-void MaybeEmitInheritedConstructorNote(Sema &S, Decl *FoundDecl) {
+void MaybeEmitInheritedConstructorNote(Sema &S, const Decl *FoundDecl) {
   // FIXME: It'd be nice to only emit a note once per using-decl per overload
   // set.
-  if (auto *Shadow = dyn_cast(FoundDecl))
+  if (const auto *Shadow = dyn_cast(FoundDecl))
 S.Diag(FoundDecl->getLocation(),
diag::note_ovl_candidate_inherited_constructor)
   << Shadow->getNominatedBaseClass();
@@ -10560,7 +10561,7 @@ bool Sema::checkAddressOfFunctionIsAvailable(const 
FunctionDecl *Function,
 
 // Don't print candidates other than the one that matches the calling
 // convention of the call operator, since that is guaranteed to exist.
-static bool shouldSkipNotingLambdaConversionDecl(FunctionDecl *Fn) {
+static bool shouldSkipNotingLambdaConversionDecl(const FunctionDecl *Fn) {
   const auto *ConvD = dyn_cast(Fn);
 
   if (!ConvD)
@@ -10580,7 +10581,7 @@ static bool 
shouldSkipNotingLambdaConversionDecl(FunctionDecl *Fn) {
 }
 
 // Notes the location of an overload candidate.
-void Sema::NoteOverloadCandidate(NamedDecl *Found, FunctionDecl *Fn,
+void Sema::NoteOverloadCandidate(const NamedDecl *Found, const FunctionDecl 
*Fn,
  OverloadCandidateRewriteKind RewriteKind,
  QualType DestType, bool TakingAddress) {
   if (TakingAddress && !checkAddressOfCandidateIsAvailable(*this, Fn))



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

[PATCH] D148484: [clang-format] Correctly format goto labels followed by blocks

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4525
-const FormatToken *Next = Right.getNextNonComment();
-if (!Next || Next->is(tok::semi))
   return false;

how is the semi case handled or is it not needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148484

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:5007-5010
+   (Left.is(tok::semi) && Left.Previous &&
+Left.Previous->is(Keywords.kw_get) && Right.is(tok::r_brace)) ||
+   (Left.is(tok::semi) && Left.Previous &&
+Left.Previous->is(Keywords.kw_set) && Right.is(tok::r_brace {





Comment at: clang/unittests/Format/FormatTestCSharp.cpp:1614
+   "string Foo { set; get; }\n"
+   "}\n",
+   Style);

Ditto below.


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

https://reviews.llvm.org/D148467

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

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.

I know it might not seem an obvious use case but there really isn't a 
requirement to not include header files more than once.. imaging if I have

  #define ARCH "win32"
  #include "MyDataStructThatContainsPlaformSpecificNamesUsingARCH.h"
  #define ARCH "win64"
  #include "MyDataStructThatContainsPlaformSpecificNamesUsingARCH.h"

Its not nice, but just because I include it twice doesn't mean its wrong? This 
change would break code written that way.




Comment at: clang/unittests/Format/SortIncludesTest.cpp:927
 
-TEST_F(SortIncludesTest, DeduplicateLocallyInEachBlock) {
   EXPECT_EQ("#include \n"

Please don't change existing tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143870

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

[PATCH] D132256: [clang-format] Add DefinitionBlockSpacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.
Herald added a reviewer: rymiel.

Cloud you include a test that contains multiple levels of nested scope, I'm 
assuming we won't add an additonal line at every {} level (or will we?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132256

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

[clang] 455c9ef - [clang][NFC] Use range-for loop in SemaLookup.cpp

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-04-17T09:49:04+02:00
New Revision: 455c9efb41de101eaf5e3d4d521097428b5c75d3

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

LOG: [clang][NFC] Use range-for loop in SemaLookup.cpp

Added: 


Modified: 
clang/lib/Sema/SemaLookup.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 2413f31744c5..a495d3ca1989 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3761,8 +3761,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
   // operator template, but not both.
   if (FoundRaw && FoundTemplate) {
 Diag(R.getNameLoc(), diag::err_ovl_ambiguous_call) << R.getLookupName();
-for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I)
-  NoteOverloadCandidate(*I, (*I)->getUnderlyingDecl()->getAsFunction());
+for (const NamedDecl *D : R)
+  NoteOverloadCandidate(D, D->getUnderlyingDecl()->getAsFunction());
 return LOLR_Error;
   }
 



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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: erichkeane.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
sberg requested review of this revision.
Herald added a project: clang.

This takes inspiration from the standard `nodiscard` attribute, which can also 
be declared on individual constructors.  (I have used this change locally for 
about a year on the LibreOffice code base, marking various compilers as 
`warn_unused`, without any positive or negative effects.  Until I happened to 
notice an additional constructor that would benefit from `warn_unused`, and 
which found about 20 unused variables across the LibreOffice code base, see 
e.g., 
https://git.libreoffice.org/core/+/7cdbe504ff3b59d3aec1b1e86caf7f24223dce72%5E! 
"Fix what looks like copy/paste typos".)

(The changes in `clang/test/Misc/pragma-attribute-strict-subjects.c`, 
`clang/test/Parser/pragma-attribute.cpp`, and 
`clang/test/Sema/pragma-attribute-strict-subjects.c` are necessary to make 
those tests not fail after adding `SubRuleForCXXConstructor` to 
`SubjectMatcherForFunction` in `clang/include/clang/Basic/Attr.td`.  It appears 
that the exact diagnostic output that is generated is somewhat brittle, 
depending on implementation details.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148505

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DeclNodes.td
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Misc/pragma-attribute-strict-subjects.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Parser/pragma-attribute.cpp
  clang/test/Sema/pragma-attribute-strict-subjects.c
  clang/test/SemaCXX/warn-unused-attribute.cpp

Index: clang/test/SemaCXX/warn-unused-attribute.cpp
===
--- clang/test/SemaCXX/warn-unused-attribute.cpp
+++ clang/test/SemaCXX/warn-unused-attribute.cpp
@@ -9,12 +9,21 @@
   TestNormal();
 };
 
+struct TestCtor {
+  __attribute__((warn_unused)) TestCtor();
+  TestCtor(int);
+  void __attribute__((warn_unused)) f(); // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
+};
+
 int main(void) {
   Test unused; // expected-warning {{unused variable 'unused'}}
   Test used;
   TestNormal normal;
   used.use();
 
-  int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, and classes}}
+  TestCtor ctorUnused; // expected-warning {{unused variable 'ctorUnused'}}
+  TestCtor ctorUsed(0);
+
+  int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
   return i;
 }
Index: clang/test/Sema/pragma-attribute-strict-subjects.c
===
--- clang/test/Sema/pragma-attribute-strict-subjects.c
+++ clang/test/Sema/pragma-attribute-strict-subjects.c
@@ -28,7 +28,7 @@
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((annotate("negatedSubRuleContradictions2"))), apply_to = any(variable(unless(is_parameter)), variable(is_thread_local), function, variable(is_global)))
-// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_global)'}}
+// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_thread_local)'}}
 // We have just one error, don't error on 'variable(is_global)'
 
 #pragma clang attribute pop
Index: clang/test/Parser/pragma-attribute.cpp
===
--- clang/test/Parser/pragma-attribute.cpp
+++ clang/test/Parser/pragma-attribute.cpp
@@ -190,7 +190,7 @@
 #pragma clang attribute pop
 #pragma clang attribute push([[clang::uninitialized]], apply_to = variable(is_global)) // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_global)'}}
 #pragma clang attribute pop
-#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_parameter)', and 'variable(unless(is_parameter))'}}
+#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter // expected-error {{attribute 'uninitialized' can't be applied to 'variable(unless(is_parameter))', and 'variable(is_parameter)'}}
 #pragma clang attribute pop
 // We're allowed to apply attributes to subsets of allowed subjects.
 #pragma clang attribute push([[clang::no_destroy]], apply_to = variable)
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test

[PATCH] D148506: [C++] Don't filter using declaration when we perform qualified look up

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, erichkeane, aaron.ballman, cor3ntin, 
clang-language-wg.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Close https://github.com/llvm/llvm-project/issues/62174

And this was originally a try to close 
https://github.com/llvm/llvm-project/issues/62158.

I don't feel this is the **correct** fix. I just think it is not bad as an 
ad-hoc patch. And let's discuss things in the higher-level in the above GitHub 
issue link.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148506

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/special/class.inhctor/p8.cpp
  clang/test/Modules/pr62158.cppm
  clang/test/SemaCXX/pr62174.cpp

Index: clang/test/SemaCXX/pr62174.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pr62174.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+namespace lib {
+namespace impl {
+template 
+inline constexpr bool test = false;
+}
+using impl::test;
+}
+
+struct foo {};
+
+template <>
+inline constexpr bool lib::test = true;
+
+static_assert(lib::test);
Index: clang/test/Modules/pr62158.cppm
===
--- /dev/null
+++ clang/test/Modules/pr62158.cppm
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/lib.cppm -o %t/lib.pcm
+// RUN: %clang_cc1 -std=c++20 %t/main.cpp -fmodule-file=lib=%t/lib.pcm \
+// RUN: -verify -fsyntax-only
+
+//--- header.h
+namespace lib::inline __1 {
+template 
+inline constexpr bool test = false;
+template 
+constexpr bool func() {
+return false;
+}
+inline constexpr bool non_templ = true;
+} // namespace lib
+
+//--- lib.cppm
+module;
+#include "header.h"
+export module lib;
+
+export namespace lib {
+using lib::test;
+using lib::func;
+using lib::non_templ;
+} // namespace lib
+
+//--- main.cpp
+// expected-no-diagnostics
+import lib;
+
+struct foo {};
+
+template <>
+inline constexpr bool lib::test = true;
+
+template <>
+constexpr bool lib::func() {
+return true;
+}
+
+static_assert(lib::test);
+static_assert(lib::func());
Index: clang/test/CXX/special/class.inhctor/p8.cpp
===
--- clang/test/CXX/special/class.inhctor/p8.cpp
+++ clang/test/CXX/special/class.inhctor/p8.cpp
@@ -29,4 +29,5 @@
 };
 static_assert(D(123).v == 123, "");
 
+// FIMXE: Why should the following line be rejected?
 template constexpr D::D(T t) : C(t) {} // expected-error {{does not match any declaration in 'D'}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1821,17 +1821,20 @@
   return OldM == NewM;
 }
 
-static bool isUsingDecl(NamedDecl *D) {
+static bool isUsingDeclAtClassScope(NamedDecl *D) {
+  if (D->getDeclContext()->isFileContext())
+return false;
+
   return isa(D) ||
  isa(D) ||
  isa(D);
 }
 
-/// Removes using shadow declarations from the lookup results.
+/// Removes using shadow declarations at class scope from the lookup results.
 static void RemoveUsingDecls(LookupResult &R) {
   LookupResult::Filter F = R.makeFilter();
   while (F.hasNext())
-if (isUsingDecl(F.next()))
+if (isUsingDeclAtClassScope(F.next()))
   F.erase();
 
   F.done();
@@ -6372,10 +6375,7 @@
 // containing the two f's declared in X, but neither of them
 // matches.
 
-// C++ [dcl.meaning]p1:
-//   [...] the member shall not merely have been introduced by a
-//   using-declaration in the scope of the class or namespace nominated by
-//   the nested-name-specifier of the declarator-id.
+// FIXME: Why do we need to remove using decls here?
 RemoveUsingDecls(Previous);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148506: [C++] Don't filter using declaration when we perform qualified look up

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:1824-1826
+static bool isUsingDeclAtClassScope(NamedDecl *D) {
+  if (D->getDeclContext()->isFileContext())
+return false;

This is the ad-hoc change. Look at the following comment.



Comment at: clang/lib/Sema/SemaDecl.cpp:6375-6378
-// C++ [dcl.meaning]p1:
-//   [...] the member shall not merely have been introduced by a
-//   using-declaration in the scope of the class or namespace nominated by
-//   the nested-name-specifier of the declarator-id.

I can't find the wording in the existing spec (at least not in current 
[dcl.meaning]). But if p8.cpp will be accepted unexpectedly if I remove 
`RemoveUsingDecls(Previous);` completely. It would look good to me if we can 
remove `RemoveUsingDecls(Previous);` completely.



Comment at: clang/test/CXX/special/class.inhctor/p8.cpp:32-33
 
+// FIMXE: Why should the following line be rejected?
 template constexpr D::D(T t) : C(t) {} // expected-error {{does 
not match any declaration in 'D'}}

I don't understand why this should be rejected. I tried to read P0136R1 but I 
failed to find the related things and I don't find a related wording in the 
current spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148506

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

[PATCH] D148206: [clang] Do not crash after suggesting typo correction to constexpr if condition

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@aaron.ballman , are you ok with the change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148206

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

[PATCH] D148484: [clang-format] Correctly format goto labels followed by blocks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:714
 // are in a control flow statements as well as several style flags.
-if (Line.First->is(tok::kw_case) ||
+if (Line.First->is(tok::kw_case) || Line.Last->is(TT_GotoLabelColon) ||
+Line.Last->endsSequence(tok::l_brace, TT_GotoLabelColon) ||

sstwcw wrote:
> Should I make this change?
> 
> Without it:
> 
> ```
> label: { break; }
> ```
> 
> With it:
> 
> ```
> label: {
>   break;
> }
> ```
> 
> Without the entire patch:
> 
> ```
> label : { break; }
> ```
I'd say no. To not change this (too much).



Comment at: clang/unittests/Format/FormatTest.cpp:3020-3024
   verifyFormat("{\n"
"  some_code();\n"
-   "test_label:;\n"
-   "  int i = 0;\n"
-   "}");

Why did you remove that?



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1711
 
+
 TEST_F(TokenAnnotatorTest, UnderstandsConditionParens) {

remove


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148484

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:1220
+/// \endcode
+bool AfterCSharpProperty;
   };

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > Please sort. :)
> Are we sure we want THIS to be alphabetic, as this changes the initializer 
> order, if someone is using the format() library in downstream code this could 
> subtly break them?
> 
> ```
> Expanded.BraceWrapping = {/*AfterCaseLabel=*/false,
> /*AfterClass=*/false,
> 
> /*AfterControlStatement=*/FormatStyle::BWACS_Never,
> /*AfterEnum=*/false,
> /*AfterFunction=*/false,
> /*AfterNamespace=*/false,
> /*AfterObjCDeclaration=*/false,
> /*AfterStruct=*/false,
> /*AfterUnion=*/false,
> /*AfterExternBlock=*/false,
> /*BeforeCatch=*/false,
> /*BeforeElse=*/false,
> /*BeforeLambdaBody=*/false,
> /*BeforeWhile=*/false,
> /*IndentBraces=*/false,
> /*SplitEmptyFunction=*/true,
> /*SplitEmptyRecord=*/true,
> /*SplitEmptyNamespace=*/true,
> /*AfterCSharpProperty=*/false};
> ```
I'd say yes, we are breaking this stuff always.

Granted this one may compile without an error, but they should get a warning of 
a missing initializer.

You could add a constructor to initialize out of struct order.


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

https://reviews.llvm.org/D148467

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

[PATCH] D148410: [Parse] Remove TimeTraceScope for "ParseTemplate"

[Ying Yi via Phabricator via cfe-commits]

MaggieYi added a comment.

Thanks @MaskRay, the fix is fine for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148410

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

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

In D143870#4273075 , @MyDeveloperDay 
wrote:

> I know it might not seem an obvious use case but there really isn't a 
> requirement to not include header files more than once.. imagine if I have
>
>   #define ARCH "win32"
>   #include "MyDataStructThatContainsPlaformSpecificNamesUsingARCH.h"
>   #define ARCH "win64"
>   #include "MyDataStructThatContainsPlaformSpecificNamesUsingARCH.h"
>
> Its not nice, but just because I include it twice doesn't mean its wrong? 
> This change would break code written that way, No?

Mhh, I need to test that. But you are right, if it removes the header in one of 
the conditional preprocessor blocks, it would be very bad.
Regarding normal blocks, the behavior was already inconsistent, as soon to 
reorder blocks was activated:
Blocks are merged if possible, then de-duplicated and split again. So I just 
made the behavior more consistent here.

Regarding the comment that I must not change existing tests: I think this rule 
is too strict, because those tests are mostly regression tests. 
But a regression tests does not test for correctness. So if a test had already 
a wrong assumption, it must be changeable. 
Also, when a behavior is now undesirable, it should be possible to adapt them.
I would say in this case I should replace the test with your case, to reflect 
that includes must not be removed, when they are in different conditional 
preprocessor blocks.
That the different "modi" should be more consistent is in my opinion desirable.

Last but not least, I currently have nearly zero free time, so I would like to 
pause this and my other phab until my thesis is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143870

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

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

> "The warning was emitted at every occurence of the function. It might be 
> confusing if it's only emitted for the definition."
> Why ? Issue is in definition, not declaration.

For me it would be confusing, because the forward declaration is naming the 
same function as the definition.
If I see that something is reported for the definition but not for the 
declaration I might think it's a bug in the 
tool, like once there is a problem with the function and the other time there 
isn't. Note that this particular 
warning is reported for the function and not for something inside the 
definition.

Also we have cross translation unit analysis, though I'm not sure this 
particular checker works there too.
Assuming it does, what happens if I forward declare the function in one 
translation unit and define it in
a different one? With this change the warning will only be output in the 
translation unit,the function is
defined in and this might silently hide some other problems in the TU the 
function is forward declared in.

> recursion_helper does not throw, it crashes.

Technically the exception is propagated through the function until a handler is 
found that catches it.

> Example with indirectly_recursive & recursion_helper behave in same way, only 
> difference is that warning is emitted only for definition.

Please add a test case for this regardless of the behaviour to see that the 
checker handles exception propagation.

> This is other bug that I'm fixing (not under this patch) related to properly 
> handling noexcept keyword.

I'm not sure what you mean by bug here.

  int recursion_helper(int n) noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
// function 'recursion_helper' which should not throw exceptions
indirectly_recursive(n);
  }
  
  int indirectly_recursive(int n) noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
// function 'indirectly_recursive' which should not throw exceptions
if (n == 0)
  throw n;
return recursion_helper(n);
  }

Because `recursion_helper()` propagates the thrown object it makes sense to 
emit a warning for that too.
Also because the warning is emitted upon every propagation it is easy to trace 
where the exception actually
comes from. Think of it like a stack trace for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148462

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:1220
+/// \endcode
+bool AfterCSharpProperty;
   };

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > Please sort. :)
> > Are we sure we want THIS to be alphabetic, as this changes the initializer 
> > order, if someone is using the format() library in downstream code this 
> > could subtly break them?
> > 
> > ```
> > Expanded.BraceWrapping = {/*AfterCaseLabel=*/false,
> > /*AfterClass=*/false,
> > 
> > /*AfterControlStatement=*/FormatStyle::BWACS_Never,
> > /*AfterEnum=*/false,
> > /*AfterFunction=*/false,
> > /*AfterNamespace=*/false,
> > /*AfterObjCDeclaration=*/false,
> > /*AfterStruct=*/false,
> > /*AfterUnion=*/false,
> > /*AfterExternBlock=*/false,
> > /*BeforeCatch=*/false,
> > /*BeforeElse=*/false,
> > /*BeforeLambdaBody=*/false,
> > /*BeforeWhile=*/false,
> > /*IndentBraces=*/false,
> > /*SplitEmptyFunction=*/true,
> > /*SplitEmptyRecord=*/true,
> > /*SplitEmptyNamespace=*/true,
> > /*AfterCSharpProperty=*/false};
> > ```
> I'd say yes, we are breaking this stuff always.
> 
> Granted this one may compile without an error, but they should get a warning 
> of a missing initializer.
> 
> You could add a constructor to initialize out of struct order.
I'm ok with making the change, just wanted to double check that we are ok to 
break the ordering.


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

https://reviews.llvm.org/D148467

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

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

@isuckatcs
"Technically the exception is propagated through the function until a handler 
is found that catches it."
No because indirectly_recursive called from recursion_helper is noexcept, so 
there will be std::terminate called.

"Also we have cross translation unit analysis"
Not in clang-tidy, and that work based on AST merge, so even if someone manage 
to run it here, it will work fine.

"Please add a test case for this regardless of the behaviour to see that the 
checker handles exception propagation."
There is test for that. The one with recursion_helper + indirectly_recursive. 
Be more specific if you want something else.

"Because recursion_helper() propagates the thrown object it makes sense to emit 
a warning for that too."
No it's not propagating thrown object. Bug: 
https://github.com/llvm/llvm-project/issues/54956

As for warnings for forward declaration, what developer have to do with such 
information ? There is nothing he can change in forward declaration to fix this 
issue. And putting 2 or more NOLINTS to silence single issue is stupid. Other 
checks validate only definitions. Forward declarations are forward 
declarations, they transparent for an exception propagation. And current 
behaviour introduce also performance penalty, as same thing is done multiple 
times. If you somehow want to point to developer issue about function 
declaration, there are notes for that, but still developer know about 
declarations, and to fix issue He/She don't need to touch this, only definition 
need to be changed (unlles decides to remove noexcept keyword, but then 
compiler will tell him about issues). Same goes for pure virtual functions, 
check does not emit warnings for them just because some implementation throw 
exception.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148462

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

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

@isuckatcs
"Note that this particular warning is reported for the function and not for 
something inside the definition."

Function declaration is not a function.

A function declaration is a statement in programming languages that declares 
the existence of a function, including its name, parameters, and return type 
(if applicable). It is used to define the function and make it available for 
use in the program.
On the other hand, a function is a set of instructions that performs a specific 
task and can be called by other parts of the program. When a function 
declaration is executed, it creates a function object that can be called as a 
function. So, while a function declaration is a necessary step in creating a 
function, it is not the function itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148462

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

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added a comment.

Thanks for the review! I'll simplify the masks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143287

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

[PATCH] D143364: [RISCV] Support scalar/fix-length vector NTLH intrinsic with different domain

[Piyou Chen via Phabricator via cfe-commits]

BeMg added inline comments.



Comment at: clang/test/CodeGen/RISCV/ntlh-intrinsics/riscv32-zihintntl.c:19
+typedef signed short v8ss __attribute__((vector_size(16)));
+typedef signed char v16sc __attribute__((vector_size(16)));
+v4si v4si1, v4si2;

craig.topper wrote:
> What about the rvv builtin types for scalable vectors?
Should we support rvv builtin types for __riscv_ntl_load/store although we 
already have plan to support RVV builtin type in nontemporal by extra RVV 
intrinsic functions?

It need some extra code to support these function with scalable vector type. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143364

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

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Jan-Patrick Lehr via Phabricator via cfe-commits]

jplehr added a comment.

I'll get back to this soon, enjoyed vacation. ;)


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

https://reviews.llvm.org/D140722

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

[PATCH] D148330: [clang] Do not crash on undefined template partial specialization

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:134
  "specifier in SFINAE context?");
-if (!hasReachableDefinition(PartialSpec))
+if (PartialSpec->hasDefinition() &&
+!hasReachableDefinition(PartialSpec))

shafik wrote:
> So I see in another location we are basically checking `!isBeginDefined()` 
> and in another place `hasCompleteDefinition()`. It is not clear to me if 
> these are all basically checking the same thing or if we should figure out 
> what is the right thing to check and do that everywhere. 
I suppose you meant `isBeingDefined()`. So, IMO `isBeingDefined()` is not 
exactly the same as having a definition, this is set to `true` when 
`TagDecl::startDefinition` I suppose this should be done when parser meets a 
definition so at this point it can't be or have a complete definition. But at 
the places where `!isBeingDefined()` is checked prior `hasReachableDefinition` 
I saw a pointer representing a definition checked for not being null. 
Interestingly, `hasReachableDefinition()` early exits if `isBeingDefined()` 
returns true, so probably this check additional doesn't make sense at all.
Maybe we should just move both checks on having a definition and not being in 
the process of definition under `hasReachableDefinition` after all. That will 
require changing unrelated places, so I would prefer doing this separately in 
any case.

I can't grep `hasCompleteDefinition()` either, so I suppose you meant 
`isCompleteDefinition()`, I think this is basically the same thing as having 
definition and additionally the declaration through which the method called is 
checked that IT IS the definition. I'm not sure we have to require it here.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148330

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

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a subscriber: arphaman.
ldionne added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431
+  // libc++.dylib in the toolchain.
+  if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
+options::OPT_nostdincxx)) &&

The more I look at this, the more I think it seems kind of weird to me that a 
linker argument would be impacted by `-nostdinc` & friends, which control 
header search paths. IMO we should use the simplest behavior and only check for 
`libc++.dylib` here, and not check for `libc++.dylib` when we determine the 
header search paths.

@arphaman Do you have an opinion?



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
   if (sysroot != "") {
 CmdArgs.push_back("-syslibroot");
 CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));

ldionne wrote:
> Where do we set the usual `/usr/lib` search path? I think it might 
> make sense to move the code you added to that place.
What I mean: there's a place where we must be adding `-L /usr/lib` in 
the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it? Would it 
make sense to move the code you added to that place instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148266

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

[PATCH] D146809: [WIP][clang-repl] Implement Value pretty printing

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 514160.
junaire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp

Index: clang/test/Interpreter/pretty-print.cpp
===
--- /dev/null
+++ clang/test/Interpreter/pretty-print.cpp
@@ -0,0 +1,79 @@
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl | FileCheck %s
+char c = 'a';
+c
+// CHECK: (char) 'a'
+
+int x = 42;
+x
+// CHECK-NEXT: (int) 42
+
+x - 2
+// CHECK-NEXT: (int) 40
+
+float f = 4.2f;
+f
+// CHECK-NEXT: (float) 4.2f
+
+double d = 4.21;
+d
+// CHECK-NEXT: (double) 4.210
+
+struct S{};
+S s;
+s
+// CHECK-NEXT: (S &) [[Addr:@0x.*]]
+
+S{}
+// CHECK-NEXT: (S) [[Addr:@0x.*]]
+
+struct SS { int* p; SS() { p = new int(42); } ~SS() { delete p; } };
+SS{}
+// CHECK-NEXT: (SS) [[Addr:@0x.*]]
+SS ss;
+ss
+// CHECK-NEXT: (SS &) [[Addr:@0x.*]]
+
+int arr[3] = {1,2,3};
+arr
+// CHECK-NEXT: (int[3]) { 1, 2, 3 }
+
+#include 
+
+auto p1 = std::make_shared(42);
+p1
+// CHECK-NEXT: (shared_ptr &) std::shared_ptr -> [[Addr:@0x.*]]
+
+#include 
+std::array a{1, 2, 3};
+a
+// CHECK-NEXT: (std::array &) { 1, 2, 3 }
+
+#include 
+std::vector v = {7, 5, 16, 8};
+v
+// CHECK-NEXT: (std::vector &) { 7, 5, 16, 8 }
+
+#include 
+std::deque dq = {7, 5, 16, 8};
+dq
+// CHECK-NEXT: (std::deque &) { 7, 5, 16, 8 }
+
+#include 
+std::forward_list fl {3,4,5,6};
+fl
+// CHECK-NEXT: (std::forward_list &) { 3, 4, 5, 6 }
+
+#include 
+std::set z = {2,4,6,8};
+z
+// CHECK-NEXT: (std::set &) { 2, 4, 6, 8 }
+
+std::multiset e {3,2,1,2,4,7,3};
+e
+// CHECK-NEXT: (std::multiset &) { 1, 2, 2, 3, 3, 4, 7 }
+%quit
+
Index: clang/lib/Interpreter/ValuePrinter.cpp
===
--- /dev/null
+++ clang/lib/Interpreter/ValuePrinter.cpp
@@ -0,0 +1,501 @@
+#include "InterpreterUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/Type.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Interpreter/Value.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+using namespace clang;
+
+static std::string PrintDeclType(const QualType &QT, NamedDecl *D) {
+  std::string Str;
+  llvm::raw_string_ostream SS(Str);
+  if (QT.hasQualifiers())
+SS << QT.getQualifiers().getAsString() << " ";
+  SS << D->getQualifiedNameAsString();
+  return Str;
+}
+
+static std::string PrintQualType(ASTContext &Ctx, QualType QT) {
+  std::string Str;
+  llvm::raw_string_ostream SS(Str);
+  PrintingPolicy Policy(Ctx.getPrintingPolicy());
+  // Print the Allocator in STL containers, for instance.
+  Policy.SuppressDefaultTemplateArgs = false;
+  Policy.SuppressUnwrittenScope = true;
+  // Print 'a >' rather than 'a>'.
+  Policy.SplitTemplateClosers = true;
+
+  struct LocalPrintingPolicyRAII {
+ASTContext &Context;
+PrintingPolicy Policy;
+
+LocalPrintingPolicyRAII(ASTContext &Ctx, PrintingPolicy &PP)
+: Context(Ctx), Policy(Ctx.getPrintingPolicy()) {
+  Context.setPrintingPolicy(PP);
+}
+~LocalPrintingPolicyRAII() { Context.setPrintingPolicy(Policy); }
+  } X(Ctx, Policy);
+
+  const QualType NonRefTy = QT.getNonReferenceType();
+
+  if (const auto *TTy = llvm::dyn_cast(NonRefTy))
+SS << PrintDeclType(NonRefTy, TTy->getDecl());
+  else if (const auto *TRy = dyn_cast(NonRefTy))
+SS << PrintDeclType(NonRefTy, TRy->getDecl());
+  else {
+const QualType Canon = NonRefTy.getCanonicalType();
+if (Canon->isBuiltinType() && !NonRefTy->isFunctionPointerType() &&
+!NonRefTy->isMemberPointerType()) {
+  SS << Canon.getAsString(Ctx.getPrintingPolicy());
+} else if (const auto *TDTy = dyn_cast(NonRefTy)) {
+  // FIXME: TemplateSpecializationType & SubstTemplateTypeParmType checks
+  // are predominately to get STL containers to print nicer and might be
+  // better handled in GetFullyQual

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

zequanwu wrote:
> hans wrote:
> > Won't the code above (line 580) make many filenames absolute and cause us 
> > to use native slashes even when we want backslashes?
> > 
> > This would also need a test.
> > Won't the code above (line 580) make many filenames absolute and cause us 
> > to use native slashes even when we want backslashes?
> Yes, I don't think we should change anything if it's converted to an absolute 
> path.
> 
> Only if the `-fdebug-compilation-dir` is not given or is an absolute path, 
> the path in `/Fo` will be converted to absolute path. Users can avoid the 
> path being converted to absolute path by passing a relative path to 
> `-fdebug-compilation-dir`, just like what we do in chrome build 
> (`-fdebug-compilation-dir=.`).
> 
> Added a test.
Apologies for all the questions, but I find the logic a bit complex still. The 
test cases are helpful though.

Could we simplify by just always using Windows slashes here, regardless of 
`-fdebug-compilation-dir` etc.?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D148435: [clang-repl] Do not assert if we have weak references left.

[Jun Zhang via Phabricator via cfe-commits]

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

This patch fixes issues that have not been found in the past. This unblocks 
D141215  and D146809 
. Since it's only relevant to the clang-repl, 
I believe this is safe to just land it. (I added these assertions in  
https://reviews.llvm.org/rGcd64a427efa0baaf1bb7ae624d4301908afc07f7 ) However, 
it will be nice if other reviewers, maybe @rjmccall could take a look at this 
as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D148435

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

[PATCH] D148206: [clang] Do not crash after suggesting typo correction to constexpr if condition

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148206

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

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon marked an inline comment as not done.
Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:808
   unsigned NumElems = numStructUnionElements(ILE->getType());
-  if (RDecl->hasFlexibleArrayMember())
+  if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
 ++NumElems;

Fznamznon wrote:
> aaron.ballman wrote:
> > Fznamznon wrote:
> > > shafik wrote:
> > > > Fznamznon wrote:
> > > > > Just for some context, numStructUnionElements checks that there is a 
> > > > > flexible array member and returns number_of_initializable_fields-1 
> > > > > for structs. For unions it just returns 1 or 0, so flexible array 
> > > > > member caused adding one more element to initlistexpr that was never 
> > > > > properly handled.
> > > > > 
> > > > > Instead of doing this change, we could probably never enter 
> > > > > initialization since the record (union) declaration is not valid, but 
> > > > > that is not the case even for other types of errors in code, for 
> > > > > example, I've tried declaring field of struct with a typo:
> > > > > 
> > > > > ```
> > > > > struct { cha x[]; } r = {1}; 
> > > > > ```
> > > > > Initialization is still performed by clang.
> > > > > Also, it seems MSVC considers flexible array member inside union as 
> > > > > valid, so the test code is probably not always invalid.
> > > > I am not sure what to think here, looking at gcc documentation for this 
> > > > extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> > > > 
> > > > and using the following code:
> > > > 
> > > > ```
> > > > struct f1 {
> > > >   int x; int y[];
> > > > } f1 = { 1, { 2, 3, 4 } }; // #1
> > > > 
> > > > struct f2 {
> > > >   struct f1 f1; int data[3];
> > > > } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> > > > 
> > > > struct { char x[]; } r = {1};  // #3
> > > > ```
> > > > 
> > > > gcc rejects 2 and 3 even though 2 comes from their documentation. Clang 
> > > > warns on 2 and MSVC rejects 2
> > > > 
> > > > CC @aaron.ballman @rsmith 
> > > Yes, I also had a feeling that we probably need to refine how these 
> > > extensions are supported by clang, that is probably a bit out of scope of 
> > > the fix though.
> > The GCC extension differs based on C vs C++: https://godbolt.org/z/E14Yz37To
> > As does the extension in Clang, but differently than GCC: 
> > https://godbolt.org/z/zYznaYPf5
> > 
> > So I agree that there's work to be done on this extension, but it's outside 
> > of the scope of the crash fix for this patch. For this patch, GCC rejects 
> > allowing a flexible array member in a union in both C and C++, but it looks 
> > like Clang rejects in C and perhaps accepts in C++: 
> > https://godbolt.org/z/bTsPn3G7b So how about we add a C++ test case for 
> > this as well -- if that still crashes, that should be fixed, but if the 
> > code is accepted, we should either decide we want to start rejecting it or 
> > we should ensure the codegen for it is correct.
> While experimenting with C++ test, I've noticed the following thing:
> ```
> union { char x[]; } r = {0}; // when in global scope generates no IR (without 
> my patch crashes)
> 
> void foo() {
>   union { char x[]; } r = {0}; // fails with error "initialization of 
> flexible array member is not allowed" in both C/C++, no crash even without my 
> patch
> }
> 
> union A {char x[]; };
> A a = {0}; // crashes even with my patch but with different assertion when 
> trying -emit-llvm
> void foo() {
>   A a = {0}; // fails with error "initialization of flexible array member is 
> not allowed" in both C and C++, no crash even without my patch
> }
> ```
> 
> It is not entirely clear to me why the behavior different for function and TU 
> scope. gcc always gives an error about flexible array in union, no matter how 
> it is used. Also, it is strange that I'm not seeing the same "initialization 
> of flexible array member is not allowed" error message for structs, just for 
> unions. I'm thinking that we don't really have proper codegen for the code 
> that I'm trying to fix and we should reject the code like gcc does. MSVC is 
> fine with all though - https://godbolt.org/z/aoarEzd56 .
> 
> WDYT?
Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

[Manuel Brito via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5184dc2d7cce: [Clang][X86] Change X86 cast intrinsics to use 
__builtin_nondeterministic_value (authored by ManuelJBrito).

Changed prior to commit:
  https://reviews.llvm.org/D143287?vs=511663&id=514165#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143287

Files:
  clang/lib/Headers/avx512fintrin.h
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avxintrin.h
  clang/test/CodeGen/X86/avx-builtins.c
  clang/test/CodeGen/X86/avx-cast-builtins.c
  clang/test/CodeGen/X86/avx512f-builtins.c
  clang/test/CodeGen/X86/avx512fp16-builtins.c

Index: clang/test/CodeGen/X86/avx512fp16-builtins.c
===
--- clang/test/CodeGen/X86/avx512fp16-builtins.c
+++ clang/test/CodeGen/X86/avx512fp16-builtins.c
@@ -325,19 +325,26 @@
 
 __m256h test_mm256_castph128_ph256(__m128h __a) {
   // CHECK-LABEL: test_mm256_castph128_ph256
-  // CHECK: shufflevector <8 x half> %{{.*}}, <8 x half> %{{.*}}, <16 x i32> 
+  // CHECK: [[A:%.*]] = freeze <8 x half> poison 
+  // CHECK: shufflevector <8 x half> %{{.*}}, <8 x half> [[A]], <16 x i32> 
   return _mm256_castph128_ph256(__a);
 }
 
 __m512h test_mm512_castph128_ph512(__m128h __a) {
   // CHECK-LABEL: test_mm512_castph128_ph512
-  // CHECK: shufflevector <8 x half> %{{.*}}, <8 x half> %{{.*}}, <32 x i32> 
+  // CHECK: [[B:%.*]] = freeze <16 x half> poison
+  // CHECK: store <16 x half> [[B]], ptr [[BA:%.*]]
+  // CHECK: [[A:%.*]] = freeze <8 x half> poison
+  // CHECK: [[SV:%.*]] = shufflevector <8 x half> %{{.*}}, <8 x half> [[A]], <16 x i32> 
+  // CHECK: [[C:%.*]] = load <16 x half>, ptr [[BA]]
+  // CHECK: shufflevector <16 x half> [[SV]], <16 x half> [[C]], <32 x i32> 
   return _mm512_castph128_ph512(__a);
 }
 
 __m512h test_mm512_castph256_ph512(__m256h __a) {
   // CHECK-LABEL: test_mm512_castph256_ph512
-  // CHECK: shufflevector <16 x half> %{{.*}}, <16 x half> %{{.*}}, <32 x i32> 
+  // CHECK: [[A:%.*]] = freeze <16 x half> poison 
+  // CHECK: shufflevector <16 x half> %{{.*}}, <16 x half> [[A]], <32 x i32> 
   return _mm512_castph256_ph512(__a);
 }
 
Index: clang/test/CodeGen/X86/avx512f-builtins.c
===
--- clang/test/CodeGen/X86/avx512f-builtins.c
+++ clang/test/CodeGen/X86/avx512f-builtins.c
@@ -8987,13 +8987,23 @@
 
 __m512 test_mm512_castps128_ps512(__m128 __A) {
   // CHECK-LABEL: @test_mm512_castps128_ps512
-  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <16 x i32> 
+  // CHECK: [[B:%.*]] = freeze <8 x float> poison
+  // CHECK: store <8 x float> [[B]], ptr [[BA:%.*]]
+  // CHECK: [[A:%.*]] = freeze <4 x float> poison 
+  // CHECK: [[SV:%.*]] = shufflevector <4 x float> %{{.*}}, <4 x float> [[A]], <8 x i32> 
+  // CHECK: [[C:%.*]] = load <8 x float>, ptr [[BA]]
+  // CHECK: shufflevector <8 x float> [[SV]], <8 x float> [[C]], <16 x i32> 
   return _mm512_castps128_ps512(__A); 
 }
 
 __m512d test_mm512_castpd128_pd512(__m128d __A) {
   // CHECK-LABEL: @test_mm512_castpd128_pd512
-  // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <8 x i32> 
+  // CHECK: [[B:%.*]] = freeze <4 x double> poison
+  // CHECK: store <4 x double> [[B]], ptr [[BA:%.*]]
+  // CHECK: [[A:%.*]] = freeze <2 x double> poison 
+  // CHECK: [[SV:%.*]] = shufflevector <2 x double> %{{.*}}, <2 x double> [[A]], <4 x i32> 
+  // CHECK: [[C:%.*]] = load <4 x double>, ptr [[BA]]
+  // CHECK: shufflevector <4 x double> [[SV]], <4 x double> [[C]], <8 x i32> 
   return _mm512_castpd128_pd512(__A); 
 }
 
@@ -9086,7 +9096,8 @@
 __m512d test_mm512_castpd256_pd512(__m256d a)
 {
   // CHECK-LABEL: @test_mm512_castpd256_pd512
-  // CHECK: shufflevector <4 x double> {{.*}} 
+  // CHECK: [[A:%.*]] = freeze <4 x double> poison 
+  // CHECK: shufflevector <4 x double> %{{.}}, <4 x double> [[A]], <8 x i32> 
   return _mm512_castpd256_pd512(a);
 }
 
@@ -9112,13 +9123,19 @@
 }
 __m512i test_mm512_castsi128_si512(__m128i __A) {
   // CHECK-LABEL: @test_mm512_castsi128_si512
-  // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <8 x i32> 
+  // CHECK: [[B:%.*]] = freeze <4 x i64> poison
+  // CHECK: store <4 x i64> [[B]], ptr [[BA:%.*]]
+  // CHECK: [[A:%.*]] = freeze <2 x i64> poison 
+  // CHECK: [[SV:%.*]] = shufflevector <2 x i64> %{{.*}}, <2 x i64> [[A]], <4 x i32> 
+  // CHECK: [[C:%.*]] = load <4 x i64>, ptr [[BA]]
+  // CHECK: shufflevector <4 x i64> [[SV]], <4 x i64> [[C]], <8 x i32> 
   return _mm512_castsi128_si512(__A); 
 }
 
 __m512i test_mm512_castsi256_si512(__m256i __A) {
   // CHECK-LABEL: @test_mm512_castsi256_si512
-  // CHECK: shufflevector <4 x i64> %{{.*}}, <4 x i64> %{{.*}}, <8 x i32> 
+  // CHECK: [[A:%.*]] = freeze <4 x i64> poison 
+  // CHECK: shufflevector <4 x i64> %{{.*}}, <4 x 

[clang] 5184dc2 - [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

[via cfe-commits]

Author: ManuelJBrito
Date: 2023-04-17T12:58:36+01:00
New Revision: 5184dc2d7cce5971f4386c6f99d04f87233dd313

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

LOG: [Clang][X86] Change X86 cast intrinsics to use 
__builtin_nondeterministic_value

The following intrinsics are currently implemented using a shufflevector with
an undefined mask, this is however incorrect according to intel's semantics for
undefined value which expect an unknown but consistent value.

With __builtin_nondeterministic_value we can now match intel's undefined value.

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

Added: 
clang/test/CodeGen/X86/avx-cast-builtins.c

Modified: 
clang/lib/Headers/avx512fintrin.h
clang/lib/Headers/avx512fp16intrin.h
clang/lib/Headers/avxintrin.h
clang/test/CodeGen/X86/avx-builtins.c
clang/test/CodeGen/X86/avx512f-builtins.c
clang/test/CodeGen/X86/avx512fp16-builtins.c

Removed: 




diff  --git a/clang/lib/Headers/avx512fintrin.h 
b/clang/lib/Headers/avx512fintrin.h
index b19d2fb90ff58..88a8cebbee301 100644
--- a/clang/lib/Headers/avx512fintrin.h
+++ b/clang/lib/Headers/avx512fintrin.h
@@ -397,14 +397,15 @@ _mm512_broadcastsd_pd(__m128d __A)
 static __inline __m512d __DEFAULT_FN_ATTRS512
 _mm512_castpd256_pd512(__m256d __a)
 {
-  return __builtin_shufflevector(__a, __a, 0, 1, 2, 3, -1, -1, -1, -1);
+  return __builtin_shufflevector(__a, __builtin_nondeterministic_value(__a), 0,
+ 1, 2, 3, 4, 5, 6, 7);
 }
 
 static __inline __m512 __DEFAULT_FN_ATTRS512
 _mm512_castps256_ps512(__m256 __a)
 {
-  return __builtin_shufflevector(__a, __a, 0,  1,  2,  3,  4,  5,  6,  7,
-  -1, -1, -1, -1, -1, -1, -1, -1);
+  return __builtin_shufflevector(__a, __builtin_nondeterministic_value(__a), 0,
+ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 
14, 15);
 }
 
 static __inline __m128d __DEFAULT_FN_ATTRS512
@@ -446,7 +447,10 @@ _mm512_castpd_si512 (__m512d __A)
 static __inline__ __m512d __DEFAULT_FN_ATTRS512
 _mm512_castpd128_pd512 (__m128d __A)
 {
-  return __builtin_shufflevector( __A, __A, 0, 1, -1, -1, -1, -1, -1, -1);
+  __m256d __B = __builtin_nondeterministic_value(__B);
+  return __builtin_shufflevector(
+  __builtin_shufflevector(__A, __builtin_nondeterministic_value(__A), 0, 
1, 2, 3),
+  __B, 0, 1, 2, 3, 4, 5, 6, 7);
 }
 
 static __inline __m512d __DEFAULT_FN_ATTRS512
@@ -464,19 +468,25 @@ _mm512_castps_si512 (__m512 __A)
 static __inline__ __m512 __DEFAULT_FN_ATTRS512
 _mm512_castps128_ps512 (__m128 __A)
 {
-return  __builtin_shufflevector( __A, __A, 0, 1, 2, 3, -1, -1, -1, -1, -1, 
-1, -1, -1, -1, -1, -1, -1);
+  __m256 __B = __builtin_nondeterministic_value(__B);
+  return __builtin_shufflevector(
+  __builtin_shufflevector(__A, __builtin_nondeterministic_value(__A), 0, 
1, 2, 3, 4, 5, 6, 7),
+  __B, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS512
 _mm512_castsi128_si512 (__m128i __A)
 {
-   return  __builtin_shufflevector( __A, __A, 0, 1, -1, -1, -1, -1, -1, -1);
+  __m256i __B = __builtin_nondeterministic_value(__B);
+  return __builtin_shufflevector(
+  __builtin_shufflevector(__A, __builtin_nondeterministic_value(__A), 0, 
1, 2, 3),
+  __B, 0, 1, 2, 3, 4, 5, 6, 7);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS512
 _mm512_castsi256_si512 (__m256i __A)
 {
-   return  __builtin_shufflevector( __A, __A, 0, 1, 2, 3, -1, -1, -1, -1);
+   return  __builtin_shufflevector( __A, 
__builtin_nondeterministic_value(__A), 0, 1, 2, 3, 4, 5, 6, 7);
 }
 
 static __inline __m512 __DEFAULT_FN_ATTRS512

diff  --git a/clang/lib/Headers/avx512fp16intrin.h 
b/clang/lib/Headers/avx512fp16intrin.h
index 5cdc37fde629f..d326586578bb3 100644
--- a/clang/lib/Headers/avx512fp16intrin.h
+++ b/clang/lib/Headers/avx512fp16intrin.h
@@ -192,22 +192,26 @@ _mm512_castph512_ph256(__m512h __a) {
 
 static __inline__ __m256h __DEFAULT_FN_ATTRS256
 _mm256_castph128_ph256(__m128h __a) {
-  return __builtin_shufflevector(__a, __a, 0, 1, 2, 3, 4, 5, 6, 7, -1, -1, -1,
- -1, -1, -1, -1, -1);
+  return __builtin_shufflevector(__a, __builtin_nondeterministic_value(__a),
+  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 
13, 14, 15);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512
 _mm512_castph128_ph512(__m128h __a) {
-  return __builtin_shufflevector(__a, __a, 0, 1, 2, 3, 4, 5, 6, 7, -1, -1, -1,
- -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 
-1,
- -1, -1, -1, -1, -1, -1, -1, -1, -1);
+  __m256h __b = __builtin_nondeterministic_value(__b);
+  return __builtin_shufflevector(
+   

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:808
   unsigned NumElems = numStructUnionElements(ILE->getType());
-  if (RDecl->hasFlexibleArrayMember())
+  if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
 ++NumElems;

Fznamznon wrote:
> Fznamznon wrote:
> > aaron.ballman wrote:
> > > Fznamznon wrote:
> > > > shafik wrote:
> > > > > Fznamznon wrote:
> > > > > > Just for some context, numStructUnionElements checks that there is 
> > > > > > a flexible array member and returns 
> > > > > > number_of_initializable_fields-1 for structs. For unions it just 
> > > > > > returns 1 or 0, so flexible array member caused adding one more 
> > > > > > element to initlistexpr that was never properly handled.
> > > > > > 
> > > > > > Instead of doing this change, we could probably never enter 
> > > > > > initialization since the record (union) declaration is not valid, 
> > > > > > but that is not the case even for other types of errors in code, 
> > > > > > for example, I've tried declaring field of struct with a typo:
> > > > > > 
> > > > > > ```
> > > > > > struct { cha x[]; } r = {1}; 
> > > > > > ```
> > > > > > Initialization is still performed by clang.
> > > > > > Also, it seems MSVC considers flexible array member inside union as 
> > > > > > valid, so the test code is probably not always invalid.
> > > > > I am not sure what to think here, looking at gcc documentation for 
> > > > > this extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> > > > > 
> > > > > and using the following code:
> > > > > 
> > > > > ```
> > > > > struct f1 {
> > > > >   int x; int y[];
> > > > > } f1 = { 1, { 2, 3, 4 } }; // #1
> > > > > 
> > > > > struct f2 {
> > > > >   struct f1 f1; int data[3];
> > > > > } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> > > > > 
> > > > > struct { char x[]; } r = {1};  // #3
> > > > > ```
> > > > > 
> > > > > gcc rejects 2 and 3 even though 2 comes from their documentation. 
> > > > > Clang warns on 2 and MSVC rejects 2
> > > > > 
> > > > > CC @aaron.ballman @rsmith 
> > > > Yes, I also had a feeling that we probably need to refine how these 
> > > > extensions are supported by clang, that is probably a bit out of scope 
> > > > of the fix though.
> > > The GCC extension differs based on C vs C++: 
> > > https://godbolt.org/z/E14Yz37To
> > > As does the extension in Clang, but differently than GCC: 
> > > https://godbolt.org/z/zYznaYPf5
> > > 
> > > So I agree that there's work to be done on this extension, but it's 
> > > outside of the scope of the crash fix for this patch. For this patch, GCC 
> > > rejects allowing a flexible array member in a union in both C and C++, 
> > > but it looks like Clang rejects in C and perhaps accepts in C++: 
> > > https://godbolt.org/z/bTsPn3G7b So how about we add a C++ test case for 
> > > this as well -- if that still crashes, that should be fixed, but if the 
> > > code is accepted, we should either decide we want to start rejecting it 
> > > or we should ensure the codegen for it is correct.
> > While experimenting with C++ test, I've noticed the following thing:
> > ```
> > union { char x[]; } r = {0}; // when in global scope generates no IR 
> > (without my patch crashes)
> > 
> > void foo() {
> >   union { char x[]; } r = {0}; // fails with error "initialization of 
> > flexible array member is not allowed" in both C/C++, no crash even without 
> > my patch
> > }
> > 
> > union A {char x[]; };
> > A a = {0}; // crashes even with my patch but with different assertion when 
> > trying -emit-llvm
> > void foo() {
> >   A a = {0}; // fails with error "initialization of flexible array member 
> > is not allowed" in both C and C++, no crash even without my patch
> > }
> > ```
> > 
> > It is not entirely clear to me why the behavior different for function and 
> > TU scope. gcc always gives an error about flexible array in union, no 
> > matter how it is used. Also, it is strange that I'm not seeing the same 
> > "initialization of flexible array member is not allowed" error message for 
> > structs, just for unions. I'm thinking that we don't really have proper 
> > codegen for the code that I'm trying to fix and we should reject the code 
> > like gcc does. MSVC is fine with all though - 
> > https://godbolt.org/z/aoarEzd56 .
> > 
> > WDYT?
> Ping?
Ouch, that's a rather confused situation, isn't it? I don't have the impression 
we actually support this extension and we do not document support for it in the 
language extensions page, so due to the crashing I'm leaning towards diagnosing 
this code as invalid per the standards. That doesn't mean someone can't come 
along and support this extension if they need it to work, but the current state 
of things is not production-ready. Any disagreement with that?

(If we go that route, we should definitely follow the potentially breaking 
changes policy 
(https://llvm.org/docs/DeveloperPolicy.html#making-potentially-break

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D148515: [Modules] Do not rewrite existing decls when deserializing class fields

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ChuanqiXu.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Classes can have implicit members that were added before fields were
deserialized. These members were previously silently removed from
`decls()` when fields were deserialized after them.

This was the root cause of a compilation error exposed in
bc95f27337c7ed77c28e713c855272848f01802a 
, added a 
test for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148515

Files:
  clang/lib/AST/Decl.cpp
  clang/test/Modules/pr61065_2.cppm


Index: clang/test/Modules/pr61065_2.cppm
===
--- /dev/null
+++ clang/test/Modules/pr61065_2.cppm
@@ -0,0 +1,71 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface -o %t/d.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fsyntax-only -verify 
-fprebuilt-module-path=%t
+
+//--- a.cppm
+export module a;
+
+struct WithCtor {
+  WithCtor();
+};
+
+export template 
+struct Getter {
+  union {
+WithCtor container;
+  };
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+export template 
+class AnySpan {
+ public:
+  AnySpan();
+  AnySpan(Getter getter)
+  : getter_(getter) {}
+
+ private:
+  Getter getter_;
+};
+
+//--- c.cppm
+export module c;
+import b;
+
+export inline void RegisterInt322(
+   AnySpan sibling_field_nums) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- d.cppm
+// expected-no-diagnostics
+export module d;
+import c;
+import b;
+
+export inline void RegisterInt32(
+   AnySpan sibling_field_nums = {}) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- e.cpp
+import d;
+import b;
+
+// expected-no-diagnostics
+void foo(AnySpan s) {
+  s = AnySpan(s);
+}
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4886,8 +4886,12 @@
   if (Decls.empty())
 return;
 
-  std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls,
+  auto [ExternalFirst, ExternalLast] = BuildDeclChain(Decls,
  
/*FieldsAlreadyLoaded=*/false);
+  ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  FirstDecl = ExternalFirst;
+  if (!LastDecl)
+LastDecl = ExternalLast;
 }
 
 bool RecordDecl::mayInsertExtraPadding(bool EmitRemark) const {


Index: clang/test/Modules/pr61065_2.cppm
===
--- /dev/null
+++ clang/test/Modules/pr61065_2.cppm
@@ -0,0 +1,71 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface -o %t/d.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fsyntax-only -verify -fprebuilt-module-path=%t
+
+//--- a.cppm
+export module a;
+
+struct WithCtor {
+  WithCtor();
+};
+
+export template 
+struct Getter {
+  union {
+WithCtor container;
+  };
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+export template 
+class AnySpan {
+ public:
+  AnySpan();
+  AnySpan(Getter getter)
+  : getter_(getter) {}
+
+ private:
+  Getter getter_;
+};
+
+//--- c.cppm
+export module c;
+import b;
+
+export inline void RegisterInt322(
+   AnySpan sibling_field_nums) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- d.cppm
+// expected-no-diagnostics
+export module d;
+import c;
+import b;
+
+export inline void RegisterInt32(
+   AnySpan sibling_field_nums = {}) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- e.cpp
+import d;
+import b;
+
+// expected-no-diagnostics
+void foo(AnySpan s) {
+  s = AnySpan(s);
+}
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4886,8 +4886,12 @@
   if (Decls.empty())
 return;
 
-  std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls,
+  auto [ExternalFirst, ExternalLast] = BuildDeclChain(Decls,
  /*FieldsAlreadyLoaded=*/false);
+ 

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> Regarding the comment that I must not change existing tests: I think this 
> rule is too strict, because those tests are mostly regression tests. 
> But a regression tests does not test for correctness. So if a test had 
> already a wrong assumption, it must be changeable.

I don't agree, you are making the assumption that the default behaviour should 
now be different from what is was before, we have 100,000+ of users did you 
canvas them to ask if we wanted your suggested correct behaviour? (especially 
when someone has split their includes into a separate include group? I assume 
for a reason)

What would be their recourse if they didn't want the duplicate removed across 
groups, I don't see one? are you expecting them to // clang-format on/off

Ultimately we try not to change the default behaviour, if we think the 
behaviour is useful (and this could be), we ask that its put behind a new 
"Option" and that by default its off.

`SortIncludes` was latterly considered a contentious addition and has been 
proved to alter code in a well publicized example, in hindsight most people 
think it should have been off by default. I don't like making assumptions that 
we should turn something on, that others might not want/need or desire. This 
feels like one of those and something that could break code.

I personally follow a Beyoncé rule when it comes to unit tests... "If I like it 
I should have put a test on it", if the test is there, I need to be persuaded 
the test is very wrong before I'm happy to let it be changed to a new behaviour.

FWIW, these are not just regression tests, they assert that the behaviour is as 
the author desired and that is considered correctness until proven otherwise, 
any new author needs to respect that, or prove its a genuine bug and not just a 
matter of style/opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143870

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

[PATCH] D148426: [clang][Interp] IntegralComplexToBoolean casts

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a minor cleanup.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1165
+return visit(LV);
+
   return false;

tbaeder wrote:
> Since complex types are arrays with two elements, we can't dereference them 
> like normal. This change just ignore the dereference.
> 
> I was wondering if it would be better to add an opcode to dereference it so 
> we have `[real, imag]` on the stack and then callers that operate on such 
> complex rvalues can `pop(); pop();` to get the values.
> I was wondering if it would be better to add an opcode to dereference it so 
> we have [real, imag] on the stack and then callers that operate on such 
> complex rvalues can pop(); pop(); to get the values.

That might be worthwhile as an optimization, but I suppose we can wait to see 
how often complex is used in constant expressions before deciding what to do 
there?



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:272-275
+assert(T->isAnyComplexType());
+
+QualType ElemType = T->getAs()->getElementType();
+

This form does the assertion for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148426

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

[PATCH] D148515: [Modules] Do not rewrite existing decls when deserializing class fields

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I plan to land this together with a reland of 
bc95f27337c7ed77c28e713c855272848f01802a 
 and 
finally close GH61065 .
I have checked this works on previously broken targets. @ChuanqiXu sorry for 
taking so long to track it down, reproducing module-related errors is quite 
hard in our setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148515

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

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:808
   unsigned NumElems = numStructUnionElements(ILE->getType());
-  if (RDecl->hasFlexibleArrayMember())
+  if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
 ++NumElems;

aaron.ballman wrote:
> Fznamznon wrote:
> > Fznamznon wrote:
> > > aaron.ballman wrote:
> > > > Fznamznon wrote:
> > > > > shafik wrote:
> > > > > > Fznamznon wrote:
> > > > > > > Just for some context, numStructUnionElements checks that there 
> > > > > > > is a flexible array member and returns 
> > > > > > > number_of_initializable_fields-1 for structs. For unions it just 
> > > > > > > returns 1 or 0, so flexible array member caused adding one more 
> > > > > > > element to initlistexpr that was never properly handled.
> > > > > > > 
> > > > > > > Instead of doing this change, we could probably never enter 
> > > > > > > initialization since the record (union) declaration is not valid, 
> > > > > > > but that is not the case even for other types of errors in code, 
> > > > > > > for example, I've tried declaring field of struct with a typo:
> > > > > > > 
> > > > > > > ```
> > > > > > > struct { cha x[]; } r = {1}; 
> > > > > > > ```
> > > > > > > Initialization is still performed by clang.
> > > > > > > Also, it seems MSVC considers flexible array member inside union 
> > > > > > > as valid, so the test code is probably not always invalid.
> > > > > > I am not sure what to think here, looking at gcc documentation for 
> > > > > > this extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> > > > > > 
> > > > > > and using the following code:
> > > > > > 
> > > > > > ```
> > > > > > struct f1 {
> > > > > >   int x; int y[];
> > > > > > } f1 = { 1, { 2, 3, 4 } }; // #1
> > > > > > 
> > > > > > struct f2 {
> > > > > >   struct f1 f1; int data[3];
> > > > > > } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> > > > > > 
> > > > > > struct { char x[]; } r = {1};  // #3
> > > > > > ```
> > > > > > 
> > > > > > gcc rejects 2 and 3 even though 2 comes from their documentation. 
> > > > > > Clang warns on 2 and MSVC rejects 2
> > > > > > 
> > > > > > CC @aaron.ballman @rsmith 
> > > > > Yes, I also had a feeling that we probably need to refine how these 
> > > > > extensions are supported by clang, that is probably a bit out of 
> > > > > scope of the fix though.
> > > > The GCC extension differs based on C vs C++: 
> > > > https://godbolt.org/z/E14Yz37To
> > > > As does the extension in Clang, but differently than GCC: 
> > > > https://godbolt.org/z/zYznaYPf5
> > > > 
> > > > So I agree that there's work to be done on this extension, but it's 
> > > > outside of the scope of the crash fix for this patch. For this patch, 
> > > > GCC rejects allowing a flexible array member in a union in both C and 
> > > > C++, but it looks like Clang rejects in C and perhaps accepts in C++: 
> > > > https://godbolt.org/z/bTsPn3G7b So how about we add a C++ test case for 
> > > > this as well -- if that still crashes, that should be fixed, but if the 
> > > > code is accepted, we should either decide we want to start rejecting it 
> > > > or we should ensure the codegen for it is correct.
> > > While experimenting with C++ test, I've noticed the following thing:
> > > ```
> > > union { char x[]; } r = {0}; // when in global scope generates no IR 
> > > (without my patch crashes)
> > > 
> > > void foo() {
> > >   union { char x[]; } r = {0}; // fails with error "initialization of 
> > > flexible array member is not allowed" in both C/C++, no crash even 
> > > without my patch
> > > }
> > > 
> > > union A {char x[]; };
> > > A a = {0}; // crashes even with my patch but with different assertion 
> > > when trying -emit-llvm
> > > void foo() {
> > >   A a = {0}; // fails with error "initialization of flexible array member 
> > > is not allowed" in both C and C++, no crash even without my patch
> > > }
> > > ```
> > > 
> > > It is not entirely clear to me why the behavior different for function 
> > > and TU scope. gcc always gives an error about flexible array in union, no 
> > > matter how it is used. Also, it is strange that I'm not seeing the same 
> > > "initialization of flexible array member is not allowed" error message 
> > > for structs, just for unions. I'm thinking that we don't really have 
> > > proper codegen for the code that I'm trying to fix and we should reject 
> > > the code like gcc does. MSVC is fine with all though - 
> > > https://godbolt.org/z/aoarEzd56 .
> > > 
> > > WDYT?
> > Ping?
> Ouch, that's a rather confused situation, isn't it? I don't have the 
> impression we actually support this extension and we do not document support 
> for it in the language extensions page, so due to the crashing I'm leaning 
> towards diagnosing this code as invalid per the standards. That doesn't mean 
> someone can't come along and support this extension if they need it to work, 
> but the current state of things is 

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, though the changes need a release note. Do you still need someone to 
commit on your behalf? (Alternatively, you could consider requesting commit 
access: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148419

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

[clang] 9d9046f - [clang] Do not crash after suggesting typo correction to constexpr if condition

[Mariya Podchishchaeva via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2023-04-17T08:28:49-04:00
New Revision: 9d9046f06d55692c5be51164694a4959d9e212b2

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

LOG: [clang] Do not crash after suggesting typo correction to constexpr if 
condition

In some cases non-null non-constant yet valid expression may reach point where
`ConditionResult` is created. For example, typo correction mechanism can return
such expression, so double check before evaluating it.

Fixes https://github.com/llvm/llvm-project/issues/61885

Reviewed By: tbaeder, aaron.ballman

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

Added: 
clang/test/SemaCXX/invalid-if-constexpr.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1f66a877c50b4..4fb1160713def 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -307,6 +307,8 @@ Bug Fixes in This Version
   bit-fields. This fixes:
   (`#61355 `_) and
   (`#61417 `_)
+- Fix crash after suggesting typo correction to constexpr if condition.
+  (`#61885 `_)
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c12660081e7c4..b05238d0352e6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12843,20 +12843,22 @@ class Sema final {
 Decl *ConditionVar;
 FullExprArg Condition;
 bool Invalid;
-bool HasKnownValue;
-bool KnownValue;
+std::optional KnownValue;
 
 friend class Sema;
 ConditionResult(Sema &S, Decl *ConditionVar, FullExprArg Condition,
 bool IsConstexpr)
-: ConditionVar(ConditionVar), Condition(Condition), Invalid(false),
-  HasKnownValue(IsConstexpr && Condition.get() &&
-!Condition.get()->isValueDependent()),
-  KnownValue(HasKnownValue &&
- !!Condition.get()->EvaluateKnownConstInt(S.Context)) {}
+: ConditionVar(ConditionVar), Condition(Condition), Invalid(false) {
+  if (IsConstexpr && Condition.get()) {
+if (std::optional Val =
+Condition.get()->getIntegerConstantExpr(S.Context)) {
+  KnownValue = !!(*Val);
+}
+  }
+}
 explicit ConditionResult(bool Invalid)
 : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
-  HasKnownValue(false), KnownValue(false) {}
+  KnownValue(std::nullopt) {}
 
   public:
 ConditionResult() : ConditionResult(false) {}
@@ -12865,11 +12867,7 @@ class Sema final {
   return std::make_pair(cast_or_null(ConditionVar),
 Condition.get());
 }
-std::optional getKnownValue() const {
-  if (!HasKnownValue)
-return std::nullopt;
-  return KnownValue;
-}
+std::optional getKnownValue() const { return KnownValue; }
   };
   static ConditionResult ConditionError() { return ConditionResult(true); }
 

diff  --git a/clang/test/SemaCXX/invalid-if-constexpr.cpp 
b/clang/test/SemaCXX/invalid-if-constexpr.cpp
new file mode 100644
index 0..7643c47488f05
--- /dev/null
+++ b/clang/test/SemaCXX/invalid-if-constexpr.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+namespace GH61885 {
+void similar() { // expected-note {{'similar' declared here}}
+  if constexpr (similer<>) {} // expected-error {{use of undeclared identifier 
'similer'; did you mean 'similar'?}}
+}
+void a() { if constexpr (__adl_swap<>) {}} // expected-error{{use of 
undeclared identifier '__adl_swap'; did you mean '__sync_swap'?}} \
+   // expected-note {{'__sync_swap' 
declared here}}
+
+int AA() { return true;} // expected-note {{'AA' declared here}}
+
+void b() { if constexpr (AAA<>) {}} // expected-error {{use of undeclared 
identifier 'AAA'; did you mean 'AA'?}}
+}
+



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

[PATCH] D148206: [clang] Do not crash after suggesting typo correction to constexpr if condition

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d9046f06d55: [clang] Do not crash after suggesting typo 
correction to constexpr if condition (authored by Fznamznon).

Changed prior to commit:
  https://reviews.llvm.org/D148206?vs=513497&id=514174#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148206

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/test/SemaCXX/invalid-if-constexpr.cpp


Index: clang/test/SemaCXX/invalid-if-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/invalid-if-constexpr.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+namespace GH61885 {
+void similar() { // expected-note {{'similar' declared here}}
+  if constexpr (similer<>) {} // expected-error {{use of undeclared identifier 
'similer'; did you mean 'similar'?}}
+}
+void a() { if constexpr (__adl_swap<>) {}} // expected-error{{use of 
undeclared identifier '__adl_swap'; did you mean '__sync_swap'?}} \
+   // expected-note {{'__sync_swap' 
declared here}}
+
+int AA() { return true;} // expected-note {{'AA' declared here}}
+
+void b() { if constexpr (AAA<>) {}} // expected-error {{use of undeclared 
identifier 'AAA'; did you mean 'AA'?}}
+}
+
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12843,20 +12843,22 @@
 Decl *ConditionVar;
 FullExprArg Condition;
 bool Invalid;
-bool HasKnownValue;
-bool KnownValue;
+std::optional KnownValue;
 
 friend class Sema;
 ConditionResult(Sema &S, Decl *ConditionVar, FullExprArg Condition,
 bool IsConstexpr)
-: ConditionVar(ConditionVar), Condition(Condition), Invalid(false),
-  HasKnownValue(IsConstexpr && Condition.get() &&
-!Condition.get()->isValueDependent()),
-  KnownValue(HasKnownValue &&
- !!Condition.get()->EvaluateKnownConstInt(S.Context)) {}
+: ConditionVar(ConditionVar), Condition(Condition), Invalid(false) {
+  if (IsConstexpr && Condition.get()) {
+if (std::optional Val =
+Condition.get()->getIntegerConstantExpr(S.Context)) {
+  KnownValue = !!(*Val);
+}
+  }
+}
 explicit ConditionResult(bool Invalid)
 : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
-  HasKnownValue(false), KnownValue(false) {}
+  KnownValue(std::nullopt) {}
 
   public:
 ConditionResult() : ConditionResult(false) {}
@@ -12865,11 +12867,7 @@
   return std::make_pair(cast_or_null(ConditionVar),
 Condition.get());
 }
-std::optional getKnownValue() const {
-  if (!HasKnownValue)
-return std::nullopt;
-  return KnownValue;
-}
+std::optional getKnownValue() const { return KnownValue; }
   };
   static ConditionResult ConditionError() { return ConditionResult(true); }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -307,6 +307,8 @@
   bit-fields. This fixes:
   (`#61355 `_) and
   (`#61417 `_)
+- Fix crash after suggesting typo correction to constexpr if condition.
+  (`#61885 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/invalid-if-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/invalid-if-constexpr.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+namespace GH61885 {
+void similar() { // expected-note {{'similar' declared here}}
+  if constexpr (similer<>) {} // expected-error {{use of undeclared identifier 'similer'; did you mean 'similar'?}}
+}
+void a() { if constexpr (__adl_swap<>) {}} // expected-error{{use of undeclared identifier '__adl_swap'; did you mean '__sync_swap'?}} \
+   // expected-note {{'__sync_swap' declared here}}
+
+int AA() { return true;} // expected-note {{'AA' declared here}}
+
+void b() { if constexpr (AAA<>) {}} // expected-error {{use of undeclared identifier 'AAA'; did you mean 'AA'?}}
+}
+
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12843,20 +12843,22 @@
 Decl *ConditionVar;
 FullExprArg Condition;
 bool Invalid;
-bool HasKnownValue;
-bool KnownValue;
+std::optional KnownValue;
 

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/crash-lambda-weak-attr.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+

I don't think we need this to be in C++2b mode explicitly, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148419

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

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet updated this revision to Diff 514179.
hazohelet added a comment.

Address comments from @aaron.ballman

- Added release note
- Do not specify std c++ version in test code


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

https://reviews.llvm.org/D148419

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/crash-lambda-weak-attr.cpp


Index: clang/test/SemaCXX/crash-lambda-weak-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Weak {
+[[gnu::weak]]void weak_method();
+};
+static_assert([](){ return &Weak::weak_method != nullptr; }()); // 
expected-error {{static assertion expression is not an integral constant 
expression}} \
+// 
expected-note {{comparison against pointer to weak member 'Weak::weak_method' 
can only be performed at runtime}} \
+// 
expected-note {{in call to}}
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13142,12 +13142,12 @@
 if (LHSValue.getDecl() && LHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << LHSValue.getDecl();
-  return true;
+  return false;
 }
 if (RHSValue.getDecl() && RHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << RHSValue.getDecl();
-  return true;
+  return false;
 }
 
 // C++11 [expr.eq]p2:
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -309,6 +309,8 @@
   (`#61417 `_)
 - Fix crash after suggesting typo correction to constexpr if condition.
   (`#61885 `_)
+- Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
+  member pointer as an invalid expression.
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/crash-lambda-weak-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Weak {
+[[gnu::weak]]void weak_method();
+};
+static_assert([](){ return &Weak::weak_method != nullptr; }()); // expected-error {{static assertion expression is not an integral constant expression}} \
+// expected-note {{comparison against pointer to weak member 'Weak::weak_method' can only be performed at runtime}} \
+// expected-note {{in call to}}
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13142,12 +13142,12 @@
 if (LHSValue.getDecl() && LHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << LHSValue.getDecl();
-  return true;
+  return false;
 }
 if (RHSValue.getDecl() && RHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << RHSValue.getDecl();
-  return true;
+  return false;
 }
 
 // C++11 [expr.eq]p2:
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -309,6 +309,8 @@
   (`#61417 `_)
 - Fix crash after suggesting typo correction to constexpr if condition.
   (`#61885 `_)
+- Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
+  member pointer as an invalid expression.
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added a comment.

ping for reviews. Thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

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

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Takuya Shimizu via Phabricator via cfe-commits]

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

In D148419#4273551 , @aaron.ballman 
wrote:

> LGTM, though the changes need a release note. Do you still need someone to 
> commit on your behalf? (Alternatively, you could consider requesting commit 
> access: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

Since I don't have commit access, I would like you to commit this on my behalf. 
Please use "Takuya Shimizu " as patch attribution.
I'll try requesting commit access. Thanks!




Comment at: clang/test/SemaCXX/crash-lambda-weak-attr.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+

aaron.ballman wrote:
> I don't think we need this to be in C++2b mode explicitly, right?
Yeah, we don't need it


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

https://reviews.llvm.org/D148419

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

[PATCH] D145581: [clang-tidy] In C++17, callee is guaranteed to be sequenced before arguments.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 514182.
mboehme added a comment.

Changes in reponse to review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145581

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1,3 +1,4 @@
+// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
 // RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
@@ -123,6 +124,7 @@
   A &operator=(A &&);
 
   void foo() const;
+  void bar(int i) const;
   int getInt() const;
 
   operator bool() const;
@@ -552,6 +554,19 @@
   std::move(a);
 }
   }
+  // Same as above, but the use and the move are in different CFG blocks.
+  {
+A a;
+for (int i = 0; i < 10; ++i) {
+  if (i < 10)
+a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:9: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE+3]]:9: note: move occurred here
+  // CHECK-NOTES: [[@LINE-3]]:9: note: the use happens in a later loop
+  if (i < 10)
+std::move(a);
+}
+  }
   // However, this case shouldn't be flagged -- the scope of the declaration of
   // 'a' is important.
   {
@@ -1307,6 +1322,40 @@
   }
 }
 
+// In a function call, the expression that determines the callee is sequenced
+// before the arguments -- but only in C++17 and later.
+namespace CalleeSequencedBeforeArguments {
+int consumeA(std::unique_ptr a);
+int consumeA(A &&a);
+
+void calleeSequencedBeforeArguments() {
+  {
+std::unique_ptr a;
+a->bar(consumeA(std::move(a)));
+// CHECK-NOTES-CXX11: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES-CXX11: [[@LINE-2]]:21: note: move occurred here
+// CHECK-NOTES-CXX11: [[@LINE-3]]:5: note: the use and move are unsequenced
+  }
+  {
+std::unique_ptr a;
+std::unique_ptr getArg(std::unique_ptr a);
+getArg(std::move(a))->bar(a->getInt());
+// CHECK-NOTES: [[@LINE-1]]:31: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:12: note: move occurred here
+// CHECK-NOTES-CXX11: [[@LINE-3]]:31: note: the use and move are unsequenced
+  }
+  {
+A a;
+// Nominally, the callee `a.bar` is evaluated before the argument
+// `consumeA(std::move(a))`, but in effect `a` is only accessed after the
+// call to `A::bar()` happens, i.e. after the argument has been evaluted.
+a.bar(consumeA(std::move(a)));
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+}
+} // namespace CalleeSequencedBeforeArguments
+
 // Some statements in templates (e.g. null, break and continue statements) may
 // be shared between the uninstantiated and instantiated versions of the
 // template and therefore have multiple parents. Make sure the sequencing code
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -219,9 +219,12 @@
   ` check when warning would be
   emitted in constructor for virtual base class initialization.
 
-- Improved :doc:`bugprone-use-after-move
-  ` to understand that there is a
-  sequence point between designated initializers.
+- In :doc:`bugprone-use-after-move
+  `:
+  - Fixed handling for designated initializers.
+  - Fix: In C++17 and later, the callee of a function is guaranteed to be
+sequenced before the arguments, so don't warn if the use happens in the
+callee and the move happens in one of the arguments.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -63,6 +63,25 @@
   return false;
 }
 
+bool isDescendantOfArgs(const Stmt *Descendant, const CallExpr *Call,
+ASTContext *Context) {
+  for (const Expr *Arg : Call->arguments()) {
+if (isDescendantOrEqual(Descendant, Arg, Context))
+  return true;
+  }
+
+  return false;
+}
+
+bool argsContain(const CallExpr *Call, const Stmt *TheStmt) {
+  for (const Expr *Arg : Call->arguments()) {
+if (Arg == TheStmt)
+  return true;
+  }
+
+  return false

[PATCH] D145581: [clang-tidy] In C++17, callee is guaranteed to be sequenced before arguments.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 514188.
mboehme added a comment.

Changes in response to review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145581

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1,3 +1,4 @@
+// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
 // RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
@@ -123,6 +124,7 @@
   A &operator=(A &&);
 
   void foo() const;
+  void bar(int i) const;
   int getInt() const;
 
   operator bool() const;
@@ -552,6 +554,19 @@
   std::move(a);
 }
   }
+  // Same as above, but the use and the move are in different CFG blocks.
+  {
+A a;
+for (int i = 0; i < 10; ++i) {
+  if (i < 10)
+a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:9: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE+3]]:9: note: move occurred here
+  // CHECK-NOTES: [[@LINE-3]]:9: note: the use happens in a later loop
+  if (i < 10)
+std::move(a);
+}
+  }
   // However, this case shouldn't be flagged -- the scope of the declaration of
   // 'a' is important.
   {
@@ -1307,6 +1322,40 @@
   }
 }
 
+// In a function call, the expression that determines the callee is sequenced
+// before the arguments -- but only in C++17 and later.
+namespace CalleeSequencedBeforeArguments {
+int consumeA(std::unique_ptr a);
+int consumeA(A &&a);
+
+void calleeSequencedBeforeArguments() {
+  {
+std::unique_ptr a;
+a->bar(consumeA(std::move(a)));
+// CHECK-NOTES-CXX11: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES-CXX11: [[@LINE-2]]:21: note: move occurred here
+// CHECK-NOTES-CXX11: [[@LINE-3]]:5: note: the use and move are unsequenced
+  }
+  {
+std::unique_ptr a;
+std::unique_ptr getArg(std::unique_ptr a);
+getArg(std::move(a))->bar(a->getInt());
+// CHECK-NOTES: [[@LINE-1]]:31: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:12: note: move occurred here
+// CHECK-NOTES-CXX11: [[@LINE-3]]:31: note: the use and move are unsequenced
+  }
+  {
+A a;
+// Nominally, the callee `a.bar` is evaluated before the argument
+// `consumeA(std::move(a))`, but in effect `a` is only accessed after the
+// call to `A::bar()` happens, i.e. after the argument has been evaluted.
+a.bar(consumeA(std::move(a)));
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+}
+} // namespace CalleeSequencedBeforeArguments
+
 // Some statements in templates (e.g. null, break and continue statements) may
 // be shared between the uninstantiated and instantiated versions of the
 // template and therefore have multiple parents. Make sure the sequencing code
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -162,6 +162,13 @@
   ` check.
   Global options of the same name should be used instead.
 
+- In :doc:`bugprone-use-after-move
+  `:
+  - Fixed handling for designated initializers.
+  - Fix: In C++17 and later, the callee of a function is guaranteed to be
+sequenced before the arguments, so don't warn if the use happens in the
+callee and the move happens in one of the arguments.
+
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`google-build-namespaces
   ` check.
@@ -219,10 +226,6 @@
   ` check when warning would be
   emitted in constructor for virtual base class initialization.
 
-- Improved :doc:`bugprone-use-after-move
-  ` to understand that there is a
-  sequence point between designated initializers.
-
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -63,6 +63,25 @@
   return false;
 }
 
+bool isDescendantOfArgs(const Stmt *Descendant, const CallExpr *Call,
+ASTContext *Context) {
+  for (const Expr *Arg : Call->arguments()) {
+if (isDescendantOrEqual(Descendant, Arg, Context))
+  return true;
+  

[PATCH] D148525: [OpenMP][AMDGPU] Refactor setting uniform work group size attribute

[Dominik Adamski via Phabricator via cfe-commits]

domada created this revision.
domada added reviewers: jsjodin, ronlieb, dpalermo, JonChesterfield, agozillon, 
gregrodgers, skatrak, raghavendhra, RogerV-AMD, saiislam.
domada added projects: OpenMP, AMDGPU.
Herald added subscribers: sunshaoce, kosarev, guansong, hiraditya, tpr, 
dstuttard, yaxunl, kzhuravl.
Herald added a project: All.
domada requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jplehr, sstefan1, wdng.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.

Work group size attribute was set in Clang specific class. That's why we cannot 
reuse this code in Flang.

If we move setting of this attribute to OpenMPIRBuilder, then we can reuse this 
code in Flang and Clang. Function `createOffloadEntry` from OpenMPIRBuilder is 
already used by Clang (via 
`OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata` function).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148525

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4892,6 +4892,8 @@
 
   // Add a function attribute for the kernel.
   Fn->addFnAttr(Attribute::get(Ctx, "kernel"));
+  if (Triple(M.getTargetTriple()).isAMDGCN())
+Fn->addFnAttr("uniform-work-group-size", "true");
 }
 
 // We only generate metadata for function that contain target regions.
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9588,12 +9588,9 @@
 
   const bool IsHIPKernel =
   M.getLangOpts().HIP && FD && FD->hasAttr();
-  const bool IsOpenMPkernel =
-  M.getLangOpts().OpenMPIsDevice &&
-  (F->getCallingConv() == llvm::CallingConv::AMDGPU_KERNEL);
 
   // TODO: This should be moved to language specific attributes instead.
-  if (IsHIPKernel || IsOpenMPkernel)
+  if (IsHIPKernel)
 F->addFnAttr("uniform-work-group-size", "true");
 
   if (M.getContext().getTargetInfo().allowAMDGPUUnsafeFPAtomics())


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4892,6 +4892,8 @@
 
   // Add a function attribute for the kernel.
   Fn->addFnAttr(Attribute::get(Ctx, "kernel"));
+  if (Triple(M.getTargetTriple()).isAMDGCN())
+Fn->addFnAttr("uniform-work-group-size", "true");
 }
 
 // We only generate metadata for function that contain target regions.
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9588,12 +9588,9 @@
 
   const bool IsHIPKernel =
   M.getLangOpts().HIP && FD && FD->hasAttr();
-  const bool IsOpenMPkernel =
-  M.getLangOpts().OpenMPIsDevice &&
-  (F->getCallingConv() == llvm::CallingConv::AMDGPU_KERNEL);
 
   // TODO: This should be moved to language specific attributes instead.
-  if (IsHIPKernel || IsOpenMPkernel)
+  if (IsHIPKernel)
 F->addFnAttr("uniform-work-group-size", "true");
 
   if (M.getContext().getTargetInfo().allowAMDGPUUnsafeFPAtomics())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

In D146358#4268327 , @tbaeder wrote:

> So, if I understand the code correctly, we call `CheckEvaluationResult` with 
> `SubObjectDecl=nullptr` when we're not checking an actual field but just an 
> array/record, so we can't run into this problem anyway, so the assert seems 
> fine.
>
> I don't fully understand the problem with the `gnu::weak` stuff, in any case, 
> adding it to  this patch seems wrong. https://godbolt.org/z/qn997n85n 
> //does// emit a note, but it's not the one this patch is about, so is that 
> even relevant?

Thanks. I opened another differential for the `gnu::weak` stuff (D148419 
).
If my understanding is correct, `Info.FFDiag` displays only the note of its 
first call and discard the notes after it. So, the uninitialized-note is not 
displayed.
You can confirm this by deleting only the FFDiag call 
(https://github.com/llvm/llvm-project/blob/6f7e5c0f1ac6cc3349a2e1479ac4208465b272c6/clang/lib/AST/ExprConstant.cpp#L13143-L13144)
 from the clang trunk and compile my godbolt code to see the note (`note: 
subobject of type 'const bool' is not initialized`)
Although the `return true` says the `[[gnu::weak]]` member pointer comparison 
is valid, constexpr engine considers the constant initialization has failed if 
there is note emitted during the initializer 
evaluation(https://github.com/llvm/llvm-project/blob/6f7e5c0f1ac6cc3349a2e1479ac4208465b272c6/clang/lib/AST/Decl.cpp#L2591-L2592)
 and it's why `[[gnu::weak]]` comparison note is actually emitted in spite of 
the wrong signal about the validness of the expression.


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

https://reviews.llvm.org/D146358

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

[PATCH] D145581: [clang-tidy] In C++17, callee is guaranteed to be sequenced before arguments.

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 3 inline comments as done.
mboehme added a comment.

In D145581#4215602 , @PiotrZSL wrote:

> Switching status of review, once you will be ready with changes (or your 
> decision), just mark it ready for review again.

Did I do this correctly? It says "Needs Review" now, though I think I didn't do 
anything specific to trigger this.

In D145581#4223185 , @PiotrZSL wrote:

> And actually there is issue for this: 
> https://github.com/llvm/llvm-project/issues/57758

Thanks, I'll update this once this change is submitted.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:222
 
-- Improved :doc:`bugprone-use-after-move
-  ` to understand that there is a
-  sequence point between designated initializers.
+- In :doc:`bugprone-use-after-move
+  `:

Eugene.Zelenko wrote:
> Please keep alphabetical order (by check name) in this section.
> Please keep alphabetical order (by check name) in this section.

Done.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1304
+  std::unique_ptr a;
+  a->foo(std::move(a));
+}

PiotrZSL wrote:
> mboehme wrote:
> > PiotrZSL wrote:
> > > mboehme wrote:
> > > > PiotrZSL wrote:
> > > > > What about scenario like this:
> > > > > 
> > > > > ```
> > > > > b.foo(a->saveBIntoAAndReturnBool(std::move(b)));
> > > > > ```
> > > > > 
> > > > > Is first "b" still guaranteed to be alive after std::move ?
> > > > I'm not exactly sure what you're asking here... or how this scenario is 
> > > > materially different from the other scenarios we already have?
> > > > 
> > > > > Is first "b" still guaranteed to be alive after std::move ?
> > > > 
> > > > The `b` in `b.foo` is guaranteed to be evaluated before the call 
> > > > `a->saveBIntoAAndReturnBool(std::move(b))` -- but I'm not sure if this 
> > > > is what you're asking?
> > > > 
> > > > Or are you asking whether the 
> > > > `a->saveBIntoAAndReturnBool(std::move(b))` can cause the underlying 
> > > > object to be destroyed before the call to `b.foo` happenss? In other 
> > > > words, do we potentially have a use-after-free here?
> > > > 
> > > > I think the answer to this depends on what exactly 
> > > > `saveBIntoAAndReturnBool()` does (what was your intent here?). I also 
> > > > think it's probably beyond the scope of this check in any case, as this 
> > > > check is about diagnosing use-after-move, not use-after-free.
> > > I see this ```b.foo(a->saveBIntoAAndReturnBool(std::move(b)));``` like 
> > > this:
> > > we call saveBIntoAAndReturnBool, that takes b by std::move, then we call 
> > > foo on already moved object.
> > > For me this is use after move, that's why I was asking.
> > > 
> > > And in "b.foo" there is almost nothing to evaluate, maybe address of foo, 
> > > but at the end foo will be called on already moved object.
> > > If we would have something like "getSomeObj(b).boo(std::move(b))" then we 
> > > can think about "evaluate", but when we directly call method on moved 
> > > object, then we got use after move
> > > 
> > > 
> > Ah, I think I understand what you're getting at now. I was assuming for 
> > some reason that `b` was also a `unique_ptr` in this example, but of course 
> > that doesn't make sense because in that case we wouldn't be able to use the 
> > dot operator on `b` (i.e. `b.foo`).
> > 
> > Distinguishing between these two cases will require making the check more 
> > sophisticated -- the logic that the callee is sequenced before the 
> > arguments is not sufficient on its own. I'll have to take a closer look at 
> > how to do this, but it will likely involve looking at the `MemberExpr` 
> > inside the `CXXMemberCallExpr`. If `MemberExpr::getBase()` is simply a 
> > `DeclRefExpr`, we'll want to do one thing, and if `MemberExpr::getBase()` 
> > is some sort of `CallExpr`, we'll want to do something else. There will 
> > likely need to be other considerations involved as well, but I wanted to 
> > sketch out in broad lines where I think this should go.
> > 
> > I'll likely take a few days to turn this around, but in the meantime I 
> > wanted to get this comment out to let you know that I now understand the 
> > issue.
> Yes but that's not so easy, as there can be thing like:
> `x.y.foo(std::move(x));`
> 
> To be honest probably easiest way would be to extract isIdenticalStmt from 
> clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp, and then we could 
> just check if callExpr callee contain argument of std::move, and that 
> argument does not contain any other callExpr before current one.
> For such cases we could warn, but for all other cases when there any other 
> sub callExpr involved we woudn't need to wanr.
> 
> to be honest I need isIdenticalStmt for my other checks, so if you decide to 
> go this route do this under separate patch.
> 
> Reasn why I mention isIdenticalStmt is because this woul

[clang] e98776a - [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via cfe-commits]

Author: Nikolas Klauser
Date: 2023-04-17T15:36:21+02:00
New Revision: e98776a180a74c08dcc07cebf26c11deac6e975a

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

LOG: [clang] Add __is_trivially_equality_comparable

This patch adds a new trait to allow standard libraries to forward `std::equal` 
calls to `memcmp` in more cases.

Reviewed By: aaron.ballman

Spies: Mordante, shafik, xbolva00, libcxx-commits, cfe-commits, ldionne

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Type.h
clang/include/clang/Basic/TokenKinds.def
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/type-traits.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index cda08066c6ab2..ddd366b637e59 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1507,6 +1507,9 @@ The following type trait primitives are supported by 
Clang. Those traits marked
   functionally equivalent to copying the underlying bytes and then dropping the
   source object on the floor. This is true of trivial types and types which
   were made trivially relocatable via the ``clang::trivial_abi`` attribute.
+* ``__is_trivially_equality_comparable`` (Clang): Returns true if comparing two
+  objects of the provided type is known to be equivalent to comparing their
+  value representations.
 * ``__is_unbounded_array`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_unsigned`` (C++, Embarcadero):
@@ -4928,7 +4931,7 @@ Note, all of debugging pragmas are subject to change.
 `dump`
 --
 Accepts either a single identifier or an expression. When a single identifier 
is passed,
-the lookup results for the identifier are printed to `stderr`. When an 
expression is passed, 
+the lookup results for the identifier are printed to `stderr`. When an 
expression is passed,
 the AST for the expression is printed to `stderr`. The expression is an 
unevaluated operand,
 so things like overload resolution and template instantiations are performed,
 but the expression has no runtime effects.

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4fb1160713def..9c8a0e1faa326 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -150,6 +150,9 @@ Non-comprehensive list of changes in this release
 - Clang now supports expressions in ``#pragma clang __debug dump``.
 - Clang now supports declaration of multi-dimensional arrays with
   ``__declspec(property)``.
+- A new builtin type trait ``__is_trivially_equaltiy_comparable`` has been 
added,
+  which checks whether comparing two instances of a type is equivalent to
+  ``memcmp(&lhs, &rhs, sizeof(T)) == 0``.
 
 New Compiler Flags
 --
@@ -287,7 +290,7 @@ Bug Fixes in This Version
 - Fix crash when handling nested immediate invocations in initializers of 
global
   variables.
   (`#58207 `_)
-- Fix crash when generating code coverage information for `PseudoObjectExpr` 
in 
+- Fix crash when generating code coverage information for `PseudoObjectExpr` in
   Clang AST.
   (`#45481 `_)
 - Fix the assertion hit when a template consteval function appears in a nested

diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 26626192e338f..54b41d248a341 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2492,7 +2492,9 @@ class ASTContext : public RefCountedBase {
 
   /// Return true if the specified type has unique object representations
   /// according to (C++17 [meta.unary.prop]p9)
-  bool hasUniqueObjectRepresentations(QualType Ty) const;
+  bool
+  hasUniqueObjectRepresentations(QualType Ty,
+ bool CheckIfTriviallyCopyable = true) const;
 
   
//======//
   //Type Operators

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9ecc29bd38fd1..af0e724311017 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -899,6 +899,9 @@ class QualType {
   /// Return true if this is a trivially relocatable type.
   bool isTriviallyRelocatableType(const ASTContext &Context) const;
 
+  /// Return true if this is a trivially equality comparable type.
+  bool isTriviallyEqualityComparableType(const ASTContext &Context) const;
+
  

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe98776a180a7: [clang] Add __is_trivially_equality_comparable 
(authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++17 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
 
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
@@ -570,7 +571,11 @@
   static_assert(__is_aggregate(DerivesAr), "");
   static_assert(__is_aggregate(DerivesArNB), "");
   static_assert(!__is_aggregate(HasCons), "");
+#if __cplusplus >= 202002L
+  static_assert(!__is_aggregate(HasDefaultCons), "");
+#else
   static_assert(__is_aggregate(HasDefaultCons), "");
+#endif
   static_assert(!__is_aggregate(HasExplicitDefaultCons), "");
   static_assert(!__is_aggregate(HasInheritedCons), "");
   static_assert(__is_aggregate(HasNoInheritedCons) == TrueAfterCpp14, "");
@@ -3098,6 +3103,168 @@
 
 } // namespace is_trivially_relocatable
 
+namespace is_trivially_equality_comparable {
+struct ForwardDeclared; // expected-note {{forward declaration of 'is_trivially_equality_comparable::ForwardDeclared'}}
+static_assert(!__is_trivially_equality_comparable(ForwardDeclared), ""); // expected-error {{incomplete type 'ForwardDeclared' used in type trait expression}}
+
+static_assert(!__is_trivially_equality_comparable(void), "");
+static_assert(__is_trivially_equality_comparable(int), "");
+static_assert(!__is_trivially_equality_comparable(int[]), "");
+static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(float), "");
+static_assert(!__is_trivially_equality_comparable(double), "");
+static_assert(!__is_trivially_equality_comparable(long double), "");
+
+struct TriviallyEqualityComparableNoDefaultedComparator {
+  int i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparableNoDefaultedComparator), "");
+
+#if __cplusplus >= 202002L
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  bool operator==(const TriviallyEqualityComparable&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasFloat&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasTailPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+
+public:
+  bool operator==(const TriviallyEqualityComparablePaddedOutBase&) const = def

[PATCH] D148529: [clang] Replace find_executable with shutil.which in creduce script

[David Spickett via Phabricator via cfe-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

distutils is deprecated and shutil.which is the suggested
replacement for this function.

https://peps.python.org/pep-0632/#migration-advice
https://docs.python.org/3/library/shutil.html#shutil.which

which was added in 3.3 
(https://docs.python.org/3/library/shutil.html#shutil.which)
and LLVM requires at least 3.6 
(https://llvm.org/docs/GettingStarted.html#software).

There is one small differnce here that shutil.which ignores the PATH
when given a path argument. However in this case I think that's actually
the behaviour we want.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148529

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -11,6 +11,7 @@
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
+import shutil
 import stat
 import sys
 import subprocess
@@ -18,7 +19,6 @@
 import shlex
 import tempfile
 import shutil
-from distutils.spawn import find_executable
 import multiprocessing
 
 verbose = False
@@ -43,12 +43,12 @@
   if cmd_path:
 # Make the path absolute so the creduce test can be run from any directory.
 cmd_path = os.path.abspath(cmd_path)
-cmd = find_executable(cmd_path)
+cmd = shutil.which(cmd_path)
 if cmd:
   return cmd
 sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
-  cmd = find_executable(cmd_name, path=cmd_dir)
+  cmd = shutil.which(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
 


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -11,6 +11,7 @@
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
+import shutil
 import stat
 import sys
 import subprocess
@@ -18,7 +19,6 @@
 import shlex
 import tempfile
 import shutil
-from distutils.spawn import find_executable
 import multiprocessing
 
 verbose = False
@@ -43,12 +43,12 @@
   if cmd_path:
 # Make the path absolute so the creduce test can be run from any directory.
 cmd_path = os.path.abspath(cmd_path)
-cmd = find_executable(cmd_path)
+cmd = shutil.which(cmd_path)
 if cmd:
   return cmd
 sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
-  cmd = find_executable(cmd_name, path=cmd_dir)
+  cmd = shutil.which(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a4edc2c - Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Aaron Ballman via cfe-commits]

Author: Takuya Shimizu
Date: 2023-04-17T09:50:46-04:00
New Revision: a4edc2c9fa35a763fc5f4c9cf6383096a13a9cf6

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

LOG: Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons 
as evaluation failure

This patch fixes the wrong signal from the constexpr evaluator that
[[gnu::weak]] member pointer comparison is valid, while it is emitting
notes on them.

I found a crashing case fixed by this change and added it as a test
case: https://godbolt.org/z/8391fGjGn

I noticed this while I was working on D146358.

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

Added: 
clang/test/SemaCXX/crash-lambda-weak-attr.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c8a0e1faa32..05189bfb6aba 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -312,6 +312,8 @@ Bug Fixes in This Version
   (`#61417 `_)
 - Fix crash after suggesting typo correction to constexpr if condition.
   (`#61885 `_)
+- Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
+  member pointer as an invalid expression.
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fbe92d054d6b..6bfb3a37b243 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13142,12 +13142,12 @@ EvaluateComparisonBinaryOperator(EvalInfo &Info, 
const BinaryOperator *E,
 if (LHSValue.getDecl() && LHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << LHSValue.getDecl();
-  return true;
+  return false;
 }
 if (RHSValue.getDecl() && RHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << RHSValue.getDecl();
-  return true;
+  return false;
 }
 
 // C++11 [expr.eq]p2:

diff  --git a/clang/test/SemaCXX/crash-lambda-weak-attr.cpp 
b/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
new file mode 100644
index ..28a516942393
--- /dev/null
+++ b/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Weak {
+[[gnu::weak]]void weak_method();
+};
+static_assert([](){ return &Weak::weak_method != nullptr; }()); // 
expected-error {{static assertion expression is not an integral constant 
expression}} \
+// 
expected-note {{comparison against pointer to weak member 'Weak::weak_method' 
can only be performed at runtime}} \
+// 
expected-note {{in call to}}
+



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

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Aaron Ballman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa4edc2c9fa35: Constexpr evaluator should treat [[gnu::weak]] 
member pointer comparisons as… (authored by hazohelet, committed by 
aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148419

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/crash-lambda-weak-attr.cpp


Index: clang/test/SemaCXX/crash-lambda-weak-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Weak {
+[[gnu::weak]]void weak_method();
+};
+static_assert([](){ return &Weak::weak_method != nullptr; }()); // 
expected-error {{static assertion expression is not an integral constant 
expression}} \
+// 
expected-note {{comparison against pointer to weak member 'Weak::weak_method' 
can only be performed at runtime}} \
+// 
expected-note {{in call to}}
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13142,12 +13142,12 @@
 if (LHSValue.getDecl() && LHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << LHSValue.getDecl();
-  return true;
+  return false;
 }
 if (RHSValue.getDecl() && RHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << RHSValue.getDecl();
-  return true;
+  return false;
 }
 
 // C++11 [expr.eq]p2:
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -312,6 +312,8 @@
   (`#61417 `_)
 - Fix crash after suggesting typo correction to constexpr if condition.
   (`#61885 `_)
+- Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
+  member pointer as an invalid expression.
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/crash-lambda-weak-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Weak {
+[[gnu::weak]]void weak_method();
+};
+static_assert([](){ return &Weak::weak_method != nullptr; }()); // expected-error {{static assertion expression is not an integral constant expression}} \
+// expected-note {{comparison against pointer to weak member 'Weak::weak_method' can only be performed at runtime}} \
+// expected-note {{in call to}}
+
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13142,12 +13142,12 @@
 if (LHSValue.getDecl() && LHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << LHSValue.getDecl();
-  return true;
+  return false;
 }
 if (RHSValue.getDecl() && RHSValue.getDecl()->isWeak()) {
   Info.FFDiag(E, diag::note_constexpr_mem_pointer_weak_comparison)
   << RHSValue.getDecl();
-  return true;
+  return false;
 }
 
 // C++11 [expr.eq]p2:
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -312,6 +312,8 @@
   (`#61417 `_)
 - Fix crash after suggesting typo correction to constexpr if condition.
   (`#61885 `_)
+- Clang constexpr evaluator now treats comparison of [[gnu::weak]]-attributed
+  member pointer as an invalid expression.
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Takuya Shimizu via Phabricator via cfe-commits]

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

@aaron.ballman 
Sorry, this change is breaking the buildbot and because I don't have commit 
access I would like you to revert it for me.
It seems we need to specify the std option in the test code.

  BUILD FAILED: 41435 expected passes 84 expected failures 28291 unsupported 
tests 1 unexpected failures (failure)
  
  Step 5 (build-unified-tree) warnings: Build unified tree
  
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/lib/CodeGen/MachineModuleInfo.cpp:73:51:
 warning: ‘this’ pointer is null [-Wnonnull]
  
  Step 6 (test-build-unified-tree-check-all) failure: 41435 expected passes 84 
expected failures 28291 unsupported tests 1 unexpected failures (failure)
   TEST 'Clang :: SemaCXX/crash-lambda-weak-attr.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/clang 
-cc1 -internal-isystem 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/lib/clang/17/include
 -nostdsysteminc -fsyntax-only -verify 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  error: 'error' diagnostics seen but not expected: 
File 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
 Line 6: a lambda expression may not appear inside of a constant expression
  error: 'warning' diagnostics seen but not expected: 
File 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
 Line 6: 'static_assert' with no message is a C++17 extension
  error: 'note' diagnostics expected but not seen: 
File 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
 Line 6: comparison against pointer to weak member 'Weak::weak_method' can only 
be performed at runtime
File 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
 Line 6: in call to
  error: 'note' diagnostics seen but not expected: 
File 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
 Line 6: non-literal type '(lambda at 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp:6:15)'
 cannot be used in a constant expression
  5 errors generated.
  
  --
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148419

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

[clang] 56f7052 - Speculatively fix a failing bot

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-04-17T10:10:00-04:00
New Revision: 56f7052d9226838b745970c4156be30ee209ee03

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

LOG: Speculatively fix a failing bot

This attempts to resolve the issue found by:
https://lab.llvm.org/buildbot/#/builders/139/builds/39296

Added: 


Modified: 
clang/test/SemaCXX/crash-lambda-weak-attr.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/crash-lambda-weak-attr.cpp 
b/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
index 28a516942393..c3866930092f 100644
--- a/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
+++ b/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
 
 struct Weak {
 [[gnu::weak]]void weak_method();
@@ -6,4 +6,3 @@ struct Weak {
 static_assert([](){ return &Weak::weak_method != nullptr; }()); // 
expected-error {{static assertion expression is not an integral constant 
expression}} \
 // 
expected-note {{comparison against pointer to weak member 'Weak::weak_method' 
can only be performed at runtime}} \
 // 
expected-note {{in call to}}
-



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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:227
+(TSTy = Ty->getAs()))
+  Result.addOuterTemplateArguments(const_cast(FTD),
+   TSTy->template_arguments(),

So I'd come up with something similar, but this ends up being a little goofy?  
And I thought it didn't really work in 1 of the cases.  I wonder if we're 
better off looking at the decl contexts to try to figure out WHICH of the cases 
we are, and just set the next decl context based on that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D147844: Emit warning when implicit cast to bool happens in an conditional operator expression when used inside an overloaded shift operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav reclaimed this revision.
chaitanyav added a comment.

re-opening the revision to make further updates as per comments on the github 
issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148419: [clang][AST] Constexpr evaluator should treat [[gnu::weak]] member pointer comparisons as evaluation failure

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148419#4273901 , @hazohelet wrote:

> @aaron.ballman 
> Sorry, this change is breaking the buildbot and because I don't have commit 
> access I would like you to revert it for me.
> It seems we need to specify the std option in the test code.
>
>   BUILD FAILED: 41435 expected passes 84 expected failures 28291 unsupported 
> tests 1 unexpected failures (failure)
>   
>   Step 5 (build-unified-tree) warnings: Build unified tree
>   
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/lib/CodeGen/MachineModuleInfo.cpp:73:51:
>  warning: ‘this’ pointer is null [-Wnonnull]
>   
>   Step 6 (test-build-unified-tree-check-all) failure: 41435 expected passes 
> 84 expected failures 28291 unsupported tests 1 unexpected failures (failure)
>    TEST 'Clang :: SemaCXX/crash-lambda-weak-attr.cpp' 
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/clang
>  -cc1 -internal-isystem 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/lib/clang/17/include
>  -nostdsysteminc -fsyntax-only -verify 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>   --
>   Exit Code: 1
>   
>   Command Output (stderr):
>   --
>   error: 'error' diagnostics seen but not expected: 
> File 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>  Line 6: a lambda expression may not appear inside of a constant expression
>   error: 'warning' diagnostics seen but not expected: 
> File 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>  Line 6: 'static_assert' with no message is a C++17 extension
>   error: 'note' diagnostics expected but not seen: 
> File 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>  Line 6: comparison against pointer to weak member 'Weak::weak_method' can 
> only be performed at runtime
> File 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>  Line 6: in call to
>   error: 'note' diagnostics seen but not expected: 
> File 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp
>  Line 6: non-literal type '(lambda at 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/SemaCXX/crash-lambda-weak-attr.cpp:6:15)'
>  cannot be used in a constant expression
>   5 errors generated.
>   
>   --
>   
>   

I noticed that as well and corrected the issue in 
a4edc2c9fa35a763fc5f4c9cf6383096a13a9cf6 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148419

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

[PATCH] D147844: Emit warning when implicit cast to bool happens in an conditional operator expression when used inside an overloaded shift operator expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! The only thing I think is missing is a release 
note, though I did have a question on a change in the tests.




Comment at: clang/test/Sema/parentheses.cpp:34
 public:
-  operator int();
+  operator bool();
   Stream &operator<<(int);

Is this change necessary? Alternatively, can we add this as an overload instead 
of changing the operator?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't see a problem with this, but the changes to the pragma tests concern me 
a bit, so I'd like aaron to scroll through.  The Expr.cpp and Sema changes both 
look right to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D148211: [clang][tests] Fix Flang driver tests for Windows

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc added a comment.

`clang/test/Driver/flang/multiple-inputs-mixed.f90` uses a more general 
FileCheck pattern:

  ! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"

Perhaps these files should use the same regexp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148211

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

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
   if (sysroot != "") {
 CmdArgs.push_back("-syslibroot");
 CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));

ldionne wrote:
> ldionne wrote:
> > Where do we set the usual `/usr/lib` search path? I think it might 
> > make sense to move the code you added to that place.
> What I mean: there's a place where we must be adding `-L /usr/lib` 
> in the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it? 
> Would it make sense to move the code you added to that place instead?
/usr/local/include is added here in 
```DarwinClang::AddClangSystemIncludeArgs```: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2346
 and /usr/include is added here: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2374
 .. But since we want to add a new argument to the linker invocation, I feel 
```darwin::Linker::AddLinkArgs``` is the best place. But let me know if you 
think otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148266

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

[Krzysztof Drewniak via Phabricator via cfe-commits]

krzysz00 updated this revision to Diff 514232.
krzysz00 added a comment.

Fix new merge conflicts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-rtn.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-non-integral-address-spaces.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.atomic.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2darraymsaa.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.3d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.store.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.atomic.dim.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.add.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.i8.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.add.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f16.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.load.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.sample.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/addrspacecast-captured.ll
  llvm/test/CodeGen/AMDGPU/annotate-kernel-features-hsa.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.v2f16-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-intrinsics-mmo-offsets.ll
  llvm/test/

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

hans wrote:
> zequanwu wrote:
> > hans wrote:
> > > Won't the code above (line 580) make many filenames absolute and cause us 
> > > to use native slashes even when we want backslashes?
> > > 
> > > This would also need a test.
> > > Won't the code above (line 580) make many filenames absolute and cause us 
> > > to use native slashes even when we want backslashes?
> > Yes, I don't think we should change anything if it's converted to an 
> > absolute path.
> > 
> > Only if the `-fdebug-compilation-dir` is not given or is an absolute path, 
> > the path in `/Fo` will be converted to absolute path. Users can avoid the 
> > path being converted to absolute path by passing a relative path to 
> > `-fdebug-compilation-dir`, just like what we do in chrome build 
> > (`-fdebug-compilation-dir=.`).
> > 
> > Added a test.
> Apologies for all the questions, but I find the logic a bit complex still. 
> The test cases are helpful though.
> 
> Could we simplify by just always using Windows slashes here, regardless of 
> `-fdebug-compilation-dir` etc.?
The object file path will be converted to absolute path depending on whether 
`-fdebug-compilation-dir` is given or whether it's absolute path. I don't think 
we want to change a native absolute path of object file to use Windows slashes. 
Or this could happen on linux: `/usr/local/src/a.obj` -> 
`\usr\local\src\a.obj`. So, we only use windows slashes when it's not an 
absolute path here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[clang-tools-extra] f099f2f - [clangd] Use all inputs to SystemIncludeExtractor in cache key

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-04-17T16:57:33+02:00
New Revision: f099f2fefbab0592c828573d816b46a475076f49

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

LOG: [clangd] Use all inputs to SystemIncludeExtractor in cache key

Instead of passing in a tooling::CompileCommand into system include
extraction, pass a limited set, whose elements are used as keys.

Also fix the issue around accepting `-isysroot=/foo` which isn't a valid
argument (or the directory should be `=/foo` not `/foo`).

Fixes https://github.com/clangd/clangd/issues/1404
Fixes https://github.com/clangd/clangd/issues/1403

This should also unblock https://reviews.llvm.org/D138546

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

Added: 


Modified: 
clang-tools-extra/clangd/SystemIncludeExtractor.cpp
clang-tools-extra/clangd/test/system-include-extractor.test

Removed: 




diff  --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp 
b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index c334697e1e238..4d359b0058280 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -32,32 +32,44 @@
 #include "CompileCommands.h"
 #include "GlobalCompilationDatabase.h"
 #include "support/Logger.h"
-#include "support/Path.h"
+#include "support/Threading.h"
 #include "support/Trace.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/ScopedPrinter.h"
-#include 
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 #include 
-#include 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
-namespace clang {
-namespace clangd {
+namespace clang::clangd {
 namespace {
 
 struct DriverInfo {
@@ -65,12 +77,138 @@ struct DriverInfo {
   std::string Target;
 };
 
+struct DriverArgs {
+  // Name of the driver program to execute or absolute path to it.
+  std::string Driver;
+  // Whether certain includes should be part of query.
+  bool StandardIncludes = true;
+  bool StandardCXXIncludes = true;
+  bool BuiltinIncludes = true;
+  // Language to use while querying.
+  std::string Lang;
+  std::string Sysroot;
+  std::string ISysroot;
+
+  bool operator==(const DriverArgs &RHS) const {
+return std::tie(Driver, StandardIncludes, StandardCXXIncludes,
+BuiltinIncludes, Lang, Sysroot, ISysroot) ==
+   std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
+RHS.BuiltinIncludes, RHS.Lang, RHS.Sysroot, ISysroot);
+  }
+
+  DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
+llvm::SmallString<128> Driver(Cmd.CommandLine.front());
+// Driver is a not a single executable name but instead a path (either
+// relative or absolute).
+if (llvm::any_of(Driver,
+ [](char C) { return llvm::sys::path::is_separator(C); })) 
{
+  llvm::sys::fs::make_absolute(Cmd.Directory, Driver);
+}
+this->Driver = Driver.str().str();
+for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) {
+  llvm::StringRef Arg = Cmd.CommandLine[I];
+
+  // Look for Language related flags.
+  if (Arg.consume_front("-x")) {
+if (Arg.empty() && I + 1 < E)
+  Lang = Cmd.CommandLine[I + 1];
+else
+  Lang = Arg.str();
+  }
+  // Look for standard/builtin includes.
+  else if (Arg == "-nostdinc" || Arg == "--no-standard-includes")
+StandardIncludes = false;
+  else if (Arg == "-nostdinc++")
+StandardCXXIncludes = false;
+  else if (Arg == "-nobuiltininc")
+BuiltinIncludes = false;
+  // Figure out sysroot
+  else if (Arg.consume_front("--sysroot")) {
+if (Arg.consume_front("="))
+  Sysroot = Arg.str();
+else if (Arg.empty() && I + 1 < E)
+  Sysroot = Cmd.CommandLine[I + 1];
+  } else if (Arg.consume_front("-isysroot")) {
+if (Arg.empty() && I + 1 < E)
+  ISysroot = Cmd.CommandLine[I + 1];
+   

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf099f2fefbab: [clangd] Use all inputs to 
SystemIncludeExtractor in cache key (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146941

Files:
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test

Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -14,8 +14,8 @@
 # Check that clangd preserves certain flags like `-nostdinc` from
 # original invocation in compile_commands.json.
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> %t.dir/bin/my_driver.sh
-# RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-isysroot /isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
@@ -32,7 +32,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -70,7 +70,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -32,32 +32,44 @@
 #include "CompileCommands.h"
 #include "GlobalCompilationDatabase.h"
 #include "support/Logger.h"
-#include "support/Path.h"
+#include "support/Threading.h"
 #include "support/Trace.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/ScopedPrinter.h"
-#include 
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 #include 
-#include 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
-namespace clang {
-namespace clangd {
+namespace clang::clangd {
 namespace {
 
 struct DriverInfo {
@@ -65,12 +77,138 @@
   std::string Target;
 };
 
+struct DriverArgs {
+  // Name of the driver program to execute or absolute path to it.
+  std::string Driver;
+  // Whether certain includes should be part of query.
+  bool StandardIncludes = true;
+  bool StandardCXXIncludes = true;
+  bool BuiltinIncludes = true;
+  // Language to use while querying.
+  std::string Lang;
+  std::string Sysroot;
+  std::string ISysroot;
+
+  bool operator==(const DriverArgs &RHS) const {
+return std::tie(Driver, StandardIncludes, StandardCXXIncludes,
+BuiltinIncludes, Lang, Sysroot, ISysroot) ==
+   st

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

[Joseph Huber via Phabricator via cfe-commits]

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

LGTM unless anyone else has any concerns.




Comment at: clang/test/OpenMP/target_team_variable_codegen.cpp:33
+//.
+// CHECK-NVIDIA: @local_a = internal addrspace(3) global [10 x i32] 
zeroinitializer, align 4
+//.

doru1004 wrote:
> jhuber6 wrote:
> > jdoerfert wrote:
> > > doru1004 wrote:
> > > > jhuber6 wrote:
> > > > > Shouldn't the Nvidia version also be undefined? Not sure why this 
> > > > > should vary depending on the target.
> > > > Perhaps NVIDIA code path can tolerate a zeroinitializer? I don't want 
> > > > to change it if it's not needed. I am basing this check on the code 
> > > > path for AMD GPUs and the initial bug that was reported.
> > > for AS 3 we should make it always poison.
> > We should probably change this in `HeadToShared` in `OpenMPOpt` as well.
> Happy to remove the guard and have it always use poison for both NVIDIA and 
> AMD.
These should be a single check line now.


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

https://reviews.llvm.org/D147572

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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:189-190
   preprocessor operators now return 1 also for attributes defined by plugins.
+- In C++, the ``__attribute__((warn_unused))`` can now also be used on 
individual
+  constructors,
 





Comment at: clang/include/clang/Basic/Attr.td:2996
   let Spellings = [GCC<"warn_unused">];
-  let Subjects = SubjectList<[Record]>;
+  let Subjects = SubjectList<[Record, CXXConstructor]>;
   let Documentation = [Undocumented];

I'm confused -- if you want this to behave like `nodiscard`, why aren't these 
changes for `warn_unused_result` (which is what implements `nodiscard`)? I 
don't think this should go on `warn_unused` because that's for the declaration 
of the type as a unit and not written on functions.

I think you don't need any changes to Attr.td for this functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D148529: [clang] Replace find_executable with shutil.which in creduce script

[Zequan Wu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148529

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

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D144634#4272857 , @koops wrote:

> 1. Adding semantic test clang/test/OpenMP/loop_bind_messages.cpp.
> 2. Changes suggested by Alexey.
> 3. >Why need to drop bind clause here? The new Directives to which loop 
> directive is being mapped to, do not contain the bind clause. (a) omp loop 
> bind(parallel)  > omp for (b) omp loop bind(teams)  ->  omp 
> distribute (c) omp loop bind(thread)  --> omp simd

But why need to drop it? It makes processing more complex. The bind clause 
itself should not be a problem, no?




Comment at: clang/lib/Sema/SemaOpenMP.cpp:638
   }
+  void setCurrentDirective(OpenMPDirectiveKind newDK) {
+SharingMapTy *Top = (SharingMapTy *)getTopOfStackOrNull();

NewDK



Comment at: clang/lib/Sema/SemaOpenMP.cpp:640
+SharingMapTy *Top = (SharingMapTy *)getTopOfStackOrNull();
+assert(Top != NULL);
+Top->Directive = newDK;

Add message to the assert.



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:2-3
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ %s -emit-llvm -o - | 
FileCheck %s

In other tests, this is used for testing with precompiled headers. You don't 
have this in your test runs currently.



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:8-13
+/*
+#include 
+#include 
+#include 
+#include 
+*/

Remove this commented code too



Comment at: clang/test/OpenMP/loop_bind_messages.cpp:1-2
+#ifndef HEADER
+#define HEADER
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=50 
-verify %s

1. No need for this in semantic test
2, More tests required, nesting of regions, more tests for incompatible clauses.


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

https://reviews.llvm.org/D144634

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

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Also, add AST printing tests


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

https://reviews.llvm.org/D144634

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

[PATCH] D148484: [clang-format] Correctly format goto labels followed by blocks

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 514249.
sstwcw added a comment.

- Remove change in line wrapping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148484

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1621,7 +1621,7 @@
 "x;\n"
 "endcase\n");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
-  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GotoLabelColon);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_CaseLabelColon);
   Tokens = Annotate("case (x)\n"
 "  x ? x : x:\n"
 "x;\n"
@@ -1629,7 +1629,7 @@
   ASSERT_EQ(Tokens.size(), 14u) << Tokens;
   EXPECT_TOKEN(Tokens[5], tok::question, TT_ConditionalExpr);
   EXPECT_TOKEN(Tokens[7], tok::colon, TT_ConditionalExpr);
-  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GotoLabelColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_CaseLabelColon);
   // Non-blocking assignments.
   Tokens = Annotate("a <= b;");
   ASSERT_EQ(Tokens.size(), 5u) << Tokens;
@@ -1724,6 +1724,21 @@
   EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
+  auto Tokens = annotate("{ x: break; }");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+  Tokens = annotate("{ case x: break; }");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+  Tokens = annotate("{ x: { break; } }");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+  Tokens = annotate("{ case x: { break; } }");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2998,6 +2998,17 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label: { some_other_code(); }\n"
+   "}");
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label: {\n"
+   "  some_other_code();\n"
+   "  some_other_code();\n"
+   "}\n"
+   "}");
   FormatStyle Style = getLLVMStyle();
   Style.IndentGotoLabels = false;
   verifyFormat("void f() {\n"
@@ -3022,6 +3033,23 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label: { some_other_code(); }\n"
+   "}",
+   Style);
+  // The opening brace may either be on the same unwrapped line as the colon or
+  // on a separate one. The formatter should recognize both.
+  Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Allman;
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "{\n"
+   "  some_other_code();\n"
+   "}\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, MultiLineControlStatements) {
@@ -16777,6 +16805,19 @@
"}\n"
"}",
CaseStyle);
+  // Goto labels should not be affected.
+  verifyFormat("switch (x) {\n"
+   "goto_label:\n"
+   "default :\n"
+   "}",
+   CaseStyle);
+  verifyFormat("switch (x) {\n"
+   "goto_label: { break; }\n"
+   "default : {\n"
+   "  break;\n"
+   "}\n"
+   "}",
+   CaseStyle);
 
   FormatStyle NoSpaceStyle = getLLVMStyle();
   EXPECT_EQ(NoSpaceStyle.SpaceBeforeCaseColon, false);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1493,6 +1493,7 @@
 }
 nextToken();
 if (FormatTok->is(tok::colon)) {
+  FormatTok->setFinalizedType(TT_CaseLabelColon);
   parseLabel();
   return;
 }
@@ -1925,6 +1926,7 @@
 if (!Style.isVerilog() && FormatTok->is(tok::colon) &&
 !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
+  FormatTok->setFinalizedType(

[PATCH] D148484: [clang-format] Correctly format goto labels followed by blocks

[sstwcw via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:4525
-const FormatToken *Next = Right.getNextNonComment();
-if (!Next || Next->is(tok::semi))
   return false;

MyDeveloperDay wrote:
> how is the semi case handled or is it not needed
It is not needed anymore.  It was added to handle a semicolon following a goto 
label. Now the type `TT_GotoLabelColon` includes the case.



Comment at: clang/unittests/Format/FormatTest.cpp:3020-3024
   verifyFormat("{\n"
"  some_code();\n"
-   "test_label:;\n"
-   "  int i = 0;\n"
-   "}");

HazardyKnusperkeks wrote:
> Why did you remove that?
It was an accident.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148484

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

[PATCH] D148529: [clang] Replace find_executable with shutil.which in creduce script

[David Spickett via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee3413736251: [clang] Replace find_executable with 
shutil.which in creduce script (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148529

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -11,6 +11,7 @@
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
+import shutil
 import stat
 import sys
 import subprocess
@@ -18,7 +19,6 @@
 import shlex
 import tempfile
 import shutil
-from distutils.spawn import find_executable
 import multiprocessing
 
 verbose = False
@@ -43,12 +43,12 @@
   if cmd_path:
 # Make the path absolute so the creduce test can be run from any directory.
 cmd_path = os.path.abspath(cmd_path)
-cmd = find_executable(cmd_path)
+cmd = shutil.which(cmd_path)
 if cmd:
   return cmd
 sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
-  cmd = find_executable(cmd_name, path=cmd_dir)
+  cmd = shutil.which(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
 


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -11,6 +11,7 @@
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
+import shutil
 import stat
 import sys
 import subprocess
@@ -18,7 +19,6 @@
 import shlex
 import tempfile
 import shutil
-from distutils.spawn import find_executable
 import multiprocessing
 
 verbose = False
@@ -43,12 +43,12 @@
   if cmd_path:
 # Make the path absolute so the creduce test can be run from any directory.
 cmd_path = os.path.abspath(cmd_path)
-cmd = find_executable(cmd_path)
+cmd = shutil.which(cmd_path)
 if cmd:
   return cmd
 sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
-  cmd = find_executable(cmd_name, path=cmd_dir)
+  cmd = shutil.which(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ee34137 - [clang] Replace find_executable with shutil.which in creduce script

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2023-04-17T15:33:06Z
New Revision: ee341373625163846f4ebc68e46aec6fb46c2c09

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

LOG: [clang] Replace find_executable with shutil.which in creduce script

distutils is deprecated and shutil.which is the suggested
replacement for this function.

https://peps.python.org/pep-0632/#migration-advice
https://docs.python.org/3/library/shutil.html#shutil.which

which was added in 3.3 
(https://docs.python.org/3/library/shutil.html#shutil.which)
and LLVM requires at least 3.6 
(https://llvm.org/docs/GettingStarted.html#software).

There is one small differnce here that shutil.which ignores the PATH
when given a path argument. However in this case I think that's actually
the behaviour we want.

Reviewed By: zequanwu

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

Added: 


Modified: 
clang/utils/creduce-clang-crash.py

Removed: 




diff  --git a/clang/utils/creduce-clang-crash.py 
b/clang/utils/creduce-clang-crash.py
index 77bd4cf0fbfcf..fa3bd470ef725 100755
--- a/clang/utils/creduce-clang-crash.py
+++ b/clang/utils/creduce-clang-crash.py
@@ -11,6 +11,7 @@
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
+import shutil
 import stat
 import sys
 import subprocess
@@ -18,7 +19,6 @@
 import shlex
 import tempfile
 import shutil
-from distutils.spawn import find_executable
 import multiprocessing
 
 verbose = False
@@ -43,12 +43,12 @@ def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   if cmd_path:
 # Make the path absolute so the creduce test can be run from any directory.
 cmd_path = os.path.abspath(cmd_path)
-cmd = find_executable(cmd_path)
+cmd = shutil.which(cmd_path)
 if cmd:
   return cmd
 sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
-  cmd = find_executable(cmd_name, path=cmd_dir)
+  cmd = shutil.which(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
 



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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet updated this revision to Diff 514252.
hazohelet added a comment.

Remove `gnu::weak` diff


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

https://reviews.llvm.org/D146358

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/test/AST/Interp/cxx20.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -761,7 +761,7 @@
 };
 
 S s1; // expected-error {{call to consteval function 'NamespaceScopeConsteval::S::S' is not a constant expression}} \
- expected-note {{subobject of type 'int' is not initialized}}
+ expected-note {{subobject 'Val' is not initialized}}
 
 template 
 struct T {
@@ -770,7 +770,7 @@
 };
 
 T t; // expected-error {{call to consteval function 'NamespaceScopeConsteval::T::T' is not a constant expression}} \
- expected-note {{subobject of type 'int' is not initialized}}
+ expected-note {{subobject 'Val' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
 
Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -409,12 +409,12 @@
 b.a.x = 2;
 return b;
   }
-  constexpr B uninit = return_uninit(); // expected-error {{constant expression}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr B uninit = return_uninit(); // expected-error {{constant expression}} expected-note {{subobject 'y' is not initialized}}
   static_assert(return_uninit().a.x == 2);
   constexpr A return_uninit_struct() {
 B b = {.b = 1};
 b.a.x = 2;
-return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note {{subobject of type 'int' is not initialized}}
+return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note {{subobject 'y' is not initialized}}
   }
   // Note that this is rejected even though return_uninit() is accepted, and
   // return_uninit() copies the same stuff wrapped in a union.
@@ -558,7 +558,7 @@
 }
   };
   constinit X x1(true);
-  constinit X x2(false); // expected-error {{constant initializer}} expected-note {{constinit}} expected-note {{subobject of type 'int' is not initialized}}
+  constinit X x2(false); // expected-error {{constant initializer}} expected-note {{constinit}} expected-note {{subobject 'n' is not initialized}}
 
   struct Y {
 struct Z { int n; }; // expected-note {{here}}
@@ -577,7 +577,7 @@
   };
   // FIXME: This is working around clang not implementing DR2026. With that
   // fixed, we should be able to test this without the injected copy.
-  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} expected-note {{subobject 'n' is not initialized}}
   constexpr Y y1 = copy(Y());
   static_assert(y1.z1.n == 1 && y1.z2.n == 2 && y1.z3.n == 3);
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -359,7 +359,7 @@
   // The constructor is still 'constexpr' here, but the result is not intended
   // to be a constant expression. The standard is not clear on how this should
   // work.
-  constexpr V v; // expected-error {{constant expression}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr V v; // expected-error {{constant expression}} expected-note {{subobject 'y' is not initialized}}
 
   constexpr int k = V().x; // FIXME: ok?
 }
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -143,9 +143,9 @@
 constexpr A() {}
   };
   constexpr A a; // expected-error {{must be initialized by a constant expression}} \
- // expected-note {{subobject of type 'int' is not initialized}} \
+ // expected-note {{subobject 'a' is not initialized}} \
  // ref-error {{must be initialized by a constant expression}} \
- // ref-note {{subobject of type 'int' is not initialized}}
+ // ref-note {{subobject 'a' is not initialized}}
 
 
   class Base {
@@ -161,18 +161,18 @@
 constexpr Derived() : Base() {}   };
 
   constexpr Derived D; // expected-error {{must be initialized by a constant expression}} \
-  

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added subscribers: aaron.ballman, rsmith.
shafik added inline comments.



Comment at: clang/test/CXX/drs/dr18xx.cpp:139
 
+namespace dr1890 { // dr1890: no drafting
+// FIXME: all the examples are well-formed.

So this is still in drafting: 
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1890

We saw this in Issaquah and we felt like 1890 would probably be resolved once 
we resolve 2335: 
https://www.open-std.org/JTC1/SC22/WG21/docs/cwg_active.html#2335

I think we are leaning towards these being well-formed but we won't really know 
till it is resolved. 



Comment at: clang/test/CXX/drs/dr23xx.cpp:42
 
+namespace dr2335 { // dr2335: no drafting
+// FIXME: all of the examples are well-formed.

My comment on 1890 applies here as well.

CC @rsmith @aaron.ballman how should we handle DRs that are still in process? 
While we may think we know the direction it is going in, it could change.

So maybe we should avoid expressing an opinion on whether these are well-formed 
or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148433

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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2996
   let Spellings = [GCC<"warn_unused">];
-  let Subjects = SubjectList<[Record]>;
+  let Subjects = SubjectList<[Record, CXXConstructor]>;
   let Documentation = [Undocumented];

aaron.ballman wrote:
> I'm confused -- if you want this to behave like `nodiscard`, why aren't these 
> changes for `warn_unused_result` (which is what implements `nodiscard`)? I 
> don't think this should go on `warn_unused` because that's for the 
> declaration of the type as a unit and not written on functions.
> 
> I think you don't need any changes to Attr.td for this functionality.
`[[nodiscard]]` and `__attribute__((warn_unused))` already have different 
semantics when applied to a class:  While both warn about unused expression 
results (`-Wunused-value`),  only the latter warns about unused variables 
(`-Wunused-variable`).  This patch keeps that difference, just extends it to 
individual constructors:
```
$ cat test.cc
struct [[nodiscard]] S1 {
  S1(int) {}
  S1(double) {}
};
struct S2 {
  [[nodiscard]] S2(int) {}
  S2(double) {}
};
struct __attribute__((warn_unused)) S3 {
  S3(int) {}
  S3(double) {}
};
struct S4 {
  __attribute__((warn_unused)) S4(int) {}
  S4(double) {}
};
int main() {
  S1(0); // expected-warning {{ignoring temporary}}
  S1(0.0); // expected-warning {{ignoring temporary}}
  S2(0); // expected-warning {{ignoring temporary}}
  S2(0.0);
  S3(0); // expected-warning {{expression result unused}}
  S3(0.0); // expected-warning {{expression result unused}}
  S4(0); // expected-warning {{expression result unused}}
  S4(0.0);
  S1 s1a(0);
  S1 s1b(0.0);
  S2 s2a(0);
  S2 s2b(0.0);
  S3 s3a(0); // expected-warning {{unused variable}}
  S3 s3b(0.0); // expected-warning {{unused variable}}
  S4 s4a(0); // expected-warning {{unused variable}}
  S4 s4b(0.0);
}
```
```
$ clang++ -Wunused-value -Wunused-variable -fsyntax-only -Xclang -verify test.cc
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Shivam Gupta via Phabricator via cfe-commits]

xgupta updated this revision to Diff 514262.
xgupta added a comment.

adjust the return and add test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

Files:
  clang/test/clang-rename/NonExistFile.cpp
  clang/tools/clang-rename/ClangRename.cpp


Index: clang/tools/clang-rename/ClangRename.cpp
===
--- clang/tools/clang-rename/ClangRename.cpp
+++ clang/tools/clang-rename/ClangRename.cpp
@@ -229,6 +229,10 @@
 Tool.applyAllReplacements(Rewrite);
 for (const auto &File : Files) {
   auto Entry = FileMgr.getFile(File);
+  if (!Entry) {
+errs() << "clang-rename: input file does not exist.\n";
+return 1;
+  }
   const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
   Rewrite.getEditBuffer(ID).write(outs());
 }
Index: clang/test/clang-rename/NonExistFile.cpp
===
--- /dev/null
+++ clang/test/clang-rename/NonExistFile.cpp
@@ -0,0 +1,2 @@
+// RUN: not clang-rename -offset=0 -new-name=plop asdasd 2>&1 | FileCheck %s
+// CHECK: clang-rename: input file does not exist.


Index: clang/tools/clang-rename/ClangRename.cpp
===
--- clang/tools/clang-rename/ClangRename.cpp
+++ clang/tools/clang-rename/ClangRename.cpp
@@ -229,6 +229,10 @@
 Tool.applyAllReplacements(Rewrite);
 for (const auto &File : Files) {
   auto Entry = FileMgr.getFile(File);
+  if (!Entry) {
+errs() << "clang-rename: input file does not exist.\n";
+return 1;
+  }
   const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
   Rewrite.getEditBuffer(ID).write(outs());
 }
Index: clang/test/clang-rename/NonExistFile.cpp
===
--- /dev/null
+++ clang/test/clang-rename/NonExistFile.cpp
@@ -0,0 +1,2 @@
+// RUN: not clang-rename -offset=0 -new-name=plop asdasd 2>&1 | FileCheck %s
+// CHECK: clang-rename: input file does not exist.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CXX/drs/dr16xx.cpp:46
+namespace dr1626 { // dr1626: no open
+// FIXME: all of the examples are well-formed
+#if __cplusplus >= 201103L

Since this is still open, should we be expressing an opinion on whether the 
examples are all well-formed or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148433

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added a comment.

In D148439#4272904 , @kbobyrev wrote:

> Oh, wait, I'm sorry, I didn't look into it closely :( Yeah, the `Input` file 
> is not really needed, most of the time the users of `clang-rename` (not sure 
> there are many with `clangd` being developed) use CLI flags for that.
>
> It's better to revert this patch. My bad, should've looked a bit closer into 
> this.

No issue, I also consider it a straightforward fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

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

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

Can you depend on `cstdio` in an LLVM toolchain test case like this?  `clang` 
doesn't install `cstdio` -- does it?  This is a c or c++ library file, so I 
think it should be out of scope for a clang test, right?  AFAICT no other clang 
test uses this header file.




Comment at: clang/test/Interpreter/dynamic-library.cpp:6
+
+#include 
+

This test fails for me like below.  


```
FAIL: Clang :: Interpreter/dynamic-library.cpp (1 of 17751)
 TEST 'Clang :: Interpreter/dynamic-library.cpp' FAILED 

Script:
--
: 'RUN: at line 4';   /local/mnt/workspace/upstream/obj_ubuntu/bin/clang -xc++ 
-o 
/local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output/libdynamic-library-test.so
 -fPIC -shared -DLIBRARY 
/local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/Inputs/dynamic-library-test.cpp
: 'RUN: at line 5';   cat 
/local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
 | env 
LD_LIBRARY_PATH=/local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output:$LD_LIBRARY_PATH
 /local/mnt/workspace/upstream/obj_ubuntu/bin/clang-repl | 
/local/mnt/workspace/upstream/obj_ubuntu/bin/FileCheck 
/local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
--
Exit Code: 1

Command Output (stderr):
--
In file included from <<< inputs >>>:1:
input_line_6:1:10: fatal error: 'cstdio' file not found
#include 
 ^~~~
error: Parsing failed.
input_line_12:1:1: error: use of undeclared identifier 'printf'
printf("Return value: %d\n", calculate_answer());
^
error: Parsing failed.
input_line_15:1:1: error: use of undeclared identifier 'printf'
printf("Variable: %d\n", ultimate_answer);
^
error: Parsing failed.
/local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp:15:11:
 error: CHECK: expected string not found in input
// CHECK: Return value: 5
  ^
:1:1: note: scanning from here
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> 
^
:1:231: note: possible intended match here
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> 


  ^

Input file: 
Check file: 
/local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp

-dump-input=help explains the following input dump.

Input was:
<<
1: clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl>  
check:15'0 
X
 error: no match found
check:15'1  


 ?   possible intended match
>>

--



Failed Tests (1):
  Clang :: Interpreter/dynamic-library.cpp

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added inline comments.



Comment at: clang/test/CXX/drs/dr23xx.cpp:42
 
+namespace dr2335 { // dr2335: no drafting
+// FIXME: all of the examples are well-formed.

shafik wrote:
> My comment on 1890 applies here as well.
> 
> CC @rsmith @aaron.ballman how should we handle DRs that are still in process? 
> While we may think we know the direction it is going in, it could change.
> 
> So maybe we should avoid expressing an opinion on whether these are 
> well-formed or not?
I asked Aaron even before uploading the patch. His response was:
> I think it's a judgement call -- if the CWG consensus seems like it makes a 
> lot of sense, then I see no reason not to test them but maybe leave a FIXME 
> comment about the issue technically being open still. If the CWG consensus 
> doesn't make sense, that might be time to get on the reflectors and ask 
> questions

Consensus documented in 2335 and drafting notes in P1787 that I quote in the 
summary made sense to me, so I published this patch. I can abandon it if there 
are fundamental issues with the them, rendering my judgement wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148433

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

[PATCH] D148496: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37

[Andrew via Phabricator via cfe-commits]

browneee accepted this revision.
browneee added a comment.

LGTM

`release_shadow_space.c:29: size_t get_rss_kb(): Assertion ```feof(f)' failed.`

`custom.cpp:1858: void test_sprintf(): Assertion ```strcmp(buf, "Hello world!") 
== 0' failed.`

Issue appears to be with the program rather than the instrumentation. Did the 
behavior of the the libc functions used in the test change?


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

https://reviews.llvm.org/D148496

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

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added inline comments.



Comment at: clang/test/Interpreter/dynamic-library.cpp:6
+
+#include 
+

bcain wrote:
> This test fails for me like below.  
> 
> 
> ```
> FAIL: Clang :: Interpreter/dynamic-library.cpp (1 of 17751)
>  TEST 'Clang :: Interpreter/dynamic-library.cpp' FAILED 
> 
> Script:
> --
> : 'RUN: at line 4';   /local/mnt/workspace/upstream/obj_ubuntu/bin/clang 
> -xc++ -o 
> /local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output/libdynamic-library-test.so
>  -fPIC -shared -DLIBRARY 
> /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/Inputs/dynamic-library-test.cpp
> : 'RUN: at line 5';   cat 
> /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
>  | env 
> LD_LIBRARY_PATH=/local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output:$LD_LIBRARY_PATH
>  /local/mnt/workspace/upstream/obj_ubuntu/bin/clang-repl | 
> /local/mnt/workspace/upstream/obj_ubuntu/bin/FileCheck 
> /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
> --
> Exit Code: 1
> 
> Command Output (stderr):
> --
> In file included from <<< inputs >>>:1:
> input_line_6:1:10: fatal error: 'cstdio' file not found
> #include 
>  ^~~~
> error: Parsing failed.
> input_line_12:1:1: error: use of undeclared identifier 'printf'
> printf("Return value: %d\n", calculate_answer());
> ^
> error: Parsing failed.
> input_line_15:1:1: error: use of undeclared identifier 'printf'
> printf("Variable: %d\n", ultimate_answer);
> ^
> error: Parsing failed.
> /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp:15:11:
>  error: CHECK: expected string not found in input
> // CHECK: Return value: 5
>   ^
> :1:1: note: scanning from here
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> 
> ^
> :1:231: note: possible intended match here
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> 
>   
>   
>   ^
> 
> Input file: 
> Check file: 
> /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
> 
> -dump-input=help explains the following input dump.
> 
> Input was:
> <<
> 1: clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> clang-repl> clang-repl> clang-repl>  
> check:15'0 
> X
>  error: no match found
> check:15'1
>   
>   
>?   possible intended match
> >>
> 
> --
> 
> 
> 
> Failed Tests (1):
>   Clang :: Interpreter/dynamic-library.cpp
> 
> ```
Thanks for your note, sounds reasonable. @argentite It should be sufficient to 
forward declare `printf`right? While we are here, can we somehow check whether 
the symbol is non-null before calling it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

[PATCH] D147928: [clang] Keep multiple-include optimization for null directives

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1183-1185
+if (!ReadAnyTokensBeforeDirective) {
+  CurPPLexer->MIOpt.ResetReadToken();
+}

NFC but matches our style guide.

One thing to consider: making the call a setter instead of a reset would 
simplify this logic into: 
`CurPPLexer->SetReadToken(ReadAnyTokensBeforeDirective);` without any branching 
required. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147928

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

[clang] 836786a - [NFC][Assignment Tracking] Rename flag variable in Options.td

[via cfe-commits]

Author: OCHyams
Date: 2023-04-17T17:43:52+01:00
New Revision: 836786ab7f6c5d94458fe4136807bfea6184e270

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

LOG: [NFC][Assignment Tracking] Rename flag variable in Options.td

Prefix the variable with 'f' to match other 'f' options.

Added: 


Modified: 
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index eb3c535a4a554..7fa1d7d636bbe 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5811,7 +5811,7 @@ def fctor_dtor_return_this : Flag<["-"], 
"fctor-dtor-return-this">,
"and non-deleting destructors. (No effect on Microsoft ABI)">,
   MarshallingInfoFlag>;
 
-def experimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignment-tracking=">,
+def fexperimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignment-tracking=">,
   Group, CodeGenOpts<"EnableAssignmentTracking">,
   NormalizedValuesScope<"CodeGenOptions::AssignmentTrackingOpts">,
   Values<"disabled,enabled,forced">, 
NormalizedValues<["Disabled","Enabled","Forced"]>,



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

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CXX/drs/dr23xx.cpp:42
 
+namespace dr2335 { // dr2335: no drafting
+// FIXME: all of the examples are well-formed.

Endill wrote:
> shafik wrote:
> > My comment on 1890 applies here as well.
> > 
> > CC @rsmith @aaron.ballman how should we handle DRs that are still in 
> > process? While we may think we know the direction it is going in, it could 
> > change.
> > 
> > So maybe we should avoid expressing an opinion on whether these are 
> > well-formed or not?
> I asked Aaron even before uploading the patch. His response was:
> > I think it's a judgement call -- if the CWG consensus seems like it makes a 
> > lot of sense, then I see no reason not to test them but maybe leave a FIXME 
> > comment about the issue technically being open still. If the CWG consensus 
> > doesn't make sense, that might be time to get on the reflectors and ask 
> > questions
> 
> Consensus documented in 2335 and drafting notes in P1787 that I quote in the 
> summary made sense to me, so I published this patch. I can abandon it if 
> there are fundamental issues with the them, rendering my judgement wrong.
I'd be curious to hear more thoughts on this.

In this particular case, this has been in drafting status since 2017, but the 
final comments on the open issue are:
```
Notes from the June, 2018 meeting:

The consensus of CWG was to treat templates and classes the same by 
"instantiating" delayed-parse regions when they are needed instead of at the 
end of the class.

See also issue 1890.
```
which seemed sufficiently firm in direction to warrant testing the behavior. I 
think any open DR being tested is subject to change in Clang and should 
continue to be documented in cxx_dr_status.html as "not resolved", though (so 
if CWG does make a change, we can still react to it without having promised the 
current behavior to users).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148433

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

[clang] 1f5e737 - [clang][cmake] Add options to pass in vcs repo and revision info

[Han Zhu via cfe-commits]

Author: Han Zhu
Date: 2023-04-17T09:50:18-07:00
New Revision: 1f5e737fc135bf991889a1364b8f8c5edc3953d2

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

LOG: [clang][cmake] Add options to pass in vcs repo and revision info

Clang may be built in an environment where Git is not available. In our case,
Clang is part of a larger monorepo which is not Git-based, and
GenerateVersionFromVCS was not able to get source info.

Provide options to pass in repo and revision info from cmake.
```
cmake \
  -DCLANG_VC_REPOSITORY=abc://repo.url.com \
  -DCLANG_VC_REVISION=abcd1234 \
  ...
```
This would allow us to prepare the source info beforehand and pass it to the
clang binary.

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

Added: 


Modified: 
clang/lib/Basic/CMakeLists.txt
llvm/cmake/modules/GenerateVersionFromVCS.cmake

Removed: 




diff  --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index 8c828e4c73225..c05036a216c74 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -14,9 +14,19 @@ set(generate_vcs_version_script 
"${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake"
 if(llvm_vc AND LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
 endif()
+if (LLVM_VC_REPOSITORY AND LLVM_VC_REVISION)
+  set(llvm_source_dir ${LLVM_SOURCE_DIR})
+  set(llvm_vc_repository ${LLVM_VC_REPOSITORY})
+  set(llvm_vc_revision ${LLVM_VC_REVISION})
+endif()
 if(clang_vc AND LLVM_APPEND_VC_REV)
   set(clang_source_dir ${CLANG_SOURCE_DIR})
 endif()
+if (CLANG_VC_REPOSITORY AND CLANG_VC_REVISION)
+  set(clang_source_dir ${CLANG_SOURCE_DIR})
+  set(clang_vc_repository ${CLANG_VC_REPOSITORY})
+  set(clang_vc_revision ${CLANG_VC_REVISION})
+endif()
 
 # Create custom target to generate the VC revision include.
 add_custom_command(OUTPUT "${version_inc}"
@@ -24,7 +34,11 @@ add_custom_command(OUTPUT "${version_inc}"
   COMMAND ${CMAKE_COMMAND} "-DNAMES=\"LLVM;CLANG\""
"-DLLVM_SOURCE_DIR=${llvm_source_dir}"
"-DCLANG_SOURCE_DIR=${clang_source_dir}"
+   "-DCLANG_VC_REPOSITORY=${clang_vc_repository}"
+   "-DCLANG_VC_REVISION=${clang_vc_revision}"
"-DHEADER_FILE=${version_inc}"
+   "-DLLVM_VC_REPOSITORY=${llvm_vc_repository}"
+   "-DLLVM_VC_REVISION=${llvm_vc_revision}"
-P "${generate_vcs_version_script}")
 
 # Mark the generated header as being generated.

diff  --git a/llvm/cmake/modules/GenerateVersionFromVCS.cmake 
b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
index d8ec54df41ebd..d5bdf43df7230 100644
--- a/llvm/cmake/modules/GenerateVersionFromVCS.cmake
+++ b/llvm/cmake/modules/GenerateVersionFromVCS.cmake
@@ -18,10 +18,7 @@ include(VersionFromVCS)
 # Handle strange terminals
 set(ENV{TERM} "dumb")
 
-function(append_info name path)
-  if(path)
-get_source_info("${path}" revision repository)
-  endif()
+function(append_info name revision repository)
   if(revision)
 file(APPEND "${HEADER_FILE}.tmp"
   "#define ${name}_REVISION \"${revision}\"\n")
@@ -39,10 +36,15 @@ function(append_info name path)
 endfunction()
 
 foreach(name IN LISTS NAMES)
-  if(NOT DEFINED ${name}_SOURCE_DIR)
+  if(${name}_VC_REPOSITORY AND ${name}_VC_REVISION)
+set(revision ${${name}_VC_REVISION})
+set(repository ${${name}_VC_REPOSITORY})
+  elseif(DEFINED ${name}_SOURCE_DIR)
+get_source_info(${${name}_SOURCE_DIR} revision repository)
+  else()
 message(FATAL_ERROR "${name}_SOURCE_DIR is not defined")
   endif()
-  append_info(${name} "${${name}_SOURCE_DIR}")
+  append_info(${name} ${revision} ${repository})
 endforeach()
 
 # Copy the file only if it has changed.



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

[PATCH] D148262: [clang][cmake] Add options to pass in vcs repo and revision info

[Han Zhu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f5e737fc135: [clang][cmake] Add options to pass in vcs repo 
and revision info (authored by zhuhan0).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148262

Files:
  clang/lib/Basic/CMakeLists.txt
  llvm/cmake/modules/GenerateVersionFromVCS.cmake


Index: llvm/cmake/modules/GenerateVersionFromVCS.cmake
===
--- llvm/cmake/modules/GenerateVersionFromVCS.cmake
+++ llvm/cmake/modules/GenerateVersionFromVCS.cmake
@@ -18,10 +18,7 @@
 # Handle strange terminals
 set(ENV{TERM} "dumb")
 
-function(append_info name path)
-  if(path)
-get_source_info("${path}" revision repository)
-  endif()
+function(append_info name revision repository)
   if(revision)
 file(APPEND "${HEADER_FILE}.tmp"
   "#define ${name}_REVISION \"${revision}\"\n")
@@ -39,10 +36,15 @@
 endfunction()
 
 foreach(name IN LISTS NAMES)
-  if(NOT DEFINED ${name}_SOURCE_DIR)
+  if(${name}_VC_REPOSITORY AND ${name}_VC_REVISION)
+set(revision ${${name}_VC_REVISION})
+set(repository ${${name}_VC_REPOSITORY})
+  elseif(DEFINED ${name}_SOURCE_DIR)
+get_source_info(${${name}_SOURCE_DIR} revision repository)
+  else()
 message(FATAL_ERROR "${name}_SOURCE_DIR is not defined")
   endif()
-  append_info(${name} "${${name}_SOURCE_DIR}")
+  append_info(${name} ${revision} ${repository})
 endforeach()
 
 # Copy the file only if it has changed.
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -14,9 +14,19 @@
 if(llvm_vc AND LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
 endif()
+if (LLVM_VC_REPOSITORY AND LLVM_VC_REVISION)
+  set(llvm_source_dir ${LLVM_SOURCE_DIR})
+  set(llvm_vc_repository ${LLVM_VC_REPOSITORY})
+  set(llvm_vc_revision ${LLVM_VC_REVISION})
+endif()
 if(clang_vc AND LLVM_APPEND_VC_REV)
   set(clang_source_dir ${CLANG_SOURCE_DIR})
 endif()
+if (CLANG_VC_REPOSITORY AND CLANG_VC_REVISION)
+  set(clang_source_dir ${CLANG_SOURCE_DIR})
+  set(clang_vc_repository ${CLANG_VC_REPOSITORY})
+  set(clang_vc_revision ${CLANG_VC_REVISION})
+endif()
 
 # Create custom target to generate the VC revision include.
 add_custom_command(OUTPUT "${version_inc}"
@@ -24,7 +34,11 @@
   COMMAND ${CMAKE_COMMAND} "-DNAMES=\"LLVM;CLANG\""
"-DLLVM_SOURCE_DIR=${llvm_source_dir}"
"-DCLANG_SOURCE_DIR=${clang_source_dir}"
+   "-DCLANG_VC_REPOSITORY=${clang_vc_repository}"
+   "-DCLANG_VC_REVISION=${clang_vc_revision}"
"-DHEADER_FILE=${version_inc}"
+   "-DLLVM_VC_REPOSITORY=${llvm_vc_repository}"
+   "-DLLVM_VC_REVISION=${llvm_vc_revision}"
-P "${generate_vcs_version_script}")
 
 # Mark the generated header as being generated.


Index: llvm/cmake/modules/GenerateVersionFromVCS.cmake
===
--- llvm/cmake/modules/GenerateVersionFromVCS.cmake
+++ llvm/cmake/modules/GenerateVersionFromVCS.cmake
@@ -18,10 +18,7 @@
 # Handle strange terminals
 set(ENV{TERM} "dumb")
 
-function(append_info name path)
-  if(path)
-get_source_info("${path}" revision repository)
-  endif()
+function(append_info name revision repository)
   if(revision)
 file(APPEND "${HEADER_FILE}.tmp"
   "#define ${name}_REVISION \"${revision}\"\n")
@@ -39,10 +36,15 @@
 endfunction()
 
 foreach(name IN LISTS NAMES)
-  if(NOT DEFINED ${name}_SOURCE_DIR)
+  if(${name}_VC_REPOSITORY AND ${name}_VC_REVISION)
+set(revision ${${name}_VC_REVISION})
+set(repository ${${name}_VC_REPOSITORY})
+  elseif(DEFINED ${name}_SOURCE_DIR)
+get_source_info(${${name}_SOURCE_DIR} revision repository)
+  else()
 message(FATAL_ERROR "${name}_SOURCE_DIR is not defined")
   endif()
-  append_info(${name} "${${name}_SOURCE_DIR}")
+  append_info(${name} ${revision} ${repository})
 endforeach()
 
 # Copy the file only if it has changed.
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -14,9 +14,19 @@
 if(llvm_vc AND LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
 endif()
+if (LLVM_VC_REPOSITORY AND LLVM_VC_REVISION)
+  set(llvm_source_dir ${LLVM_SOURCE_DIR})
+  set(llvm_vc_repository ${LLVM_VC_REPOSITORY})
+  set(llvm_vc_revision ${LLVM_VC_REVISION})
+endif()
 if(clang_vc AND LLVM_APPEND_VC_REV)
   set(clang_source_dir ${CLANG_SOURCE_DIR})
 endif()
+if (CLANG_VC_REPOSITORY AND CLANG_VC_REVISION)
+  set(clang_source_dir ${CLANG_SOURCE_DIR})
+  set(cl

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Anubhab Ghosh via Phabricator via cfe-commits]

argentite added a comment.

We should probably also address the lack of linker issue as well. Should we go 
for a precompiled dynamic library file? There seems to be some "precedent" of 
this in other tests.




Comment at: clang/test/Interpreter/dynamic-library.cpp:6
+
+#include 
+

sgraenitz wrote:
> bcain wrote:
> > This test fails for me like below.  
> > 
> > 
> > ```
> > FAIL: Clang :: Interpreter/dynamic-library.cpp (1 of 17751)
> >  TEST 'Clang :: Interpreter/dynamic-library.cpp' FAILED 
> > 
> > Script:
> > --
> > : 'RUN: at line 4';   /local/mnt/workspace/upstream/obj_ubuntu/bin/clang 
> > -xc++ -o 
> > /local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output/libdynamic-library-test.so
> >  -fPIC -shared -DLIBRARY 
> > /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/Inputs/dynamic-library-test.cpp
> > : 'RUN: at line 5';   cat 
> > /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
> >  | env 
> > LD_LIBRARY_PATH=/local/mnt/workspace/upstream/obj_ubuntu/tools/clang/test/Interpreter/Output:$LD_LIBRARY_PATH
> >  /local/mnt/workspace/upstream/obj_ubuntu/bin/clang-repl | 
> > /local/mnt/workspace/upstream/obj_ubuntu/bin/FileCheck 
> > /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
> > --
> > Exit Code: 1
> > 
> > Command Output (stderr):
> > --
> > In file included from <<< inputs >>>:1:
> > input_line_6:1:10: fatal error: 'cstdio' file not found
> > #include 
> >  ^~~~
> > error: Parsing failed.
> > input_line_12:1:1: error: use of undeclared identifier 'printf'
> > printf("Return value: %d\n", calculate_answer());
> > ^
> > error: Parsing failed.
> > input_line_15:1:1: error: use of undeclared identifier 'printf'
> > printf("Variable: %d\n", ultimate_answer);
> > ^
> > error: Parsing failed.
> > /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp:15:11:
> >  error: CHECK: expected string not found in input
> > // CHECK: Return value: 5
> >   ^
> > :1:1: note: scanning from here
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> 
> > ^
> > :1:231: note: possible intended match here
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> 
> > 
> > 
> > 
> >   ^
> > 
> > Input file: 
> > Check file: 
> > /local/mnt/workspace/upstream/llvm-project/clang/test/Interpreter/dynamic-library.cpp
> > 
> > -dump-input=help explains the following input dump.
> > 
> > Input was:
> > <<
> > 1: clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
> > clang-repl> clang-repl> clang-repl>  
> > check:15'0 
> > X
> >  error: no match found
> > check:15'1  
> > 
> > 
> >  ?   possible intended match
> > >>
> > 
> > --
> > 
> > 
> > 
> > Failed Tests (1):
> >   Clang :: Interpreter/dynamic-library.cpp
> > 
> > ```
> Thanks for your note, sounds reasonable. @argentite It should be sufficient 
> to forward declare `printf`right? While we are here, can we somehow check 
> whether the symbol is non-null before calling it?
Yes I think I can do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

zequanwu wrote:
> hans wrote:
> > zequanwu wrote:
> > > hans wrote:
> > > > Won't the code above (line 580) make many filenames absolute and cause 
> > > > us to use native slashes even when we want backslashes?
> > > > 
> > > > This would also need a test.
> > > > Won't the code above (line 580) make many filenames absolute and cause 
> > > > us to use native slashes even when we want backslashes?
> > > Yes, I don't think we should change anything if it's converted to an 
> > > absolute path.
> > > 
> > > Only if the `-fdebug-compilation-dir` is not given or is an absolute 
> > > path, the path in `/Fo` will be converted to absolute path. Users can 
> > > avoid the path being converted to absolute path by passing a relative 
> > > path to `-fdebug-compilation-dir`, just like what we do in chrome build 
> > > (`-fdebug-compilation-dir=.`).
> > > 
> > > Added a test.
> > Apologies for all the questions, but I find the logic a bit complex still. 
> > The test cases are helpful though.
> > 
> > Could we simplify by just always using Windows slashes here, regardless of 
> > `-fdebug-compilation-dir` etc.?
> The object file path will be converted to absolute path depending on whether 
> `-fdebug-compilation-dir` is given or whether it's absolute path. I don't 
> think we want to change a native absolute path of object file to use Windows 
> slashes. Or this could happen on linux: `/usr/local/src/a.obj` -> 
> `\usr\local\src\a.obj`. So, we only use windows slashes when it's not an 
> absolute path here.
Thanks, that makes sense. Can you add a comment with that explanation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:227
+(TSTy = Ty->getAs()))
+  Result.addOuterTemplateArguments(const_cast(FTD),
+   TSTy->template_arguments(),

erichkeane wrote:
> So I'd come up with something similar, but this ends up being a little goofy? 
>  And I thought it didn't really work in 1 of the cases.  I wonder if we're 
> better off looking at the decl contexts to try to figure out WHICH of the 
> cases we are, and just set the next decl context based on that?
sure, we can try. In the meantime - regarding GH62110 - do you happen to have 
any thoughts on that ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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