[clang] dbfe446 - [Clang] Implement CWG2640 Allow more characters in an n-char sequence

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2022-12-13T09:02:52+01:00
New Revision: dbfe446ef3b230e8d8421a6e79793fe6f405267f

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

LOG: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

Reviewed By: #clang-language-wg, aaron.ballman, tahonermann

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Lex/Lexer.h
clang/lib/Lex/Lexer.cpp
clang/lib/Lex/LiteralSupport.cpp
clang/test/CXX/drs/dr26xx.cpp
clang/test/Lexer/char-escapes-delimited.c
clang/test/Lexer/unicode.c
clang/test/Preprocessor/ucn-pp-identifier.c
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3aef8c93f6ff..cad59b0ce5de 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -705,6 +705,7 @@ C++2b Feature Support
 - Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 
`_).
   This change was applied to C++20 as a Defect Report.
 - Implemented "Permitting static constexpr variables in constexpr functions" 
(`P2647R1 _`).
+- Implemented `CWG2640 Allow more characters in an n-char sequence 
_`.
 
 CUDA/HIP Language Changes in Clang
 --

diff  --git a/clang/include/clang/Lex/Lexer.h b/clang/include/clang/Lex/Lexer.h
index 748a112b7d57..35c6a7bdd5ca 100644
--- a/clang/include/clang/Lex/Lexer.h
+++ b/clang/include/clang/Lex/Lexer.h
@@ -772,7 +772,7 @@ class Lexer : public PreprocessorLexer {
   llvm::Optional
   tryReadNumericUCN(const char *&StartPtr, const char *SlashLoc, Token 
*Result);
   llvm::Optional tryReadNamedUCN(const char *&StartPtr,
-   Token *Result);
+   const char *SlashLoc, Token 
*Result);
 
   /// Read a universal character name.
   ///

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index fc0b42f4688c..3866c2c85f18 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -1194,15 +1194,16 @@ static char GetTrigraphCharForLetter(char Letter) {
 /// whether trigraphs are enabled or not.
 static char DecodeTrigraphChar(const char *CP, Lexer *L, bool Trigraphs) {
   char Res = GetTrigraphCharForLetter(*CP);
-  if (!Res || !L) return Res;
+  if (!Res)
+return Res;
 
   if (!Trigraphs) {
-if (!L->isLexingRawMode())
+if (L && !L->isLexingRawMode())
   L->Diag(CP-2, diag::trigraph_ignored);
 return 0;
   }
 
-  if (!L->isLexingRawMode())
+  if (L && !L->isLexingRawMode())
 L->Diag(CP-2, diag::trigraph_converted) << StringRef(&Res, 1);
   return Res;
 }
@@ -3241,7 +3242,7 @@ llvm::Optional Lexer::tryReadNumericUCN(const 
char *&StartPtr,
   if (!Delimited)
 break;
   if (Diagnose)
-Diag(BufferPtr, diag::warn_delimited_ucn_incomplete)
+Diag(SlashLoc, diag::warn_delimited_ucn_incomplete)
 << StringRef(KindLoc, 1);
   return std::nullopt;
 }
@@ -3260,7 +3261,7 @@ llvm::Optional Lexer::tryReadNumericUCN(const 
char *&StartPtr,
 
   if (Count == 0) {
 if (Diagnose)
-  Diag(StartPtr, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
+  Diag(SlashLoc, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
: diag::warn_ucn_escape_no_digits)
   << StringRef(KindLoc, 1);
 return std::nullopt;
@@ -3268,13 +3269,13 @@ llvm::Optional Lexer::tryReadNumericUCN(const 
char *&StartPtr,
 
   if (Delimited && Kind == 'U') {
 if (Diagnose)
-  Diag(StartPtr, diag::err_hex_escape_no_digits) << StringRef(KindLoc, 1);
+  Diag(SlashLoc, diag::err_hex_escape_no_digits) << StringRef(KindLoc, 1);
 return std::nullopt;
   }
 
   if (!Delimited && Count != NumHexDigits) {
 if (Diagnose) {
-  Diag(BufferPtr, diag::warn_ucn_escape_incomplete);
+  Diag(SlashLoc, diag::warn_ucn_escape_incomplete);
   // If the user wrote \U1234, suggest a fixit to \u.
   if (Count == 4 && NumHexDigits == 8) {
 CharSourceRange URange = makeCharRange(*this, KindLoc, KindLoc + 1);
@@ -3286,15 +3287,18 @@ llvm::Optional Lexer::tryReadNumericUCN(const 
char *&StartPtr,
   }
 
   if (Delimited && PP) {
-Diag(BufferPtr, PP->getLangOpts().CPlusPlus2b
-? diag::warn_cxx2b_delimited_escape_sequence
-: diag::ext_delimited_escape_sequence)
+Diag(SlashLoc, PP->getLangOpts().CPlusPlus2b
+   ? diag::warn_cxx2b_delimited_escape_sequence
+   : diag::ext_delimited_escape_sequence)
 << /*delimited*/ 0 << (PP->getLangOpts().

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

[Corentin Jabot via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdbfe446ef3b2: [Clang] Implement CWG2640 Allow more 
characters in an n-char sequence (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/CXX/drs/dr26xx.cpp
  clang/test/Lexer/char-escapes-delimited.c
  clang/test/Lexer/unicode.c
  clang/test/Preprocessor/ucn-pp-identifier.c
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -15647,7 +15647,7 @@
 https://wg21.link/cwg2640";>2640
 accepted
 Allow more characters in an n-char sequence
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2641";>2641
Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
+// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef -DTRIGRAPHS=1
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -fno-trigraphs
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -DTRIGRAPHS=1 -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
 // RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -ftrigraphs -DTRIGRAPHS=1
 // RUN: not %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -Wundef 2>&1 | FileCheck -strict-whitespace %s
 
@@ -40,7 +40,6 @@
// ext-warning {{extension}} cxx2b-warning {{before C++2b}}
 #define \N{WASTEBASKET} // expected-error {{macro name must be an identifier}} \
 // ext-warning {{extension}} cxx2b-warning {{before C++2b}}
-
 #define a\u0024
 
 #if \u0110 // expected-warning {{is not defined, evaluates to 0}}
@@ -121,20 +120,39 @@
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
-#define \N{// expected-warning {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}} expected-error {{macro name must be an identifier}}
+#define \N{
+// expected-warning@-1 {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}}
+// expected-error@-2 {{macro name must be an identifier}}
 #define \N{}   // expected-warning {{empty delimited universal character name; treating as '\' 'N' '{' '}'}} expected-error {{macro name must be an identifier}}
 #define \N{NOTATHING}  // expected-error {{'NOTATHING' is not a valid Unicode character name}} \
// expected-error {{macro name must be an identifier}}
 #define \NN// expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} expected-error {{macro name must be an identifier}}
 #define \N{GREEK_SMALL-LETTERALPHA}  // expected-error {{'GREEK_SMALL-LETTERALPHA' is not a valid Unicode character name}} \
  // expected-note {{characters names in Unicode escape sequences are sensitive to case and whitespaces}}
+#define \N{🤡}  // expected-error {{'🤡' is not a valid Unicode character name}} \
+// expected-error {{macro name must be an identifier}}
 
 #define CONCAT(A, B) A##B
-int CONCAT(\N{GREEK, CAPITALLETTERALPHA}); // expected-error{{expected}} \
-   // expected-warning {{incomplete delimited universal character name}}
+int CONCAT(\N{GREEK
+, CAPITALLETTERALPHA});
+// expected-error@-2 {{expected}} \
+// expected-warning@-2 {{incomplete delimited universal character name}}
+
+int \N{\
+LATIN CAPITAL LETTER A WITH GRAVE};
+//ext-warning@-2 {{extension}} cxx2b-warning@-2 {{before C++2b}}
 
 #ifdef TRIGRAPHS
-int \N?? = 0; // expected-warning{{extension}} cxx2b-warning {{before C++2b}} \
+int \N?? = 0; // cxx2b-warning {{before C++2b}} \
+//ext-warning {{ex

[PATCH] D139211: [WIP][clang-format] Properly handle the C11 _Generic keyword.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1691
+(CurrentState.IsCSharpGenericTypeConstraint) || GenericSelection ||
 (Style.isJavaScript() && EndsInComma) ||
 (State.Line->MustBeDeclaration && !BinPackDeclaration) ||

I wonder if this says isJavaScript() here because of the 

```foo(name: type,name2: type2) syntax```

if that might be the case then given the format you present in your test then 
maybe it IS correct to have GenericSelection here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139211

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 482377.
cor3ntin added a comment.

remove llvm::None


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Sema/UsedDeclVisitor.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CXX/drs/dr26xx.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/CodeGenCXX/meminit-initializers-odr.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx11-default-member-initializers.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -15593,7 +15593,7 @@
 https://wg21.link/cwg2631";>2631
 DR
 Immediate function evaluations in default arguments
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2632";>2632
Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,73 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ SL::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ SL::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
+
+
+// FIXME
+// Init captures are subexpressions of the lambda expression
+// so according to the standard immediate invocations in init captures
+// should be evaluated at the call site.
+// However Clang does not yet implement this as it would introduce
+// a fair bit of complexity.
+// We intend to implement that functionality once we find real world
+// use cases that require it.
+constexpr int test_init_capture(int a =
+[b = SL::current().line()] { return b; }()) {
+  return a;
+}
+#ifdef USE_CONSTEVAL
+static_assert(test_init_capture() == __LINE__ - 4);
+#else
+static_assert(test_init_capture() == __LINE__ );
+#endif
+
+namespace check_immediate_invocations_in_templates {
+
+template 
+struct G {
+T line = __builtin_LINE();
+};
+template 
+struct S {
+int i = G{}.line;
+};
+static_assert(S{}.i != // intentional new line
+  S{}.i);
+
+template 
+constexpr int f(int i = G{}.line) {
+return i;
+}
+
+static_assert(f() != // intentional new line
+  f());
+}
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
+consteval int undefined();  // e

[clang] 697bfa4 - Revert "[UpdateTestChecks] Match define for labels"

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-12-13T09:15:35+01:00
New Revision: 697bfa40a3f853d8b9103ead53cd80a4f7c5a7df

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

LOG: Revert "[UpdateTestChecks] Match define for labels"

This reverts commit a25aeef8d6592c6cf5f4e5854cc39af49633.

This changes the default output of UTC, and as such introduces
spurious changes whenever existing tests are regenerated.

I've indicated in https://reviews.llvm.org/D139006#3989954 how
this can be implemented without causing test churn.

Added: 


Modified: 
clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected

clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected

clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.plain.expected
clang/test/utils/update_cc_test_checks/Inputs/def-and-decl.c.expected
clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected

clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp.expected

clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected

clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected

clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected

clang/test/utils/update_cc_test_checks/Inputs/global-hex-value-regex.c.expected
clang/test/utils/update_cc_test_checks/Inputs/global-value-regex.c.expected
clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.expected

clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c.expected

clang/test/utils/update_cc_test_checks/Inputs/replace-value-regex-across-runs.c.expected
clang/test/utils/update_cc_test_checks/check-globals.test

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/argument_name_reuse.ll.plain.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/custom-tool.ll.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.globals.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.globals.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.globals.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.globals.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.plain.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.scrub.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/sometimes_deleted_function.ll.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected
llvm/utils/UpdateTestChecks/common.py

Removed: 

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll.expected
llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test



diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected 
b/clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
index ccd308688374..6c7dd730e543 100644
--- a/clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
+++ b/clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
@@ -8,7 +8,7 @@ class Foo {
 public:
   explicit Foo(int x);
   ~Foo();
-// CHECK-LABEL: define {{[^@]+}}@_ZNK3Foo23function_defined_inlineEi(
+// CHECK-LABEL: @_ZNK3Foo23function_defined_inlineEi(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %class.Foo*, align 8
 // CHECK-NEXT:[[ARG_ADDR:%.*]] = alloca i32, align 4
@@ -27,7 +27,7 @@ public:
   inline int function_defined_out_of_line(int arg) const;
 };
 
-// CHECK-LABEL: define {{[^@]+}}@_ZN3FooC1Ei(
+// CHECK-LABEL: @_ZN3FooC1Ei(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %class.Foo*, align 8
 // CHECK-NEXT:[[X_ADDR:%.*]] = alloca i32, align 4
@@ -39,7 

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> The reason for me to work on this feature is the fact that clang-format 
> doesn't work reliable for changing the location of the const. After using 
> clang-tidy and clang-format some consts were still one the right hand side

Its true we made the decision if clang-format couldn't work it out we'd bail 
out and NOT make the change. We felt it was better to be correct than change 
all the const's

but if you could show us the examples it might help us work through those cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D139398: [AMDGPU] Add bf16 storage support

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening, some i16 -> fp32 bf16_to_fp operations can be left 
over.
+// Lower those to (f32 (fp_extend (f16 (bitconvert x
+if (!Op->getValueType(0).isFloatingPoint() ||
+Op->getOperand(0).getValueType() != MVT::i16)

arsenm wrote:
> Pierre-vh wrote:
> > arsenm wrote:
> > > Pierre-vh wrote:
> > > > arsenm wrote:
> > > > > Pierre-vh wrote:
> > > > > > arsenm wrote:
> > > > > > > Pierre-vh wrote:
> > > > > > > > arsenm wrote:
> > > > > > > > > Pierre-vh wrote:
> > > > > > > > > > arsenm wrote:
> > > > > > > > > > > The generic legalizer should have handled this?
> > > > > > > > > > It looks like those operations are not implemented in the 
> > > > > > > > > > generic legalizer, e.g. I get 
> > > > > > > > > > ``` 
> > > > > > > > > > Do not know how to promote this operator's operand!
> > > > > > > > > > ```
> > > > > > > > > Right, this is the code that would go there
> > > > > > > > Do I just copy/paste this code in that PromoteInt function, and 
> > > > > > > > keep a copy here too in LowerOperation? (not really a fan of 
> > > > > > > > copy-pasting code in different files, I'd rather keep it all 
> > > > > > > > here)
> > > > > > > > We need to have the lowering too AFAIK, it didn't go well when 
> > > > > > > > I tried to remove it
> > > > > > > I'm not following why you need to handle it here
> > > > > > IIRC:
> > > > > >  - I need to handle FP_TO_BF16 in ReplaceNodeResult because that's 
> > > > > > what the Integer Legalizer calls (through CustomLowerNode)
> > > > > >  - I need to handle both opcodes in LowerOperation because 
> > > > > > otherwise they'll fail selection. They can be left over from 
> > > > > > expanding/legalizing other operations.
> > > > > But why are they custom? We don't have to handle FP16_TO_FP or 
> > > > > FP_TO_FP16 there, and they aren't custom lowered. They have the same 
> > > > > basic properties. We have this:
> > > > > 
> > > > > 
> > > > > ```
> > > > > setOperationAction(ISD::FP16_TO_FP, MVT::i16, Promote);
> > > > > AddPromotedToType(ISD::FP16_TO_FP, MVT::i16, MVT::i32);
> > > > > setOperationAction(ISD::FP_TO_FP16, MVT::i16, Promote);
> > > > > AddPromotedToType(ISD::FP_TO_FP16, MVT::i16, MVT::i32);
> > > > > ```
> > > > > 
> > > > > I'd expect the same basic pattern
> > > > PromoteIntegerOperand, PromoteFloatOperand and PromoteIntegerResult 
> > > > don't handle FP_TO_BF16 and BF16_TO_FP, and unless we put a Custom 
> > > > lowering mode it'll assert/unreachable.
> > > > I tried to make it work (for a while) using the default expand but I 
> > > > can't quite get it to work. It feels like there is some legalizer work 
> > > > missing for handling BF16 like we want to.
> > > > Even though it's not ideal I think the custom lowering is easiest
> > > What about Expand? that's where the implemented part is
> > Last I tried, Expand will emit a libcall in many cases that we don't handle
> Library call is supposed to be a distinct action now, the DAG only did about 
> 5% of the work to migrate to using it. This code can go to the default expand 
> action
Does it need to happen in this commit? It'll delay the review quite a bit I 
think if other people have to review it
If it needs to happen, when what do I need to do? Use the Expand action & fix 
the legalizer in places where it needs to be fixed?

I feel like it might be better suited for a follow-up patch; I can create a 
task and pick it up when I come back from vacation if you want


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139398

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

[PATCH] D139507: [Intrinsic] Rename flt.rounds intrinsic to get.rounding

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 482381.
qiucf retitled this revision from "[Intrinsic] Add get.rounding as alias to 
flt.rounds and rename related DAG nodes" to "[Intrinsic] Rename flt.rounds 
intrinsic to get.rounding".
qiucf edited the summary of this revision.
qiucf added a comment.

Use AutoUpgrade to rename flt.rounds to get.rounding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139507

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-msp430.c
  clang/test/CodeGen/builtins.c
  libcxxabi/test/test_demangle.pass.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/IntrinsicLowering.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AArch64/arm64-fpcr.ll
  llvm/test/CodeGen/AArch64/strict-fp-opt.ll
  llvm/test/CodeGen/ARM/2012-03-05-FPSCR-bug.ll
  llvm/test/CodeGen/ARM/fpscr-intrinsics.ll
  llvm/test/CodeGen/ARM/no-fpscr-liveness.ll
  llvm/test/CodeGen/MSP430/flt_rounds.ll
  llvm/test/CodeGen/PowerPC/frounds.ll
  llvm/test/CodeGen/RISCV/flt-rounds.ll
  llvm/test/CodeGen/RISCV/fpenv.ll
  llvm/test/CodeGen/X86/flt-rounds.ll

Index: llvm/test/CodeGen/X86/flt-rounds.ll
===
--- llvm/test/CodeGen/X86/flt-rounds.ll
+++ llvm/test/CodeGen/X86/flt-rounds.ll
@@ -3,7 +3,7 @@
 ; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=-sse2 -verify-machineinstrs < %s | FileCheck %s --check-prefix=X86
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s --check-prefix=X64
 
-declare i32 @llvm.flt.rounds()
+declare i32 @llvm.get.rounding()
 
 define i32 @test_flt_rounds() nounwind {
 ; X86-LABEL: test_flt_rounds:
@@ -31,7 +31,7 @@
 ; X64-NEXT:shrl %cl, %eax
 ; X64-NEXT:andl $3, %eax
 ; X64-NEXT:retq
-  %1 = call i32 @llvm.flt.rounds()
+  %1 = call i32 @llvm.get.rounding()
   ret i32 %1
 }
 
@@ -172,21 +172,21 @@
 ; X64-NEXT:retq
 entry:
   %call = tail call i32 @fesetround(i32 1024)
-  %0 = tail call i32 @llvm.flt.rounds()
+  %0 = tail call i32 @llvm.get.rounding()
   %cmp = icmp ne i32 %0, 3
   %spec.select = zext i1 %cmp to i32
   %call1 = tail call i32 @fesetround(i32 0)
-  %1 = tail call i32 @llvm.flt.rounds()
+  %1 = tail call i32 @llvm.get.rounding()
   %cmp2 = icmp eq i32 %1, 1
   %inc4 = select i1 %cmp, i32 2, i32 1
   %errs.1 = select i1 %cmp2, i32 %spec.select, i32 %inc4
   %call6 = tail call i32 @fesetround(i32 3072)
-  %2 = tail call i32 @llvm.flt.rounds()
+  %2 = tail call i32 @llvm.get.rounding()
   %cmp7 = icmp ne i32 %2, 0
   %inc9 = zext i1 %cmp7 to i32
   %spec.select22 = add nuw nsw i32 %errs.1, %inc9
   %call11 = tail call i32 @fesetround(i32 2048)
-  %3 = tail call i32 @llvm.flt.rounds()
+  %3 = tail call i32 @llvm.get.rounding()
   %cmp12 = icmp ne i32 %3, 2
   %inc14.neg = sext i1 %cmp12 to i32
   %cmp16 = icmp ne i32 %spec.select22, %inc14.neg
Index: llvm/test/CodeGen/RISCV/fpenv.ll
===
--- llvm/test/CodeGen/RISCV/fpenv.ll
+++ llvm/test/CodeGen/RISCV/fpenv.ll
@@ -22,7 +22,7 @@
 ; RV64IF-NEXT:srl a0, a1, a0
 ; RV64IF-NEXT:andi a0, a0, 7
 ; RV64IF-NEXT:ret
-  %rm = call i32 @llvm.flt.rounds()
+  %rm = call i32 @llvm.get.rounding()
   ret i32 %rm
 }
 
@@ -122,4 +122,4 @@
 }
 
 declare void @llvm.set.rounding(i32)
-declare i32 @llvm.flt.rounds()
+declare i32 @llvm.get.rounding()
Index: llvm/test/CodeGen/RISCV/flt-rounds.ll
===
--- llvm/test/CodeGen/RISCV/flt-rounds.ll
+++ llvm/test/CodeGen/RISCV/flt-rounds.ll
@@ -4,7 +4,7 @@
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV64I %s
 
-declare i32 @llvm.flt.rounds()
+declare i32 @llvm.get.rounding()
 
 define i32 @test_flt_rounds() nounwind {
 ; RV32I-LABEL: test_flt_rounds:
@@ -16,6 +16,6 @@
 ; RV64I:   # %bb.0:
 ; RV64I-NEXT:li a0, 1
 ; RV64I-NEXT:ret
-  %1 = call i32 @llvm.flt.rounds()
+  %1 = call i32 @llvm.get.rounding()
   ret i32 %1
 }
Index: llvm/test/CodeGen/PowerPC/frounds.ll
===
--- llvm/test/CodeGen/PowerPC/frounds.ll
+++ llvm/test/CodeGen/Pow

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

this is actually an incorrect change (even builds shouldn't be succeeding). as 
the values here can also be `nullptr` (which cannot be stored in a 
StringLiteral) but moreover we later on assign these to `Prefixes` array, which 
is of type `char*`, hence the conversion should also be failing.

but in general i'd actually expect people to be assigning "nullptr"s to these 
`char*`s, hence if this was a purely mechanical migration without some extra 
constraints, it might still blow up at runtime even if it succeeds compiles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[PATCH] D139507: [Intrinsic] Rename flt.rounds intrinsic to get.rounding

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: libcxxabi/test/test_demangle.pass.cpp:12917
 
{"_ZNK4llvm17X86TargetLowering15LowerTRAMPOLINEENS_7SDValueERNS_12SelectionDAGE",
 "llvm::X86TargetLowering::LowerTRAMPOLINE(llvm::SDValue, llvm::SelectionDAG&) 
const"},
-
{"_ZNK4llvm17X86TargetLowering16LowerFLT_ROUNDS_ENS_7SDValueERNS_12SelectionDAGE",
 "llvm::X86TargetLowering::LowerFLT_ROUNDS_(llvm::SDValue, llvm::SelectionDAG&) 
const"},
+
{"_ZNK4llvm17X86TargetLowering16LowerGET_ROUNDINGENS_7SDValueERNS_12SelectionDAGE",
 "llvm::X86TargetLowering::LowerGET_ROUNDING(llvm::SDValue, 
llvm::SelectionDAG&) const"},
 {"_ZNK4llvm17X86TargetLowering9LowerCTLZENS_7SDValueERNS_12SelectionDAGE", 
"llvm::X86TargetLowering::LowerCTLZ(llvm::SDValue, llvm::SelectionDAG&) const"},

Demangling tests should not be updated.



Comment at: llvm/docs/LangRef.rst:24567
 
+For compatibility, ``llvm.flt.rounds`` has the same behavior as this intrinsic.
+

Can drop this note now.



Comment at: llvm/lib/IR/AutoUpgrade.cpp:916
+  return true;
+}
+break;

This needs a test in llvm/test/Bitcode. Create a bitcode file using flt.rounds 
prior to this change, and then test llvm-dis on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139507

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

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh updated this revision to Diff 482384.
Pierre-vh added a comment.

Put the assert back in, use alternative fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139713

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp


Index: clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only %s -std=c++14
+
+// Checks that Clang doesn't crash/assert on the nested call to "kaboom"
+// in "bar()".
+//
+// This is an interesting test case for `ExprConstant.cpp`'s `CallStackFrame`
+// because it triggers the following chain of events:
+// 0. `CheckEnableIf` calls `EvaluateWithSubstitution`.
+//  1. The outer call to "kaboom" gets evaluated.
+//   2. The expr for "a" gets evaluated, it has a version X;
+//  a temporary with the key (a, X) is created.
+// 3. The inner call to "kaboom" gets evaluated.
+//   4. The expr for "a" gets evaluated, it has a version Y;
+//  a temporary with the key (a, Y) is created.
+//   5. The expr for "b" gets evaluated, it has a version Y;
+//  a temporary with the key (b, Y) is created.
+//   6. `EvaluateWithSubstitution` looks at "b" but cannot evaluate it
+//  because it's value-dependent (due to the call to "f.foo()").
+//
+// When `EvaluateWithSubstitution` bails out while evaluating the outer
+// call, it attempts to fetch "b"'s param slot to clean it up.
+//
+// This used to cause an assertion failure in `getTemporary` because
+// a temporary with the key "(b, Y)" (created at step 4) existed but
+// not one for "(b, X)", which is what it was trying to fetch.
+
+template
+__attribute__((enable_if(true, "")))
+T kaboom(T a, T b) {
+  return b;
+}
+
+struct A {
+  double foo();
+};
+
+template 
+struct B {
+  A &f;
+
+  void bar() {
+kaboom(kaboom(0.0, 1.0), f.foo());
+  }
+};
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -16059,9 +16059,13 @@
 if ((*I)->isValueDependent() ||
 !EvaluateCallArg(PVD, *I, Call, Info) ||
 Info.EvalStatus.HasSideEffects) {
-  // If evaluation fails, throw away the argument entirely.
-  if (APValue *Slot = Info.getParamSlot(Call, PVD))
-*Slot = APValue();
+  // If evaluation fails, throw away the argument entirely unless I is
+  // value-dependent. In those cases, the condition above will 
short-circuit
+  // before calling `EvaluateCallArg` and no param slot is created.
+  if (!(*I)->isValueDependent()) {
+if (APValue *Slot = Info.getParamSlot(Call, PVD))
+  *Slot = APValue();
+  }
 }
 
 // Ignore any side-effects from a failed evaluation. This is safe because


Index: clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only %s -std=c++14
+
+// Checks that Clang doesn't crash/assert on the nested call to "kaboom"
+// in "bar()".
+//
+// This is an interesting test case for `ExprConstant.cpp`'s `CallStackFrame`
+// because it triggers the following chain of events:
+// 0. `CheckEnableIf` calls `EvaluateWithSubstitution`.
+//  1. The outer call to "kaboom" gets evaluated.
+//   2. The expr for "a" gets evaluated, it has a version X;
+//  a temporary with the key (a, X) is created.
+// 3. The inner call to "kaboom" gets evaluated.
+//   4. The expr for "a" gets evaluated, it has a version Y;
+//  a temporary with the key (a, Y) is created.
+//   5. The expr for "b" gets evaluated, it has a version Y;
+//  a temporary with the key (b, Y) is created.
+//   6. `EvaluateWithSubstitution` looks at "b" but cannot evaluate it
+//  because it's value-dependent (due to the call to "f.foo()").
+//
+// When `EvaluateWithSubstitution` bails out while evaluating the outer
+// call, it attempts to fetch "b"'s param slot to clean it up.
+//
+// This used to cause an assertion failure in `getTemporary` because
+// a temporary with the key "(b, Y)" (created at step 4) existed but
+// not one for "(b, X)", which is what it was trying to fetch.
+
+template
+__attribute__((enable_if(true, "")))
+T kaboom(T a, T b) {
+  return b;
+}
+
+struct A {
+  double foo();
+};
+
+template 
+struct B {
+  A &f;
+
+  void bar() {
+kaboom(kaboom(0.0, 1.0), f.foo());
+  }
+};
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ 

[clang] f1f1b60 - Implement CWG2631

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2022-12-13T09:57:05+01:00
New Revision: f1f1b60c7ba607e9ffe3bc012161d43ef95ac773

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

LOG: Implement CWG2631

Implement https://cplusplus.github.io/CWG/issues/2631.html.

Immediate calls in default arguments and defaults members
are not evaluated.

Instead, we evaluate them when constructing a
`CXXDefaultArgExpr`/`BuildCXXDefaultInitExpr`.

The immediate calls are executed by doing a
transform on the initializing expression.

Note that lambdas are not considering subexpressions so
we do not need to transform them.

As a result of this patch, unused default member
initializers are not considered odr-used, and
errors about members binding to local variables
in an outer scope only surface at the point
where a constructor is defined.

Reviewed By: aaron.ballman, #clang-language-wg

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

Added: 
clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
clang/test/CodeGenCXX/meminit-initializers-odr.cpp
clang/test/PCH/default-argument-with-immediate-calls.cpp
clang/test/SemaCXX/cxx2a-consteval-default-params.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/Parse/ParseCXXInlineMethods.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Sema/UsedDeclVisitor.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/CXX/class/class.local/p1-0x.cpp
clang/test/CXX/drs/dr26xx.cpp
clang/test/CodeGenCXX/builtin-source-location.cpp
clang/test/SemaCXX/cxx11-default-member-initializers.cpp
clang/test/SemaCXX/source_location.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cad59b0ce5def..73f2962ff2fc7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -645,6 +645,11 @@ C++ Language Changes in Clang
 - Implemented DR2358 allowing init captures in lambdas in default arguments.
 - implemented `DR2654 `_ which undeprecates
   all compound assignements operations on volatile qualified variables.
+- Implemented DR2631. Invalid ``consteval`` calls in default arguments and 
default
+  member initializers are diagnosed when and if the default is used.
+  This Fixes `Issue 56379 `_
+  and changes the value of ``std::source_location::current()``
+  used in default parameters calls compared to previous versions of Clang.
 
 C++20 Feature Support
 ^

diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index c2f6751d62ede..e99ebe6126824 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1244,8 +1244,12 @@ class CXXThrowExpr : public Expr {
 /// This wraps up a function call argument that was created from the
 /// corresponding parameter's default argument, when the call did not
 /// explicitly supply arguments for all of the parameters.
-class CXXDefaultArgExpr final : public Expr {
+class CXXDefaultArgExpr final
+: public Expr,
+  private llvm::TrailingObjects {
   friend class ASTStmtReader;
+  friend class ASTReader;
+  friend TrailingObjects;
 
   /// The parameter whose default is being used.
   ParmVarDecl *Param;
@@ -1254,7 +1258,7 @@ class CXXDefaultArgExpr final : public Expr {
   DeclContext *UsedContext;
 
   CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *Param,
-DeclContext *UsedContext)
+Expr *RewrittenExpr, DeclContext *UsedContext)
   : Expr(SC,
  Param->hasUnparsedDefaultArg()
  ? Param->getType().getNonReferenceType()
@@ -1263,28 +1267,58 @@ class CXXDefaultArgExpr final : public Expr {
  Param->getDefaultArg()->getObjectKind()),
 Param(Param), UsedContext(UsedContext) {
 CXXDefaultArgExprBits.Loc = Loc;
+CXXDefaultArgExprBits.HasRewrittenInit = RewrittenExpr != nullptr;
+if (RewrittenExpr)
+  *getTrailingObjects() = RewrittenExpr;
 setDependence(computeDependence(this));
   }
 
+  CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit)
+  : Expr(CXXDefaultArgExprClass, Empty) {
+CXXDefaultArgExprBits.HasRewrittenInit = HasRewrittenInit;
+  }
+
+  size_t numTrailing

[PATCH] D136554: Implement CWG2631

[Corentin Jabot 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 rGf1f1b60c7ba6: Implement CWG2631 (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Sema/UsedDeclVisitor.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CXX/drs/dr26xx.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/CodeGenCXX/meminit-initializers-odr.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx11-default-member-initializers.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -15593,7 +15593,7 @@
 https://wg21.link/cwg2631";>2631
 DR
 Immediate function evaluations in default arguments
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2632";>2632
Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,73 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ SL::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ SL::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
+
+
+// FIXME
+// Init captures are subexpressions of the lambda expression
+// so according to the standard immediate invocations in init captures
+// should be evaluated at the call site.
+// However Clang does not yet implement this as it would introduce
+// a fair bit of complexity.
+// We intend to implement that functionality once we find real world
+// use cases that require it.
+constexpr int test_init_capture(int a =
+[b = SL::current().line()] { return b; }()) {
+  return a;
+}
+#ifdef USE_CONSTEVAL
+static_assert(test_init_capture() == __LINE__ - 4);
+#else
+static_assert(test_init_capture() == __LINE__ );
+#endif
+
+namespace check_immediate_invocations_in_templates {
+
+template 
+struct G {
+T line = __builtin_LINE();
+};
+template 
+struct S {
+int i = G{}.line;
+};
+static_assert(S{}.i != // intentional new line
+  S{}.i);
+
+template 
+constexpr int f(int i = G{}.line) {
+return i;
+}
+
+static_assert(f() != // intentional new line
+  f());
+}
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -fsyntax-only -

[clang-tools-extra] 48e6ff9 - [clangd] Fix some header guard names, NFC

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-12-13T10:00:51+01:00
New Revision: 48e6ff9ad3eb1971de6d7ba12e31754781aff675

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

LOG: [clangd] Fix some header guard names, NFC

Per the LLVM code style, there should be no trailing `_` on the header
guard name.

Added: 


Modified: 
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/Transport.h
clang-tools-extra/clangd/support/Context.h
clang-tools-extra/clangd/support/MemoryTree.h
clang-tools-extra/clangd/support/Trace.h

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index de03e471be40f..738d1b3094ae5 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -10,8 +10,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
 
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
@@ -238,4 +238,4 @@ bool isExpandedFromParameterPack(const ParmVarDecl *D);
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H

diff  --git a/clang-tools-extra/clangd/Transport.h 
b/clang-tools-extra/clangd/Transport.h
index 70762176364f1..4e80ea95b8537 100644
--- a/clang-tools-extra/clangd/Transport.h
+++ b/clang-tools-extra/clangd/Transport.h
@@ -15,8 +15,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H
 
 #include "Feature.h"
 #include "llvm/ADT/StringRef.h"

diff  --git a/clang-tools-extra/clangd/support/Context.h 
b/clang-tools-extra/clangd/support/Context.h
index 2f403aebd3bba..13f959a9da449 100644
--- a/clang-tools-extra/clangd/support/Context.h
+++ b/clang-tools-extra/clangd/support/Context.h
@@ -11,8 +11,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_CONTEXT_H_
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_CONTEXT_H_
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_CONTEXT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_CONTEXT_H
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"

diff  --git a/clang-tools-extra/clangd/support/MemoryTree.h 
b/clang-tools-extra/clangd/support/MemoryTree.h
index 07935fd814db3..c3fc32183cd54 100644
--- a/clang-tools-extra/clangd/support/MemoryTree.h
+++ b/clang-tools-extra/clangd/support/MemoryTree.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_MEMORYTREE_H_
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_MEMORYTREE_H_
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_MEMORYTREE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_MEMORYTREE_H
 
 #include "Trace.h"
 #include "llvm/ADT/DenseMap.h"

diff  --git a/clang-tools-extra/clangd/support/Trace.h 
b/clang-tools-extra/clangd/support/Trace.h
index 52ee2ae617da6..1bfc75b874d8a 100644
--- a/clang-tools-extra/clangd/support/Trace.h
+++ b/clang-tools-extra/clangd/support/Trace.h
@@ -14,8 +14,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_TRACE_H_
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_TRACE_H_
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_TRACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_TRACE_H
 
 #include "support/Context.h"
 #include "llvm/ADT/StringRef.h"



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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line created this revision.
addr2line added a reviewer: MaskRay.
addr2line added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, StephenFan.
Herald added a reviewer: njames93.
Herald added a project: All.
addr2line requested review of this revision.
Herald added a subscriber: cfe-commits.

Just change the optional, NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -13,7 +13,7 @@
 #include "ClangTidyOptions.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
-#include "llvm/ADT/Optional.h"
+#include 
 #include 
 #include 
 #include 
@@ -155,7 +155,7 @@
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``std::nullopt``.
-llvm::Optional get(StringRef LocalName) const;
+std::optional get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -170,7 +170,7 @@
 /// global ``CheckOptions``. Gets local option first. If local is not
 /// present, falls back to get global option. If global option is not
 /// present either, return ``std::nullopt``.
-llvm::Optional getLocalOrGlobal(StringRef LocalName) const;
+std::optional getLocalOrGlobal(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -190,9 +190,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName) const {
-  if (llvm::Optional Value = get(LocalName)) {
+  if (std::optional Value = get(LocalName)) {
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -227,9 +227,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName) const {
-  llvm::Optional ValueOr = get(LocalName);
+  std::optional ValueOr = get(LocalName);
   bool IsGlobal = false;
   if (!ValueOr) {
 IsGlobal = true;
@@ -274,9 +274,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), false, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -314,9 +314,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), true, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -378,9 +378,9 @@
   private:
 using NameAndValue = std::pair;
 
-llvm::Optional getEnumInt(StringRef LocalName,
-   ArrayRef Mapping,
-   bool CheckGlobal, bool IgnoreCase) const;
+std::optional getEnumInt(StringRef LocalName,
+  ArrayRef Mapping,
+  bool CheckGlobal, bool IgnoreCase) const;
 
 template 
 std::enable_if_t::value, std::vector>
@@ -399,7 +399,6 @@
 void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
   int64_t Value) const;
 
-
 std::string NamePrefix;
 const ClangTidyOptions::OptionMap &CheckOptions;
 ClangTidyContext *Context;
@@ -433,7 +432,7 @@
 /// If the corresponding key can't be parsed as a bool, emit a
 /// diagnostic and return ``std::nullopt``.
 template <>
-llvm::Optional
+std::optional
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context`` and parse it as a bool.
@@ -445,7 +444,7 @@
 /// If the corresponding key can't be parsed as a bool, emit a
 /// diagnostic an

[PATCH] D139919: use std::optional in ClangTidyCheck

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp requested changes to this revision.
carlosgalvezp added a comment.
This revision now requires changes to proceed.

AFAIK it's preferred to use the LLVM types instead of the Standard types:

> When both C++ and the LLVM support libraries provide similar functionality, 
> and there isn’t a specific reason to favor the C++ implementation, it is 
> generally preferable to use the LLVM library

https://llvm.org/docs/CodingStandards.html#c-standard-library


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139919

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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line added a comment.

Sorry, I just saw a lot of changes in the llvm repo that change llvm::Optional 
to std::optional, like this one 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139919

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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D139919#3991242 , @carlosgalvezp 
wrote:

> AFAIK it's preferred to use the LLVM types instead of the Standard types:
>
>> When both C++ and the LLVM support libraries provide similar functionality, 
>> and there isn’t a specific reason to favor the C++ implementation, it is 
>> generally preferable to use the LLVM library
>
> https://llvm.org/docs/CodingStandards.html#c-standard-library

This should be read with a grain of salt. For example, it apparently doesn't 
apply to things are being deprecated. `llvm::Optional` is being actively 
removed and many of its functions are deprecated for eventual removal. A large 
portion of the other components in llvm-project have migrated. See 
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716/14




Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:457
 
-
 } // namespace tidy
 } // namespace clang

drop whitespace change

use line-based clang-format to prevent updating unrelated lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139919

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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D139919#3991247 , @addr2line wrote:

> Sorry, I just saw a lot of changes in the llvm repo that change 
> llvm::Optional to std::optional, like this one 
> .

Interesting, then I don't know, perhaps the guidelines have changed? Would be 
good to get input from the other reviewers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139919

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

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(NewState))
+  if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
 reportBug(Call, N, Constraint.get(), Summary, C);

Szelethus wrote:
> Let me know if I got this right. The reason behind `generateErrorNode` not 
> behaving like it usually does for other checkers is because of the explicitly 
> supplied `NewState` parameter -- in its absence, the //current// path of 
> execution is sunk. With this parameter, a new parallel node is. Correct?
The `NewState` only sets the state of the new error node, if it is nullptr the 
current state is used. A new node is always added. The other new node functions 
(`addTransition`, `generateNonFatalErrorNode`, `generateSink` and `addSink`) 
have a version that can take a predecessor node, only `generateErrorNode` did 
not have this (and I can not find out why part of these is called "generate" 
and other part "add" instead of using only "generate" or "add").

The new function is used when a node sequence `CurrentNode->A->B->ErrorNode` is 
needed. Without the new function it is only possible to make a 
`CurrentNode->ErrorNode` transition, and the following incorrect graph is 
created:
```
CurrentNode->A->B
  |->ErrorNode
```
The code here does exactly this (before the fix), in `NewNode` a sequence of 
nodes is appended (like A and B above), and if then an error node is created it 
is added to the CurrentNode. Not this is needed here, the error node should 
come after B. Otherwise analysis can continue after node B (that path is 
invalid because a constraint violation was found).
(The "CurrentNode" is a `Pred` value that is stored in `CheckerContext` and not 
changed if other nodes are added.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D139919#3991250 , @MaskRay wrote:

> In D139919#3991242 , @carlosgalvezp 
> wrote:
>
>> AFAIK it's preferred to use the LLVM types instead of the Standard types:
>>
>>> When both C++ and the LLVM support libraries provide similar functionality, 
>>> and there isn’t a specific reason to favor the C++ implementation, it is 
>>> generally preferable to use the LLVM library
>>
>> https://llvm.org/docs/CodingStandards.html#c-standard-library
>
> This should be read with a grain of salt. For example, it apparently doesn't 
> apply to things are being deprecated. `llvm::Optional` is being actively 
> removed and many of its functions are deprecated for eventual removal. A 
> large portion of the other components in llvm-project have migrated. See 
> https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Awesome, I wasn't aware. Thanks for the clarification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139919

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

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

kadircet wrote:
> this is actually an incorrect change (even builds shouldn't be succeeding). 
> as the values here can also be `nullptr` (which cannot be stored in a 
> StringLiteral) but moreover we later on assign these to `Prefixes` array, 
> which is of type `char*`, hence the conversion should also be failing.
> 
> but in general i'd actually expect people to be assigning "nullptr"s to these 
> `char*`s, hence if this was a purely mechanical migration without some extra 
> constraints, it might still blow up at runtime even if it succeeds compiles.
Builds (and test suite) all succeed (at least locally :-)). This patch also 
modifies tablegen to *not* generate `nullptr`, but empty string instead. The 
constructor of `OptTable::Info` has been modified to turn these "Empty string 
terminated array" into regular `ArrayRef`, which makes iteration code much more 
straight forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 482393.
merrymeerkat added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139868

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1314,7 +1314,8 @@
 [&Diagnostics,
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
-const TypeErasedDataflowAnalysisState &State) mutable {
+const TransferStateForDiagnostics
+&State) mutable {
   auto EltDiagnostics =
   Diagnoser.diagnose(Ctx, &Elt, State.Env);
   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
-   PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
-  ASTContext &Ctx, const CFGElement &Elt,
-  const TypeErasedDataflowAnalysisState &State) {
-if (PrevPostVisitCFG) {
-  PrevPostVisitCFG(Ctx, Elt, State);
-}
-// FIXME: Extend retrieval of state for non statement constructs.
-auto Stmt = Elt.getAs();
-if (!Stmt)
-  return;
-auto It = StmtToAnnotations.find(Stmt->getStmt());
-if (It == StmtToAnnotations.end())
-  return;
-auto *Lattice =
-llvm::any_cast(&State.Lattice.Value);
-auto [_, InsertSuccess] =
-AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
-(void)_;
-(void)InsertSuccess;
-assert(InsertSuccess);
-  };
+  auto PostVisitCFG =
+  [&StmtToAnnotations, &AnnotationStates,
+   P

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line updated this revision to Diff 482395.
addr2line added a comment.

remove the change to the unrelated lines.


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

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -13,7 +13,7 @@
 #include "ClangTidyOptions.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
-#include "llvm/ADT/Optional.h"
+#include 
 #include 
 #include 
 #include 
@@ -155,7 +155,7 @@
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``std::nullopt``.
-llvm::Optional get(StringRef LocalName) const;
+std::optional get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -170,7 +170,7 @@
 /// global ``CheckOptions``. Gets local option first. If local is not
 /// present, falls back to get global option. If global option is not
 /// present either, return ``std::nullopt``.
-llvm::Optional getLocalOrGlobal(StringRef LocalName) const;
+std::optional getLocalOrGlobal(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -190,9 +190,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName) const {
-  if (llvm::Optional Value = get(LocalName)) {
+  if (std::optional Value = get(LocalName)) {
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -227,9 +227,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName) const {
-  llvm::Optional ValueOr = get(LocalName);
+  std::optional ValueOr = get(LocalName);
   bool IsGlobal = false;
   if (!ValueOr) {
 IsGlobal = true;
@@ -274,9 +274,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), false, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -314,9 +314,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), true, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -378,9 +378,9 @@
   private:
 using NameAndValue = std::pair;
 
-llvm::Optional getEnumInt(StringRef LocalName,
-   ArrayRef Mapping,
-   bool CheckGlobal, bool IgnoreCase) const;
+std::optional getEnumInt(StringRef LocalName,
+  ArrayRef Mapping,
+  bool CheckGlobal, bool IgnoreCase) const;
 
 template 
 std::enable_if_t::value, std::vector>
@@ -433,7 +433,7 @@
 /// If the corresponding key can't be parsed as a bool, emit a
 /// diagnostic and return ``std::nullopt``.
 template <>
-llvm::Optional
+std::optional
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context`` and parse it as a bool.
@@ -445,7 +445,7 @@
 /// If the corresponding key can't be parsed as a bool, emit a
 /// diagnostic and return \p Default.
 template <>
-llvm::Optional
+std::optional
 ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const;
 
 /// Stores an option with the check-local name \p LocalName with
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -51,7 +51,7 @@
 : Na

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

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

Szelethus added a comment.

IIRC we talked about it would only really make sense to evaluate this patch 
stack as a whole, not piece by piece, but I'm not seeing results on open source 
projects here either. Can you please post them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: hokein, sammccall.
Herald added a subscriber: mgrang.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Introduce signals to rank providers of a symbol.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139921

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,16 +8,20 @@
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/SmallVector.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 
 namespace clang::include_cleaner {
 namespace {
+using testing::ElementsAre;
 using testing::UnorderedElementsAre;
 
 std::string guard(llvm::StringRef Code) {
@@ -31,7 +35,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -47,11 +51,12 @@
   void buildAST() { AST = std::make_unique(Inputs); }
 
   llvm::SmallVector findHeaders(llvm::StringRef FileName) {
-return include_cleaner::findHeaders(
+auto Headers = include_cleaner::findHeaders(
 AST->sourceManager().translateFileLineCol(
 AST->fileManager().getFile(FileName).get(),
 /*Line=*/1, /*Col=*/1),
 AST->sourceManager(), &PI);
+return {Headers.begin(), Headers.end()};
   }
   const FileEntry *physicalHeader(llvm::StringRef FileName) {
 return AST->fileManager().getFile(FileName).get();
@@ -153,5 +158,129 @@
physicalHeader("exporter.h")));
 }
 
+class HeadersForSymbolTest : public FindHeadersTest {
+protected:
+  llvm::SmallVector headersForFoo() {
+struct Visitor : public RecursiveASTVisitor {
+  const NamedDecl *Out = nullptr;
+  bool VisitNamedDecl(const NamedDecl *ND) {
+if (ND->getName() == "foo") {
+  EXPECT_TRUE(Out == nullptr || Out == ND->getCanonicalDecl())
+  << "Found multiple matches for foo.";
+  Out = cast(ND->getCanonicalDecl());
+}
+return true;
+  }
+};
+Visitor V;
+V.TraverseDecl(AST->context().getTranslationUnitDecl());
+if (!V.Out)
+  ADD_FAILURE() << "Couldn't find any decls named foo.";
+assert(V.Out);
+return headersForSymbol(*V.Out, AST->sourceManager(), &PI);
+  }
+};
+
+TEST_F(HeadersForSymbolTest, Deduplicates) {
+  Inputs.Code = R"cpp(
+#include "foo.h"
+  )cpp";
+  Inputs.ExtraFiles["foo.h"] = guard(R"cpp(
+// IWYU pragma: private, include "foo.h"
+void foo();
+void foo();
+  )cpp");
+  buildAST();
+  EXPECT_THAT(
+  headersForFoo(),
+  UnorderedElementsAre(physicalHeader("foo.h"),
+   // FIXME: de-duplicate across different kinds.
+   Header("\"foo.h\"")));
+}
+
+TEST_F(HeadersForSymbolTest, RankingSortsByName) {
+  Inputs.Code = R"cpp(
+#include "bar.h"
+#include "fox.h"
+  )cpp";
+  Inputs.ExtraFiles["fox.h"] = guard(R"cpp(
+void foo();
+  )cpp");
+  Inputs.ExtraFiles["bar.h"] = guard(R"cpp(
+void foo();
+  )cpp");
+  buildAST();
+  EXPECT_THAT(headersForFoo(),
+  ElementsAre(physicalHeader("bar.h"), physicalHeader("fox.h")));
+}
+
+TEST_F(HeadersForSymbolTest, Ranking) {
+  // Sorting is done over (canonical, public, complete) triplet.
+  Inputs.Code = R"cpp(
+#include "private.h"
+#include "public.h"
+#include "public_complete.h"
+  )cpp";
+  Inputs.ExtraFiles["public.h"] = guard(R"cpp(
+struct foo;
+  )cpp");
+  Inputs.ExtraFiles["private.h"] = guard(R"cpp(
+// IWYU pragma: private, include "canonical.h"
+struct foo;
+  )cpp");
+  Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
+  buildAST();
+  EXPECT_THAT(headersForFoo(), ElementsAre(Header("\"canonical.h\""),
+   physicalHeader("pu

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

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

Oops, the check on Debian failed. I'm looking into it...




Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:457
 
-
 } // namespace tidy
 } // namespace clang

MaskRay wrote:
> drop whitespace change
> 
> use line-based clang-format to prevent updating unrelated lines.
Thanks for pointing out. Fixed already.


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

https://reviews.llvm.org/D139919

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

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line updated this revision to Diff 482404.
addr2line marked an inline comment as done.
addr2line added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

It also need to change the return type in YAMLParser.


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

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  llvm/include/llvm/Support/YAMLParser.h
  llvm/lib/Support/YAMLParser.cpp

Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -760,7 +761,7 @@
   return EscapedInput;
 }
 
-llvm::Optional yaml::parseBool(StringRef S) {
+std::optional yaml::parseBool(StringRef S) {
   switch (S.size()) {
   case 1:
 switch (S.front()) {
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -78,7 +79,7 @@
 std::string escape(StringRef Input, bool EscapePrintable = true);
 
 /// Parse \p S as a bool according to https://yaml.org/type/bool.html.
-llvm::Optional parseBool(StringRef S);
+std::optional parseBool(StringRef S);
 
 /// This class represents a YAML stream potentially containing multiple
 ///documents.
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -13,7 +13,7 @@
 #include "ClangTidyOptions.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
-#include "llvm/ADT/Optional.h"
+#include 
 #include 
 #include 
 #include 
@@ -155,7 +155,7 @@
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``std::nullopt``.
-llvm::Optional get(StringRef LocalName) const;
+std::optional get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -170,7 +170,7 @@
 /// global ``CheckOptions``. Gets local option first. If local is not
 /// present, falls back to get global option. If global option is not
 /// present either, return ``std::nullopt``.
-llvm::Optional getLocalOrGlobal(StringRef LocalName) const;
+std::optional getLocalOrGlobal(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -190,9 +190,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName) const {
-  if (llvm::Optional Value = get(LocalName)) {
+  if (std::optional Value = get(LocalName)) {
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -227,9 +227,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName) const {
-  llvm::Optional ValueOr = get(LocalName);
+  std::optional ValueOr = get(LocalName);
   bool IsGlobal = false;
   if (!ValueOr) {
 IsGlobal = true;
@@ -274,9 +274,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), false, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -314,9 +314,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), true, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -378,9 +378,9 @@
   private:
 using NameAndValue = std::pair;
 
- 

[PATCH] D139919: use std::optional in ClangTidyCheck

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Sounds like the change to the YAML parser can potentially affect many other 
components other than clang-tidy - should that be done in a separate patch?


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

https://reviews.llvm.org/D139919

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

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
ckandeler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is needed for clients that would like to visualize matching
opening and closing angle brackets, which can be valuable in non-trivial
template declarations or instantiations.
It is not possible to do this with simple lexing, as the tokens
could also refer to operators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]$TemplateParameter[[T]]$AngleBracketClose[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]int$AngleBracketClose[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]int$AngleBracketClose[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]], typename = void$AngleBracketClose[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$AngleBracketOpen[[<]]class $TemplateParameter_def[[TT]]$AngleBracketClose[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$AngleBracketOpen[[<]]$TemplateParameter[[TT]]$AngleBracketClose[[>]] $Field_decl[[AA]];
   };
-  template
+  template$AngleBracketOpen[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$AngleBracketClose[[>]]
   class $Class_def[[BB]] {};
   template
   class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
   template
   class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$AngleBracketOpen[[<]]template$AngleBracketOpen[[<]]class$AngleBracketClose[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$AngleBracketClose[[>]]
+  $TemplateParameter[[T]]$AngleBracketOpen[[<]]$TemplateParameter[[C]]$AngleBracketClose[[>]] $Function_decl[[f]]();
 
-  template
+  template$AngleBracketOpen[[<]]typename$AngleBracketClose[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $AngleBracketOpen[[<]]class $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$AngleBracketOpen[[<]]float$AngleBracketClose[[>]];
+  template struct $Class_def[[Tmpl]]$AngleBracketOpen[[<]]double$AngleBracketClose[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$AngleBrack

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line updated this revision to Diff 482412.
addr2line added a comment.

Also in YAMLTraits.


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

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  llvm/include/llvm/Support/YAMLParser.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp

Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -886,7 +887,7 @@
 }
 
 StringRef ScalarTraits::input(StringRef Scalar, void *, bool &Val) {
-  if (llvm::Optional Parsed = parseBool(Scalar)) {
+  if (std::optional Parsed = parseBool(Scalar)) {
 Val = *Parsed;
 return StringRef();
   }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -760,7 +761,7 @@
   return EscapedInput;
 }
 
-llvm::Optional yaml::parseBool(StringRef S) {
+std::optional yaml::parseBool(StringRef S) {
   switch (S.size()) {
   case 1:
 switch (S.front()) {
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -78,7 +79,7 @@
 std::string escape(StringRef Input, bool EscapePrintable = true);
 
 /// Parse \p S as a bool according to https://yaml.org/type/bool.html.
-llvm::Optional parseBool(StringRef S);
+std::optional parseBool(StringRef S);
 
 /// This class represents a YAML stream potentially containing multiple
 ///documents.
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -13,7 +13,7 @@
 #include "ClangTidyOptions.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
-#include "llvm/ADT/Optional.h"
+#include 
 #include 
 #include 
 #include 
@@ -155,7 +155,7 @@
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``std::nullopt``.
-llvm::Optional get(StringRef LocalName) const;
+std::optional get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -170,7 +170,7 @@
 /// global ``CheckOptions``. Gets local option first. If local is not
 /// present, falls back to get global option. If global option is not
 /// present either, return ``std::nullopt``.
-llvm::Optional getLocalOrGlobal(StringRef LocalName) const;
+std::optional getLocalOrGlobal(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -190,9 +190,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName) const {
-  if (llvm::Optional Value = get(LocalName)) {
+  if (std::optional Value = get(LocalName)) {
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -227,9 +227,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 getLocalOrGlobal(StringRef LocalName) const {
-  llvm::Optional ValueOr = get(LocalName);
+  std::optional ValueOr = get(LocalName);
   bool IsGlobal = false;
   if (!ValueOr) {
 IsGlobal = true;
@@ -274,9 +274,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName, bool IgnoreCase = false) const {
-  if (llvm::Optional ValueOr =
+  if (std::optional ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), false, IgnoreCase))
 return static_cast(*ValueOr);
   return std::nullopt;
@@ -314,9 +314,9 @@
 /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
 /// supply the mapping required to convert between ``T`` and a string.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line added a comment.

In D139919#3991407 , @carlosgalvezp 
wrote:

> Sounds like the change to the YAML parser can potentially affect many other 
> components other than clang-tidy - should that be done in a separate patch?

Only these left.

  ./clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  95:  if (std::optional Parsed = llvm::yaml::parseBool(Value))
  
  ./clang-tools-extra/clangd/ConfigYAML.cpp
  365:  if (auto Bool = llvm::yaml::parseBool(**Scalar))
  
  ./clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  427:  llvm::Optional Parsed = llvm::yaml::parseBool(Iter->getValue());


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

https://reviews.llvm.org/D139919

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

[PATCH] D139928: [clang][DebugInfo] Backport emitting DW_AT_default_value for default template arguments

[Michael Buch via Phabricator via cfe-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, dblaikie.
Herald added a subscriber: hiraditya.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

With this patch Clang emits `DW_AT_default_value` to indicate defaulted
template arguments (this usage was introduced in DWARFv5) in earlier
versions of DWARF, unless compiling with `-gstrict-dwarf`.

Changes:

1. Previously the DwarfVersion check in `CGDebugInfo` was inconsistent: For 
non-type template arguments we attached the attribute to the debug metadata in 
DWARFv5 only. Whereas for type template arguments we didn't have such a version 
restriction. With this patch we attach the attribute regardless of DWARF 
version (and instead offload the check to the AsmPrinter).
2. The AsmPrinter will now also check for whether we compiled with 
`-gstrict-dwarf`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139928

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-parameter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h


Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -350,6 +350,10 @@
 
   virtual bool isDwoUnit() const = 0;
   const MCSymbol *getCrossSectionRelativeBaseAddress() const override;
+
+  /// Returns 'true' if 'DW_AT_default_value' should be emitted
+  /// to indicate defaulted template arguments.
+  bool shouldEmitTemplateDefaultAttr() const;
 };
 
 class DwarfTypeUnit final : public DwarfUnit {
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1060,7 +1060,7 @@
 addType(ParamDIE, TP->getType());
   if (!TP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, TP->getName());
-  if (TP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (TP->isDefault() && shouldEmitTemplateDefaultAttr())
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
 }
 
@@ -1074,7 +1074,7 @@
 addType(ParamDIE, VP->getType());
   if (!VP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, VP->getName());
-  if (VP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (VP->isDefault() && shouldEmitTemplateDefaultAttr())
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
   if (Metadata *Val = VP->getValue()) {
 if (ConstantInt *CI = mdconst::dyn_extract(Val))
@@ -1852,3 +1852,9 @@
 void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) {
   DD->getAddressPool().resetUsedFlag(true);
 }
+
+bool DwarfUnit::shouldEmitTemplateDefaultAttr() const {
+  assert(Asm != nullptr);
+  assert(DD != nullptr);
+  return !Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= 5;
+}
Index: clang/test/CodeGenCXX/debug-info-template-parameter.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-parameter.cpp
+++ clang/test/CodeGenCXX/debug-info-template-parameter.cpp
@@ -3,6 +3,9 @@
 
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 
-disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | 
llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,DWARFv4
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -gstrict-dwarf -triple x86_64 
%s -O0 -disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | 
llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,STRICT
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: 
![[F1_TYPE:[0-9]+]]
@@ -20,6 +23,24 @@
 // PRE17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i8 1)
 // CXX17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i1 true)
 
+// DWARF-DUMP: DW_TAG_class_type
+// DWARF-DUMP:   DW_AT_name  ("foo")
+// DWARF-DUMP:   DW_TAG_template_type_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "char")
+// DWARF-DUMP-NEXT:DW_AT_name("T")
+// DWA

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line updated this revision to Diff 482418.
addr2line added a comment.

finally


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

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  llvm/include/llvm/Support/YAMLParser.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp

Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -886,7 +887,7 @@
 }
 
 StringRef ScalarTraits::input(StringRef Scalar, void *, bool &Val) {
-  if (llvm::Optional Parsed = parseBool(Scalar)) {
+  if (std::optional Parsed = parseBool(Scalar)) {
 Val = *Parsed;
 return StringRef();
   }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -760,7 +761,7 @@
   return EscapedInput;
 }
 
-llvm::Optional yaml::parseBool(StringRef S) {
+std::optional yaml::parseBool(StringRef S) {
   switch (S.size()) {
   case 1:
 switch (S.front()) {
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -78,7 +79,7 @@
 std::string escape(StringRef Input, bool EscapePrintable = true);
 
 /// Parse \p S as a bool according to https://yaml.org/type/bool.html.
-llvm::Optional parseBool(StringRef S);
+std::optional parseBool(StringRef S);
 
 /// This class represents a YAML stream potentially containing multiple
 ///documents.
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -10,7 +10,8 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H
 
 #include "../utils/RenamerClangTidyCheck.h"
-#include "llvm/ADT/Optional.h"
+#include 
+
 namespace clang {
 namespace tidy {
 namespace readability {
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -424,7 +424,7 @@
   if (Iter == StrMap.end())
 return false;
 
-  llvm::Optional Parsed = llvm::yaml::parseBool(Iter->getValue());
+  std::optional Parsed = llvm::yaml::parseBool(Iter->getValue());
   return *Parsed;
 }
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -13,7 +13,7 @@
 #include "ClangTidyOptions.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
-#include "llvm/ADT/Optional.h"
+#include 
 #include 
 #include 
 #include 
@@ -155,7 +155,7 @@
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``std::nullopt``.
-llvm::Optional get(StringRef LocalName) const;
+std::optional get(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -170,7 +170,7 @@
 /// global ``CheckOptions``. Gets local option first. If local is not
 /// present, falls back to get global option. If global option is not
 /// present either, return ``std::nullopt``.
-llvm::Optional getLocalOrGlobal(StringRef LocalName) const;
+std::optional getLocalOrGlobal(StringRef LocalName) const;
 
 /// Read a named option from the ``Context``.
 ///
@@ -190,9 +190,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
 template 
-std::enable_if_t::value, llvm::Optional>
+std::enable_if_t::value, std::optional>
 get(StringRef LocalName) const {
-  if (llvm::Optional Value = get(LocalName)) {
+  if (std::optional Value = get(LocalName)) {
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -227,9 +227,9 @@
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and ret

[PATCH] D139928: [clang][DebugInfo] Backport emitting DW_AT_default_value for default template arguments

[Michael Buch via Phabricator via cfe-commits]

Michael137 updated this revision to Diff 482419.
Michael137 added a comment.

- Fix test formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139928

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-parameter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h


Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -350,6 +350,10 @@
 
   virtual bool isDwoUnit() const = 0;
   const MCSymbol *getCrossSectionRelativeBaseAddress() const override;
+
+  /// Returns 'true' if 'DW_AT_default_value' should be emitted
+  /// to indicate defaulted template arguments.
+  bool shouldEmitTemplateDefaultAttr() const;
 };
 
 class DwarfTypeUnit final : public DwarfUnit {
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1060,7 +1060,7 @@
 addType(ParamDIE, TP->getType());
   if (!TP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, TP->getName());
-  if (TP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (TP->isDefault() && shouldEmitTemplateDefaultAttr())
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
 }
 
@@ -1074,7 +1074,7 @@
 addType(ParamDIE, VP->getType());
   if (!VP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, VP->getName());
-  if (VP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (VP->isDefault() && shouldEmitTemplateDefaultAttr())
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
   if (Metadata *Val = VP->getValue()) {
 if (ConstantInt *CI = mdconst::dyn_extract(Val))
@@ -1852,3 +1852,9 @@
 void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) {
   DD->getAddressPool().resetUsedFlag(true);
 }
+
+bool DwarfUnit::shouldEmitTemplateDefaultAttr() const {
+  assert(Asm != nullptr);
+  assert(DD != nullptr);
+  return !Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= 5;
+}
Index: clang/test/CodeGenCXX/debug-info-template-parameter.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-parameter.cpp
+++ clang/test/CodeGenCXX/debug-info-template-parameter.cpp
@@ -3,6 +3,9 @@
 
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 
-disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | 
llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,DWARFv4
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -gstrict-dwarf -triple x86_64 
%s -O0 -disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | 
llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,STRICT
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: 
![[F1_TYPE:[0-9]+]]
@@ -20,6 +23,24 @@
 // PRE17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i8 1)
 // CXX17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i1 true)
 
+// DWARF-DUMP: DW_TAG_class_type
+// DWARF-DUMP:   DW_AT_name  ("foo")
+// DWARF-DUMP:   DW_TAG_template_type_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "char")
+// DWARF-DUMP-NEXT:DW_AT_name("T")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+// DWARF-DUMP:   DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "int")
+// DWARF-DUMP-NEXT:DW_AT_name("i")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+// DWARF-DUMP:   DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "bool")
+// DWARF-DUMP-NEXT:DW_AT_name("b")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+
 template 
 class foo {
 };
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugIn

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
 const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

serge-sans-paille wrote:
> kadircet wrote:
> > this is actually an incorrect change (even builds shouldn't be succeeding). 
> > as the values here can also be `nullptr` (which cannot be stored in a 
> > StringLiteral) but moreover we later on assign these to `Prefixes` array, 
> > which is of type `char*`, hence the conversion should also be failing.
> > 
> > but in general i'd actually expect people to be assigning "nullptr"s to 
> > these `char*`s, hence if this was a purely mechanical migration without 
> > some extra constraints, it might still blow up at runtime even if it 
> > succeeds compiles.
> Builds (and test suite) all succeed (at least locally :-)). This patch also 
> modifies tablegen to *not* generate `nullptr`, but empty string instead. The 
> constructor of `OptTable::Info` has been modified to turn these "Empty string 
> terminated array" into regular `ArrayRef`, which makes iteration code much 
> more straight forward.
> This patch also modifies tablegen to *not* generate nullptr

Oh i see, i definitely missed that one. It might be better to somehow separate 
that piece out (or at least mention somewhere in the patch description).

But nevertheless, as mentioned these values get assigned into `Prefixes` array 
mentioned above, which is a `char *`. hence the conversion **must** fail. are 
you sure you have `clang-tools-extra` in your enabled projects?

this also checks for a `nullptr` below (which I marked in a separate comment).



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:502
   for (unsigned A = ID; A != DriverID::OPT_INVALID; A = NextAlias[A]) {
 if (Prefixes[A] == nullptr) // option groups.
   continue;

this place for example is still checking for `nullptr`s as termination.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:511
 // Iterate over each spelling of the alias, e.g. -foo vs --foo.
 for (auto *Prefix = Prefixes[A]; *Prefix != nullptr; ++Prefix) {
   llvm::SmallString<64> Buf(*Prefix);

also this place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Ironically as an aside... looking back at the review because of the discource 
article.

The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years 
ago was to cover exactly this use case...and many others like it where users 
have a function which returns a boolean and takes no arguments are pointer or 
reference (empty() included)

https://reviews.llvm.org/D55433

Shouldn't people be using this checker to add nodiscard and let the compiler to 
the heavy lifting. Was there a use case as to why that solution doesn't solve 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[clang-tools-extra] bcb457c - [clangd] Fix a semantic highlighting crash on dependent code.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-12-13T12:19:41+01:00
New Revision: bcb457c68e20120f0bdd0a59e4b4ce90b8121310

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

LOG: [clangd] Fix a semantic highlighting crash on dependent code.

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 6745e2594ead3..f667567407793 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -619,14 +619,14 @@ class CollectExtraHighlightings
 
   bool VisitCXXNewExpr(CXXNewExpr *E) {
 auto &Token = H.addToken(E->getBeginLoc(), HighlightingKind::Operator);
-if (isa(E->getOperatorNew()))
+if (isa_and_present(E->getOperatorNew()))
   Token.addModifier(HighlightingModifier::UserDefined);
 return true;
   }
 
   bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
 auto &Token = H.addToken(E->getBeginLoc(), HighlightingKind::Operator);
-if (isa(E->getOperatorDelete()))
+if (isa_and_present(E->getOperatorDelete()))
   Token.addModifier(HighlightingModifier::UserDefined);
 return true;
   }

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index de3d099986fe9..bc890bf088a52 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -932,6 +932,16 @@ sizeof...($TemplateParameter[[Elements]]);
   auto $LocalVariable_def[[k]] = 
$Operator[[&]]$Class[[Foo]]::$Method[[foo]];
   ($Parameter[[f]]$Operator[[.*]]$LocalVariable[[k]])(); // no crash 
on VisitCXXMemberCallExpr
 }
+  )cpp",
+  R"cpp(
+template
+class $Class_def[[Foo]] {};
+
+template
+void $Function_def[[k]]() {
+  auto $LocalVariable_def[[s]] = $Operator[[new]] 
$Class[[Foo]]<$TemplateParameter[[T]]>();
+  $Operator[[delete]] $LocalVariable[[s]];
+}
   )cpp"};
   for (const auto &TestCase : TestCases)
 // Mask off scope modifiers to keep the tests manageable.



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

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

This patch causes a clangd crash on dependent code (e.g. running on 
`ASTMatchers.h`), fixed in 
https://github.com/llvm/llvm-project/commit/bcb457c68e20120f0bdd0a59e4b4ce90b8121310.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594

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

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D136594#3991530 , @hokein wrote:

> This patch causes a clangd crash on dependent code (e.g. running on 
> `ASTMatchers.h`), fixed in 
> https://github.com/llvm/llvm-project/commit/bcb457c68e20120f0bdd0a59e4b4ce90b8121310.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594

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

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(NewState))
+  if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
 reportBug(Call, N, Constraint.get(), Summary, C);

balazske wrote:
> Szelethus wrote:
> > Let me know if I got this right. The reason behind `generateErrorNode` not 
> > behaving like it usually does for other checkers is because of the 
> > explicitly supplied `NewState` parameter -- in its absence, the //current// 
> > path of execution is sunk. With this parameter, a new parallel node is. 
> > Correct?
> The `NewState` only sets the state of the new error node, if it is nullptr 
> the current state is used. A new node is always added. The other new node 
> functions (`addTransition`, `generateNonFatalErrorNode`, `generateSink` and 
> `addSink`) have a version that can take a predecessor node, only 
> `generateErrorNode` did not have this (and I can not find out why part of 
> these is called "generate" and other part "add" instead of using only 
> "generate" or "add").
> 
> The new function is used when a node sequence `CurrentNode->A->B->ErrorNode` 
> is needed. Without the new function it is only possible to make a 
> `CurrentNode->ErrorNode` transition, and the following incorrect graph is 
> created:
> ```
> CurrentNode->A->B
>   |->ErrorNode
> ```
> The code here does exactly this (before the fix), in `NewNode` a sequence of 
> nodes is appended (like A and B above), and if then an error node is created 
> it is added to the CurrentNode. Not this is needed here, the error node 
> should come after B. Otherwise analysis can continue after node B (that path 
> is invalid because a constraint violation was found).
> (The "CurrentNode" is a `Pred` value that is stored in `CheckerContext` and 
> not changed if other nodes are added.)
I've been wondering that, especially looking at the test case. Seems like this 
loop runs only once, how come that new nodes are added on top of `CurrentNode` 
(which, in this case refers to `C.getPredecessor()`, right?)? I checked the 
checker's code, and I can't really see why `A` and `B` would ever appear. Isn't 
that a bug?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(NewState))
+  if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
 reportBug(Call, N, Constraint.get(), Summary, C);

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > Let me know if I got this right. The reason behind `generateErrorNode` 
> > > not behaving like it usually does for other checkers is because of the 
> > > explicitly supplied `NewState` parameter -- in its absence, the 
> > > //current// path of execution is sunk. With this parameter, a new 
> > > parallel node is. Correct?
> > The `NewState` only sets the state of the new error node, if it is nullptr 
> > the current state is used. A new node is always added. The other new node 
> > functions (`addTransition`, `generateNonFatalErrorNode`, `generateSink` and 
> > `addSink`) have a version that can take a predecessor node, only 
> > `generateErrorNode` did not have this (and I can not find out why part of 
> > these is called "generate" and other part "add" instead of using only 
> > "generate" or "add").
> > 
> > The new function is used when a node sequence 
> > `CurrentNode->A->B->ErrorNode` is needed. Without the new function it is 
> > only possible to make a `CurrentNode->ErrorNode` transition, and the 
> > following incorrect graph is created:
> > ```
> > CurrentNode->A->B
> >   |->ErrorNode
> > ```
> > The code here does exactly this (before the fix), in `NewNode` a sequence 
> > of nodes is appended (like A and B above), and if then an error node is 
> > created it is added to the CurrentNode. Not this is needed here, the error 
> > node should come after B. Otherwise analysis can continue after node B 
> > (that path is invalid because a constraint violation was found).
> > (The "CurrentNode" is a `Pred` value that is stored in `CheckerContext` and 
> > not changed if other nodes are added.)
> I've been wondering that, especially looking at the test case. Seems like 
> this loop runs only once, how come that new nodes are added on top of 
> `CurrentNode` (which, in this case refers to `C.getPredecessor()`, right?)? I 
> checked the checker's code, and I can't really see why `A` and `B` would ever 
> appear. Isn't that a bug?
My thinking was that each checker, unless it does state splits, should really 
only create a single node per callback, right? The new state, however many 
changes it contains, should be added all at once in the single callback, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 482422.
ckandeler added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -130,17 +130,17 @@
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
+template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
 struct $Class_def[[A]] {
   $TemplateParameter[[T]] $Field_decl[[t]];
 };
   }
-  template
-  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
+  struct $Class_def[[C]] : $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]$TemplateParameter[[T]]$AngleBracketClose[[>]] {
 typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]] $Variable_def[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
+  $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]int$AngleBracketClose[[>]] $Variable_def[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]]$AngleBracketOpen[[<]]int$AngleBracketClose[[>]] $Class_decl[[AAA]];
   struct $Class_def[[B]] {
 $Class_decl_constrDestr[[B]]();
 ~$Class_decl_constrDestr[[B]]();
@@ -243,36 +243,36 @@
   typedef float $Primitive_decl[[F]];
 )cpp",
   R"cpp(
-  template
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]], typename = void$AngleBracketClose[[>]]
   class $Class_def[[A]] {
 $TemplateParameter[[T]] $Field_decl[[AA]];
 $TemplateParameter[[T]] $Method_decl[[foo]]();
   };
-  template
+  template$AngleBracketOpen[[<]]class $TemplateParameter_def[[TT]]$AngleBracketClose[[>]]
   class $Class_def[[B]] {
-$Class[[A]]<$TemplateParameter[[TT]]> $Field_decl[[AA]];
+$Class[[A]]$AngleBracketOpen[[<]]$TemplateParameter[[TT]]$AngleBracketClose[[>]] $Field_decl[[AA]];
   };
-  template
+  template$AngleBracketOpen[[<]]class $TemplateParameter_def[[TT]], class $TemplateParameter_def[[GG]]$AngleBracketClose[[>]]
   class $Class_def[[BB]] {};
   template
   class $Class_def[[BB]]<$TemplateParameter[[T]], int> {};
   template
   class $Class_def[[BB]]<$TemplateParameter[[T]], $TemplateParameter[[T]]*> {};
 
-  template class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]>
-  $TemplateParameter[[T]]<$TemplateParameter[[C]]> $Function_decl[[f]]();
+  template$AngleBracketOpen[[<]]template$AngleBracketOpen[[<]]class$AngleBracketClose[[>]] class $TemplateParameter_def[[T]], class $TemplateParameter_def[[C]]$AngleBracketClose[[>]]
+  $TemplateParameter[[T]]$AngleBracketOpen[[<]]$TemplateParameter[[C]]$AngleBracketClose[[>]] $Function_decl[[f]]();
 
-  template
+  template$AngleBracketOpen[[<]]typename$AngleBracketClose[[>]]
   class $Class_def[[Foo]] {};
 
-  template
+  template$AngleBracketOpen[[<]]typename $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
   void $Function_decl[[foo]]($TemplateParameter[[T]] ...);
 )cpp",
   R"cpp(
-  template 
+  template $AngleBracketOpen[[<]]class $TemplateParameter_def[[T]]$AngleBracketClose[[>]]
   struct $Class_def[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  extern template struct $Class_def[[Tmpl]];
-  template struct $Class_def[[Tmpl]];
+  extern template struct $Class_def[[Tmpl]]$AngleBracketOpen[[<]]float$AngleBracketClose[[>]];
+  template struct $Class_def[[Tmpl]]$AngleBracketOpen[[<]]double$AngleBracketClose[[>]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -335,17 +335,17 @@
 )cpp",
   R"cpp(
   class $Class_def[[G]] {};
-  template<$Class[[G]] *$TemplateParameter_def_readonly[[U]]>
+  template$AngleBracketOpen[[<]]$Class[[G]] *$TemplateParameter_def_readonly[[U]]$AngleBracketClose[[>]]
   class $Class_def[[GP]] {};
-  template<$Class[[G]] &$TemplateParameter_def_readonly[[U]]>
+  template$AngleBracketOpen[[<]]$Class[[G]] &$TemplateParameter_def_readonly[[U]]$AngleBracketClose[[>]]
   class $Class_def[[GR]] {};
-  template
+  template$AngleBracketOpen[[<]]int *$TemplateParameter_def_readonly[[U

[PATCH] D139919: use std::optional in ClangTidyCheck

[Yuao Ma via Phabricator via cfe-commits]

addr2line updated this revision to Diff 482424.

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

https://reviews.llvm.org/D139919

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  llvm/include/llvm/Support/YAMLParser.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp

Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -886,7 +886,7 @@
 }
 
 StringRef ScalarTraits::input(StringRef Scalar, void *, bool &Val) {
-  if (llvm::Optional Parsed = parseBool(Scalar)) {
+  if (std::optional Parsed = parseBool(Scalar)) {
 Val = *Parsed;
 return StringRef();
   }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -760,7 +760,7 @@
   return EscapedInput;
 }
 
-llvm::Optional yaml::parseBool(StringRef S) {
+std::optional yaml::parseBool(StringRef S) {
   switch (S.size()) {
   case 1:
 switch (S.front()) {
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -78,7 +79,7 @@
 std::string escape(StringRef Input, bool EscapePrintable = true);
 
 /// Parse \p S as a bool according to https://yaml.org/type/bool.html.
-llvm::Optional parseBool(StringRef S);
+std::optional parseBool(StringRef S);
 
 /// This class represents a YAML stream potentially containing multiple
 ///documents.
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H
 
 #include "../utils/RenamerClangTidyCheck.h"
-#include "llvm/ADT/Optional.h"
+#include 
 namespace clang {
 namespace tidy {
 namespace readability {
@@ -57,7 +57,7 @@
   struct HungarianNotationOption {
 HungarianNotationOption() : HPType(HungarianPrefixType::HPT_Off) {}
 
-llvm::Optional Case;
+std::optional Case;
 HungarianPrefixType HPType;
 llvm::StringMap General;
 llvm::StringMap CString;
@@ -69,14 +69,14 @@
   struct NamingStyle {
 NamingStyle() = default;
 
-NamingStyle(llvm::Optional Case, const std::string &Prefix,
+NamingStyle(std::optional Case, const std::string &Prefix,
 const std::string &Suffix, const std::string &IgnoredRegexpStr,
 HungarianPrefixType HPType);
 NamingStyle(const NamingStyle &O) = delete;
 NamingStyle &operator=(NamingStyle &&O) = default;
 NamingStyle(NamingStyle &&O) = default;
 
-llvm::Optional Case;
+std::optional Case;
 std::string Prefix;
 std::string Suffix;
 // Store both compiled and non-compiled forms so original value can be
@@ -120,12 +120,12 @@
 
   struct FileStyle {
 FileStyle() : IsActive(false), IgnoreMainLikeFunctions(false) {}
-FileStyle(SmallVectorImpl> &&Styles,
+FileStyle(SmallVectorImpl> &&Styles,
   HungarianNotationOption HNOption, bool IgnoreMainLike)
 : Styles(std::move(Styles)), HNOption(std::move(HNOption)),
   IsActive(true), IgnoreMainLikeFunctions(IgnoreMainLike) {}
 
-ArrayRef> getStyles() const {
+ArrayRef> getStyles() const {
   assert(IsActive);
   return Styles;
 }
@@ -139,7 +139,7 @@
 bool isIgnoringMainLikeFunction() const { return IgnoreMainLikeFunctions; }
 
   private:
-SmallVector, 0> Styles;
+SmallVector, 0> Styles;
 HungarianNotationOption HNOption;
 bool IsActive;
 bool IgnoreMainLikeFunctions;
@@ -168,13 +168,13 @@
 
   StyleKind findStyleKind(
   const NamedDecl *D,
-  ArrayRef> NamingStyles,
+  ArrayRef> NamingStyles,
   bool IgnoreMainLikeFunctions) const;
 
-  llvm::Optional getFailureInfo(
+  std::optional getFailureInfo(
   StringRef Type, StringRef Name, const NamedDecl *ND,
   SourceLocation Location,
-  ArrayRef> NamingStyles,
+  ArrayRef> NamingStyles,
   const IdentifierNamingCheck::HungarianNotationOption &HNOption,
   StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) const;
 
@@ -182,10 +182,10 @@
  bool IncludeMainLike) const;
 
 private:
-  llvm::Optional
+  std::optional
   getDeclFailureInfo(const NamedDecl *Decl,
  const SourceManager &SM) const override;
-  llvm

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

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

Szelethus added a comment.

In D135360#3981420 , @balazske wrote:

> My current approach is that the POSIX is more strict than the C standard 
> (POSIX allows a subset of what C allows). I do not see (errno related) 
> contradiction between these standards

Alright, so if we wanna support the C standard thinking, we would just create 
an option that would be a little more noisy. Sounds fair enough.

> (except initialization to zero of errno missing from POSIX, and possible 
> negative value in errno in POSIX). One problem exists now, that `errno` is 
> set to a non-zero (but not positive only) value at error. Probably we should 
> change this to be always positive at error, according to the C standard 
> `errno` is positive only, and POSIX does not tell that errno can be negative.

According to this (which I assume you've seen as well): 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

> Description
> ===
>
> [...]
> Some of the functionality described on this reference page extends the ISO C 
> standard. Any conflict between the requirements described here and the ISO C 
> standard is unintentional. [...]
> [...]
>
> Issue 6
> ===
>
> Values for errno are now required to be distinct positive values rather than 
> non-zero values. This change is for alignment with the ISO/IEC 9899:1999 
> standard.

So yes, I'd say its reasonable to assume in POSIX modthat if `errno` is 
non-zero, it is **positive**.

> The checker currently works only in POSIX mode for every function, the 
> **ModelPOSIX** setting controls only if additional functions are added (all 
> with POSIX rules) (these functions could be added with C rules too).

I see a lot of mention of POSIX vs. the C standard, yet there seems to be no 
mention of these differences in the comments added by this patch or already 
commited. Maybe we could do better on documenting this important difference in 
behaviour.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1818-1857
+// int fgetpos(FILE *restrict stream, fpos_t *restrict pos);
+// From 'The Open Group Base Specifications Issue 7, 2018 edition':
+// "The fgetpos() function shall not change the setting of errno if
+// successful."
+addToFunctionSummaryMap(
+"fgetpos",
+Signature(ArgTypes{FilePtrRestrictTy, FPosTPtrRestrictTy},

martong wrote:
> It is very good to see these new summaries! Would they be meaningful and 
> usable without the changes in the StreamChecker? If yes, then I think we 
> should split this into 2 patches.
Can you please do this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135360

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

[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

[Shilei Tian via Phabricator via cfe-commits]

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



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:184
+
+  auto AddStream =
+  [&](size_t Task,

jdoerfert wrote:
> jhuber6 wrote:
> > tianshilei1992 wrote:
> > > jhuber6 wrote:
> > > > tianshilei1992 wrote:
> > > > > jhuber6 wrote:
> > > > > > tianshilei1992 wrote:
> > > > > > > Is there any way that we don't write it to a file here?
> > > > > > Why do we need to invoke LTO here? I figured that we could call the 
> > > > > > backend directly since we have no need to actually link any filies, 
> > > > > > and we may not have a need to run more expensive optimizations when 
> > > > > > the bitcode is already optimized. If you do that then you should be 
> > > > > > able to just use a `raw_svector_ostream` as your output stream and 
> > > > > > get the compiled output written to that buffer.
> > > > > For the purpose of this basic JIT support, we indeed just need 
> > > > > backend. However, since we have the plan for super optimization, 
> > > > > etc., having an optimization pipeline here is also useful.
> > > > We should be able to configure our own optimization pipeline in that 
> > > > case, we might want the extra control as well.
> > > which means we basically rewrite the function `opt` and `backend` in 
> > > `LTO.cpp`. I thought about just invoking backend before, especially using 
> > > LTO requires us to build the resolution table. However, after a second 
> > > thought, I think it would be better to just use LTO.
> > Building the passes isn't too complicated, it would take up the same amount 
> > of space as the symbol resolutions and has the advantage that we don't need 
> > to write the output to a file. I could write an implementation for this to 
> > see how well it works.
> Agreed. We can test it in a follow up and decide then.
I already switched to build the pipeline ourselves and drop LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I am reluctant to add the dependency edge to rocm device libs to openmp's GPU 
runtime.

We currently require that library for libm, which I'm also not thrilled about, 
but at least you can currently build and run openmp programs (that don't use 
libm, like much of our tests) without it.

My problem with device libs is that it usually doesn't build with trunk. It 
follows a rolling dev model tied to rocm clang and when upstream does something 
that takes a long time to apply, device libs doesn't build until rocm catches 
up. I've literally never managed to compile any branch of device libs with 
trunk clang without modifying the source, generally to delete directories that 
don't look necessary for libm.

Further, selecting an ABI based on runtime code found in a library which is 
hopefully constant folded is a weird layering choice. The compiler knows what 
ABI it is emitting code for, and that's how it picks files from device libs to 
effect that choice, but it would make far more sense to me for the compiler 
back end to set this stuff up itself.

Also, if we handle ABI in the back end, then we don't get the inevitable 
problem of rocm device libs and trunk clang having totally different ideas of 
what the ABI is as they drift in and out of sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

[Moritz Sichert via Phabricator via cfe-commits]

MoritzS added a comment.

Thanks for the patch! I just tried it out and I think this enables many 
interesting use cases for WebAssembly when using C/C++.

Currently the lowering to wasm does not work when using externrefs when 
compiling without optimizations. Is that intended behavior?

I have this example C++ program (similar to the `wasm-externref.c` test):

  void extern_func(__externref_t ref);
  void my_func(__externref_t ref) {
  extern_func(ref);
  }

If I compile this without optimizations (exact command: `clang++ 
--target=wasm32 -mreference-types -c ./test_externref.cpp`), I get the 
following error:

  fatal error: error in backend: Cannot select: 0x62166bd0: externref,ch = 
load<(dereferenceable load (s0) from %ir.2)> 0x62166b58, 
TargetFrameIndex:i32<0>, undef:i32
0x62166a68: i32 = TargetFrameIndex<0>
0x62166ae0: i32 = undef

The reason for that can be seen when we look at the generated IR for this 
program:

  ; Function Attrs: mustprogress noinline optnone
  define hidden void @_Z7my_func11externref_t(ptr addrspace(10) %0) #0 {
%2 = alloca ptr addrspace(10), align 1
store ptr addrspace(10) %0, ptr %2, align 1
%3 = load ptr addrspace(10), ptr %2, align 1
call void @_Z11extern_func11externref_t(ptr addrspace(10) %3)
ret void
  }
  
  declare void @_Z11extern_func11externref_t(ptr addrspace(10)) #1

Apparently when no optimizations are enabled, clang will write all function 
arguments to an alloca'd variable. This obviously won't work if the value is an 
externref. The test case in `wasm-externref.c` also has the alloca and store 
with an externref value as expected output, which I think should never be 
generated.

With -O3 this doesn't happen as the alloca'd variables are optimized out. But I 
still think it should also work with -O0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: openmp/libomptarget/DeviceRTL/src/Mapping.cpp:19
 #pragma omp begin declare target device_type(nohost)
-
+extern const uint16_t __oclc_ABI_version;
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"

jhuber6 wrote:
> What if this isn't defined? We should be able to use the OpenMP library 
> without the AMD device libraries. Should it be extern weak?
It should be put into AMD's `declare variant`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 482440.
sdesmalen added a comment.

Changed arm_new_za to be a declaration attribute instead of a type attribute 
(this was something that @rsandifo-arm pointed out to me recently)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-dump-sme-attributes.cpp
  clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/aarch64-sme-attrs-no-sme.c
  clang/test/Sema/aarch64-sme-func-attrs.c

Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -0,0 +1,233 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -fsyntax-only -verify=expected-cpp -x c++ %s
+
+// Valid attributes
+
+__attribute__((arm_streaming)) void sme_arm_streaming(void);
+__attribute__((arm_streaming_compatible)) void sme_arm_streaming_compatible(void);
+
+__attribute__((arm_new_za)) void sme_arm_new_za(void) {}
+__attribute__((arm_shared_za)) void sme_arm_shared_za(void);
+__attribute__((arm_preserves_za)) void sme_arm_preserves_za(void);
+
+__attribute__((arm_streaming, arm_new_za)) void sme_arm_streaming_new_za(void) {}
+__attribute__((arm_streaming, arm_shared_za)) void sme_arm_streaming_shared_za(void);
+__attribute__((arm_streaming, arm_preserves_za)) void sme_arm_streaming_preserves_za(void);
+
+__attribute__((arm_streaming_compatible, arm_new_za)) void sme_arm_sc_new_za(void) {}
+__attribute__((arm_streaming_compatible, arm_shared_za)) void sme_arm_sc_shared_za(void);
+__attribute__((arm_streaming_compatible, arm_preserves_za)) void sme_arm_sc_preserves_za(void);
+
+__attribute__((arm_shared_za, arm_preserves_za)) void sme_arm_shared_preserves_za(void);
+
+__attribute__((arm_locally_streaming)) void sme_arm_locally_streaming(void) { }
+__attribute__((arm_locally_streaming, arm_streaming)) void sme_arm_streaming_and_locally_streaming(void) { }
+__attribute__((arm_locally_streaming, arm_streaming_compatible)) void sme_arm_streaming_and_streaming_compatible(void) { }
+
+__attribute__((arm_locally_streaming, arm_new_za)) void sme_arm_ls_new_za(void) { }
+__attribute__((arm_locally_streaming, arm_shared_za)) void sme_arm_ls_shared_za(void) { }
+__attribute__((arm_locally_streaming, arm_preserves_za)) void sme_arm_ls_preserves_za(void) { }
+
+// Valid attributes on function pointers
+
+void __attribute__((arm_streaming)) streaming_ptr(void);
+typedef __attribute__((arm_streaming)) void (*fptrty1) (void);
+fptrty1 call_streaming_func() { return streaming_ptr; }
+
+void __attribute__((arm_streaming_compatible)) streaming_compatible_ptr(void);
+typedef __attribute__((arm_streaming_compatible)) void (*fptrty2) (void);
+fptrty2 call_sc_func() { return streaming_compatible_ptr; }
+
+void __attribute__((arm_shared_za)) shared_za_ptr(void);
+typedef __attribute__((arm_shared_za)) void (*fptrty3) (void);
+fptrty3 call_shared_za_func() { return shared_za_ptr; }
+
+void __attribute__((arm_preserves_za)) preserves_za_ptr(void);
+typedef __attribute__((arm_preserves_za)) void (*fptrty4) (void);
+fptrty4 call_preserve_za_func() { return preserves_za_ptr; }
+
+void __attribute__((arm_shared_za, arm_preserves_za)) shared_preserves_za_ptr(void);
+typedef __attribute__((arm_shared_za, arm_preserves_za)) void (*fptrty5) (void);
+fptrty5 call_shared_preserve_za_func() { return shared_preserves_za_ptr; }
+
+typedef void (*fptrty6) (void);
+fptrty6 cast_nza_func_to_normal() { return sme_arm_new_za; }
+fptrty6 cast_ls_func_to_normal() { return sme_arm_locally_streaming; }
+
+// FIXME: Add invalid function pointer assignments such as assigning:
+//   1. A streaming compatible function to a normal function pointer,
+//   2. A locally streaming function to a streaming function pointer,
+// etc.
+
+// Invalid attributes
+
+// expected-cpp-error@+4 {{'arm_streaming_compatible' and 'arm_streaming' attributes are not compatible}}
+// expected-cpp-note@+3 {{conflicting attribute is here}}
+// expected-error@+2 {{'arm_streaming_compatible' and 'arm_streaming' attributes are not

[PATCH] D139450: Warn about unsupported ibmlongdouble

[Tulio Magno Quites Machado Filho via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/lit.local.cfg:26
+if config.ppc_linux_default_ieeelongdouble == "ON":
+  config.available_features.add('ppc_linux_default_ieeelongdouble')

qiucf wrote:
> tuliom wrote:
> > qiucf wrote:
> > > Can we assume if we are compiling with `-mabi=ieeelongdouble`, then 
> > > libc++ 'must' be built with the same long double ABI? If I understand 
> > > correctly, they're unrelated.
> > @qiucf I didn't understand this part. Are you suggesting to remove the long 
> > double warnings because the way the compiler was built is unrelated to the 
> > way libc++ was built?
> Ah, I misunderstood meaning of 'defaults to IEEE long double' in last review. 
> We can assume 'how the compiler was built' is the same as 'how the libc++ was 
> built'.
> 
> But when 'how the libc++ was built' conflicts with 'how the compiler compiles 
> current program', we expect a warning, right? If so, this looks reasonable.
Yes, exactly.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139450

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

[clang] 82d50fe - [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

[Dmitri Gribenko via cfe-commits]

Author: Dani Ferreira Franco Moura
Date: 2022-12-13T14:49:07+01:00
New Revision: 82d50fef9b7c1dfdff7f9265340fde40a34870cf

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

LOG: [clang][dataflow] Change the diagnoser API to receive a correctly typed 
lattice element

Previously, the diagnoser could only receive the Environment at a given program 
point. Now, it receives the complete dataflow state: the environment and 
lattice element.

This change does not contain any tests because we modify the checkDataflow 
function to rely on the newly introduced lattice element in PostVisitCFG, and 
existing tests that verify lattice elements depend on this overload of 
checkDataflow.

Reviewed By: gribozavr2, ymandel

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h 
b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
index 76d18c1d24463..37894ab37dd8d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+++ b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
@@ -48,6 +48,16 @@ template  struct TransferState {
   Environment &Env;
 };
 
+/// A read-only version of TransferState.
+template  struct TransferStateForDiagnostics {
+  TransferStateForDiagnostics(const LatticeT &Lattice, const Environment &Env)
+  : Lattice(Lattice), Env(Env) {}
+
+  /// Current lattice element.
+  const LatticeT &Lattice;
+  const Environment &Env;
+};
+
 template 
 using MatchSwitchMatcher = ast_matchers::internal::Matcher;
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 511ff71ed1d43..d20e1ed0d34fd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@ template  struct AnalysisInputs {
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@ template  struct AnalysisInputs {
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@ checkDataflow(AnalysisInputs AI,
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@ checkDataflow(AnalysisInputs AI,
   // Save the states computed for program points immediately following 
annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

[Dmitri Gribenko 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 rG82d50fef9b7c: [clang][dataflow] Change the diagnoser API to 
receive a correctly typed lattice… (authored by merrymeerkat, committed by 
gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139868

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1314,7 +1314,8 @@
 [&Diagnostics,
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
-const TypeErasedDataflowAnalysisState &State) mutable {
+const TransferStateForDiagnostics
+&State) mutable {
   auto EltDiagnostics =
   Diagnoser.diagnose(Ctx, &Elt, State.Env);
   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
-   PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
-  ASTContext &Ctx, const CFGElement &Elt,
-  const TypeErasedDataflowAnalysisState &State) {
-if (PrevPostVisitCFG) {
-  PrevPostVisitCFG(Ctx, Elt, State);
-}
-// FIXME: Extend retrieval of state for non statement constructs.
-auto Stmt = Elt.getAs();
-if (!Stmt)
-  return;
-auto It = StmtToAnnotations.find(Stmt->getStmt());
-if (It == StmtToAnnotations.end())
-  return;
-auto *Lattice =
-llvm::any_cast(&State.Lattice.Value);
-auto [_, InsertSuccess] =
-AnnotationStates.insert({It->

[PATCH] D139006: [UpdateTestChecks] Match define for labels

[Sanjay Patel via Phabricator via cfe-commits]

spatel added subscribers: arsenm, spatel.
spatel added a comment.

I agree with the revert - we'd need lots of commits like this:
b7232dafe69eb04c14217 
 (cc 
@arsenm)

Another possibility to fix this with less churn:
Add a warning during (re-)generation for any test where the CHECK-LABEL could 
be going wrong. So scan the file for a call to a function name that is also 
defined in that file?
We already have a warning like that for variables named `%tmp`. So if a test 
file contains the potentially buggy construct, then warn the user that they 
should regenerate with "--function-signature" to be safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139006

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

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D139737#3989927 , @NoQ wrote:

> Ok so at the conference @ymandel suggested us to use libTooling's 
> Transformers API to make our lives easier 
> (https://clang.llvm.org/docs/ClangTransformerTutorial.html). I'm fairly 
> certain we should at least consider using them, but I haven't looked into it 
> deeply enough yet. "Changing the type of a variable" is a problem that 
> ideally should be solved only once, because it's a problem that's easy to 
> formulate but difficult to "get right". Transformers appear to be a 
> collection of solutions to problems of this kind. We should see if they 
> already provide an out-of-the-box solution, or if they have bits and pieces 
> of machinery we can reuse in our solution.



1. Transformer is good when you are basing your changes around AST matchers.  
It is a collection of combinators over the `MatchResult` type. From what I'm 
seeing here, though, you don't seem to be basing it on AST matchers, in which 
case the libraries are not as useful.
2. We (google) have a tool for changing the type of a variable which takes into 
account all cascading changes to other declarations that result from that type 
change. It is built on Transformer and another library that builds a graph of 
relationships in the AST, relevant to the type-rewriting problem.  
Unfortunately, that's all internal -- not for IP reasons but simply because we 
never had reason to upstream it (nor an obvioius place to put it). So, I'd say 
that this advanced tooling is useful here if you want to _selectively_ change 
type T to type S, so you need tooling to tell you specifically which S's to 
update. If you're changing all T to S, then the problem is simpler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139737

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

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(NewState))
+  if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
 reportBug(Call, N, Constraint.get(), Summary, C);

Szelethus wrote:
> Szelethus wrote:
> > balazske wrote:
> > > Szelethus wrote:
> > > > Let me know if I got this right. The reason behind `generateErrorNode` 
> > > > not behaving like it usually does for other checkers is because of the 
> > > > explicitly supplied `NewState` parameter -- in its absence, the 
> > > > //current// path of execution is sunk. With this parameter, a new 
> > > > parallel node is. Correct?
> > > The `NewState` only sets the state of the new error node, if it is 
> > > nullptr the current state is used. A new node is always added. The other 
> > > new node functions (`addTransition`, `generateNonFatalErrorNode`, 
> > > `generateSink` and `addSink`) have a version that can take a predecessor 
> > > node, only `generateErrorNode` did not have this (and I can not find out 
> > > why part of these is called "generate" and other part "add" instead of 
> > > using only "generate" or "add").
> > > 
> > > The new function is used when a node sequence 
> > > `CurrentNode->A->B->ErrorNode` is needed. Without the new function it is 
> > > only possible to make a `CurrentNode->ErrorNode` transition, and the 
> > > following incorrect graph is created:
> > > ```
> > > CurrentNode->A->B
> > >   |->ErrorNode
> > > ```
> > > The code here does exactly this (before the fix), in `NewNode` a sequence 
> > > of nodes is appended (like A and B above), and if then an error node is 
> > > created it is added to the CurrentNode. Not this is needed here, the 
> > > error node should come after B. Otherwise analysis can continue after 
> > > node B (that path is invalid because a constraint violation was found).
> > > (The "CurrentNode" is a `Pred` value that is stored in `CheckerContext` 
> > > and not changed if other nodes are added.)
> > I've been wondering that, especially looking at the test case. Seems like 
> > this loop runs only once, how come that new nodes are added on top of 
> > `CurrentNode` (which, in this case refers to `C.getPredecessor()`, right?)? 
> > I checked the checker's code, and I can't really see why `A` and `B` would 
> > ever appear. Isn't that a bug?
> My thinking was that each checker, unless it does state splits, should really 
> only create a single node per callback, right? The new state, however many 
> changes it contains, should be added all at once in the single callback, no?
The problem is that multiple NoteTags are added. It is only possible to add a 
single NoteTag in a single transition. This is why in line 969 (in the 
currently shown code at time of this comment) `addTransition` is used for every 
new `SuccessSt` (otherwise `NewState` could be used without `NewNode`). Or is 
there a possibility for multiple NoteTags at one transition, or can such a 
feature be added? (But if the other state add functions all have a version that 
accepts a predecessor node, why is `generateErrorNode` exception?) (This state 
apply loop was changed in the recent time at least once.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[PATCH] D138614: [Clang][OpenMP][AMDGPU] Fix capture of variably modified type alias in teams distribute

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added a comment.

ping


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

https://reviews.llvm.org/D138614

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

[PATCH] D138614: [Clang][OpenMP][AMDGPU] Fix capture of variably modified type alias in teams distribute

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:246-249
+  if (!IsVMTTy)
+EscapedParameters.insert(VD);
+  else if (!IsForCombinedParallelRegion)
+return;

Also, can you make it specific to pinter types?



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:243
 if (!FD->getType()->isReferenceType()) {
-  assert(!VD->getType()->isVariablyModifiedType() &&
+  bool isVMT = VD->getType()->isVariablyModifiedType();
+  assert((!isVMT || (isVMT && !IsForCombinedParallelRegion)) &&

doru1004 wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > doru1004 wrote:
> > > > > > ABataev wrote:
> > > > > > > Is it for pointers only? Or for other types too?
> > > > > > I am not sure about that distinction; it allows for the same types 
> > > > > > as before except that when the directive is not a combined parallel 
> > > > > > for directive it returns immediately because the variable will then 
> > > > > > have to be captured and shared.
> > > > > Correction: it allows for VMTs to pass through in cases in which you 
> > > > > don't use a combined parallel for directive.
> > > > `IsVMTTy`?
> > > How can it be captured by value? Why does it happen? My first question 
> > > relates to the assertion you changed. I assume, it applies only to the 
> > > pointers. Otherwise, please add more context how we can capture mariably 
> > > modifiable type by value.
> > Yes it should be called `IsVMTTy`
> It only applies to pointers, it's the pointer that needs to be used correctly.
Can you change the assert check then to something like this:
```
assert((VD->getType()->isPointerType() || 
!VD->getType()->isVariablyModifiedType()) && 
```


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

https://reviews.llvm.org/D138614

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

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl added a comment.



In D139717#3987963 , @MaskRay wrote:

>> This change is breaking internal builds. We use the -Xfoo pattern but can 
>> now no longer manage whether we allow an unused -Xfoo option to pass as a 
>> warning or promote it to an error.
>
> Which `-Xfoo` is used? Do your internal builds reserve `-Xfoo` as a 
> placeholder to do other non-upstream work? Then you can consider `-Gfoo` if 
> your builds don't need the mips semantics.

We use -Xparser

> Is it possible to clean up the previously-ignored-with-a-warning `-Xfoo`?

Possibly. Maybe preferably. Builds are breaking now though. Dependent projects 
would have to be notified and changes scheduled. Was there a diff that we 
should have been part of? It looks like at least two of us are surprised with 
downstream consequences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139717

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

[PATCH] D139302: [RISCV] Add Syntacore SCR1 CPU model

[Dmitrii Petrov via Phabricator via cfe-commits]

dnpetrov-sc updated this revision to Diff 482449.
dnpetrov-sc marked an inline comment as done.
dnpetrov-sc added a comment.

- Added syntacore prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139302

Files:
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td

Index: llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
===
--- /dev/null
+++ llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
@@ -0,0 +1,207 @@
+//==- RISCVSchedSyntacoreSCR1.td - Syntacore SCR1 Scheduling Definitions *- tablegen -*-=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+
+// SCR1: https://github.com/syntacore/scr1
+
+// This model covers SYNTACORE_SCR1_CFG_RV32IMC_MAX configuration (syntacore-scr1-max).
+// SYNTACORE_SCR1_CFG_RV32IC_BASE (syntacore-scr1-base) configuration have essentially
+// same scheduling characteristics.
+
+// SCR1 is single-issue in-order processor
+def SyntacoreSCR1Model : SchedMachineModel {
+  let MicroOpBufferSize = 0;
+  let IssueWidth = 1;
+  let LoadLatency = 2;
+  let MispredictPenalty = 3;
+  let CompleteModel = 0;
+  let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
+ HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
+ HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,
+ HasVInstructions];
+}
+
+let SchedModel = SyntacoreSCR1Model in {
+
+let BufferSize = 0 in {
+def SCR1_ALU : ProcResource<1>;
+def SCR1_LSU : ProcResource<1>;
+def SCR1_MUL : ProcResource<1>;
+def SCR1_DIV : ProcResource<1>;
+def SCR1_CFU : ProcResource<1>;
+}
+
+// Branching
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer arithmetic and logic
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer multiplication: single-cycle multiplier in SCR1_CFG_RV32IMC_MAX
+def : WriteRes;
+def : WriteRes;
+
+// Integer division: latency 33, inverse throughput 33
+let Latency = 33, ResourceCycles = [33] in {
+def : WriteRes;
+def : WriteRes;
+}
+
+// Load/store instructions on SCR1 have latency 2 and inverse throughput 2
+// (SCR1_CFG_RV32IMC_MAX includes TCM)
+let Latency = 2, ResourceCycles=[2] in {
+// Memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+}
+
+let Unsupported = true in {
+// Atomic memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP load/store
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP instructions
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+def : WriteRes;
+}
+
+// Others
+def : WriteRes;
+def : WriteRes;
+
+def : InstRW<[WriteIALU], (instrs COPY)>;
+
+//===--===//
+// Bypasses (none)
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : Re

[PATCH] D139302: [RISCV] Add Syntacore SCR1 CPU model

[Dmitrii Petrov via Phabricator via cfe-commits]

dnpetrov-sc updated this revision to Diff 482455.
dnpetrov-sc added a comment.

- Fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139302

Files:
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td

Index: llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
===
--- /dev/null
+++ llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
@@ -0,0 +1,207 @@
+//==- RISCVSchedSyntacoreSCR1.td - Syntacore SCR1 Scheduling Definitions *- tablegen -*-=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+
+// SCR1: https://github.com/syntacore/scr1
+
+// This model covers SYNTACORE_SCR1_CFG_RV32IMC_MAX configuration (syntacore-scr1-max).
+// SYNTACORE_SCR1_CFG_RV32IC_BASE (syntacore-scr1-base) configuration has essentially
+// same scheduling characteristics.
+
+// SCR1 is single-issue in-order processor
+def SyntacoreSCR1Model : SchedMachineModel {
+  let MicroOpBufferSize = 0;
+  let IssueWidth = 1;
+  let LoadLatency = 2;
+  let MispredictPenalty = 3;
+  let CompleteModel = 0;
+  let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
+ HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
+ HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,
+ HasVInstructions];
+}
+
+let SchedModel = SyntacoreSCR1Model in {
+
+let BufferSize = 0 in {
+def SCR1_ALU : ProcResource<1>;
+def SCR1_LSU : ProcResource<1>;
+def SCR1_MUL : ProcResource<1>;
+def SCR1_DIV : ProcResource<1>;
+def SCR1_CFU : ProcResource<1>;
+}
+
+// Branching
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer arithmetic and logic
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer multiplication: single-cycle multiplier in SCR1_CFG_RV32IMC_MAX
+def : WriteRes;
+def : WriteRes;
+
+// Integer division: latency 33, inverse throughput 33
+let Latency = 33, ResourceCycles = [33] in {
+def : WriteRes;
+def : WriteRes;
+}
+
+// Load/store instructions on SCR1 have latency 2 and inverse throughput 2
+// (SCR1_CFG_RV32IMC_MAX includes TCM)
+let Latency = 2, ResourceCycles=[2] in {
+// Memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+}
+
+let Unsupported = true in {
+// Atomic memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP load/store
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP instructions
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+def : WriteRes;
+}
+
+// Others
+def : WriteRes;
+def : WriteRes;
+
+def : InstRW<[WriteIALU], (instrs COPY)>;
+
+//===--===//
+// Bypasses (none)
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAd

[PATCH] D139458: [clangd] Full support for #import insertions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ASTSignals.cpp:28
+  // Source files: Use #import if the ObjC language flag is enabled
+  if (isHeaderFile(AST.tuPath(), AST.getLangOpts())) {
+if (AST.getLangOpts().ObjC) {

can we rewrite this as:
```
bool preferImports(const ParsedAST &AST) {
  // If we don't have any objc-ness, always prefer #include
  if (!AST.getLangOpts().ObjC)
 return false;
  // If this is not a header file and has objc set as language, prefer #import
  if (!isHeaderFile(...))
 return true;
  // Headers lack proper compile flags most of the time, so we might treat a 
header as objc accidentally.
  // Perform some extra checks to make sure this works. 

  // Any file with a #import, should keep #import-ing.
  for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
if (Inc.Directive == tok::pp_import)
   return true;
  }
  
  // Any file declaring an objc struct should also #import.
  // No need to look over the references, as the file doesn't have any 
#imports, it must be declaring interesting Objc-like decls.
  for(Decl *D: AST.getLocalTopLevelDecls()) {
if (isa<...InterestingDeclTypes...>(D)
   return true;
   });
  return false;
}
```

It's fine to run findExplicitReferences for an extra time here, as we do it 
only when this is an ObjC header with no `#import`s



Comment at: clang-tools-extra/clangd/ASTSignals.h:33
   llvm::StringMap RelatedNamespaces;
+  /// Whether include insertion should suggest imports or includes.
+  Symbol::IncludeDirective InsertionDirective =

can we rather say `Preferred PP-directive to use for inclusions by the file.`



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:569
   }
+  Symbol::IncludeDirective Directive = Symbol::IncludeDirective::Include;
+  if (Signals)

we already have access to `Clang->getLangOpts()` here. we also have the 
`FileName` available so we can check for header-ness as well. we also have 
preamble includes to scan for #import directives (well, we might be building an 
AST without a preamble sometimes but it's fine if IncludeFixer can't suggest 
`#import`s in those cases).

so can we detect import-ness based on these here? we're only lacking 
information about whether the file declares any objc symbols, e.g. we might 
fail to suggest #import directives in a objc file without any #import 
statements, but I'd say it's fine considering all the extra complexity needed 
to propagate it from previous run. (if this turns out to be an important UX 
issue we can consider plumbing this information directly, so please leave a 
FIXME).



Comment at: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp:91
+  HeaderTU.Code = R"cpp(
+  #include "TestTU.h"
+  )cpp";

can you put this test first with a comment like `objc lang option is not enough 
for headers`?



Comment at: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp:97
+  HeaderTU.Code = R"cpp(
+  #include "TestTU.h"
+

this only works because we actually implicitly `#import` contents of 
`HeaderTU.HeaderCode` in TestTU.

can you instead clear up the  `HeaderCode` and define the interface directly in 
the header file?



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2727
 
-  Results = completions("Fun^", {Sym}, {}, "Foo.m").Completions;
+  CodeCompleteOptions Opts;
+  Opts.ImportInsertions = true;

can you use this `Opts` (without Signals) in the previous test as well to make 
sure we're `#include`ing the symbol in a non-import context even if the option 
is set?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139458

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

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght added inline comments.



Comment at: clang/lib/Format/Format.cpp:809
Style.AllowShortCaseLabelsOnASingleLine);
+IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+   Style.AllowShortCompoundRequirementOnASingleLine);

MyDeveloperDay wrote:
> haven't we use "Requires" in other options? What is the definition of 
> Compound?
> 
> I might be tempted for this to be AllShortRequiresOnASingleLine but then it 
> be an enum with the following options
> 
> ```
> Leave
> Never
> Always
> Compound
> ```
There are some options related to requires like IndentRequiresClause, 
RequiresClausePosition, etc. But as for single line and brace wrapping, 
requires is not yet considered.

"Compound" is from Compound Requirements section in this page 
https://en.cppreference.com/w/cpp/language/requires

I think this is a better way, I would try implementing it.



Comment at: clang/lib/Format/Format.cpp:1265
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;

MyDeveloperDay wrote:
> why would the default be true, is that what happens today?
yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834

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

[PATCH] D139398: [AMDGPU] Add bf16 storage support

[Matt Arsenault via Phabricator via cfe-commits]

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

LGTM. with nits The lower could handle the vector case easily, but it didn't 
before either




Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2915
+SDValue Op = Node->getOperand(0);
+if (Op.getValueType() == MVT::bf16)
+  Op = DAG.getNode(ISD::ANY_EXTEND, dl, MVT::i32,

Braces



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2944
+// not supported on the target and was softened to i16 for storage.
+if (Node->getValueType(0) == MVT::bf16)
+  Op = DAG.getNode(ISD::BITCAST, dl, MVT::bf16,

Braces



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2948
+else {
+  assert(Node->getValueType(0).isScalarInteger());
+  Op = DAG.getAnyExtOrTrunc(Op, dl, Node->getValueType(0));

Don't see the point of this assert, should work fine for vectors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139398

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

[PATCH] D139045: [HIP] support --offload-arch=native

[Yaxun Liu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
yaxunl marked 2 inline comments as done.
Closed by commit rGe8fd998e6194: [HIP] support --offload-arch=native (authored 
by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D139045?vs=482240&id=482463#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3067,6 +3067,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == "native" &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3067,6 +3067,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == "native" &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e8fd998 - [HIP] support --offload-arch=native

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2022-12-13T10:09:33-05:00
New Revision: e8fd998e6194d8749b83b30813e3cb74e5261770

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

LOG: [HIP] support --offload-arch=native

This patch detects system GPU and use them
in --offload-arch if 'native' is specified. If system GPU
cannot be detected clang will fall back to the default GPU arch.

Reviewed by: Artem Belevich

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/AMDGPU.h

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 969d38560018b..c7efe60b23359 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3067,6 +3067,17 @@ class OffloadingActionBuilder final {
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == "native" &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.h 
b/clang/lib/Driver/ToolChains/AMDGPU.h
index 0ac13ecaa6345..3f5461a1c5696 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {



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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please take a loon on https://github.com/llvm/llvm-project/issues/59487.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D138868: AMDGPU/clang: Remove target features from address space test builtins

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

ping


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

https://reviews.llvm.org/D138868

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1fdbe5c573b9: [clang][PPC] Checking Unknown Values Passed to 
-mcpu (authored by Qiongsi Wu ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

Files:
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/ppc-cpus.c

Index: clang/test/Driver/ppc-cpus.c
===
--- clang/test/Driver/ppc-cpus.c
+++ clang/test/Driver/ppc-cpus.c
@@ -5,6 +5,15 @@
 // RUN: %clang -### -c -target powerpc64 %s -mcpu=native 2>&1 | FileCheck --check-prefix=MCPU_NATIVE %s
 // MCPU_NATIVE-NOT: "-target-cpu" "native"
 
+/// Check that we are passing unknown mcpu options to the backend so an error
+/// can be triggered.
+// RUN: %clang -### -c -target powerpc64 %s -mcpu=asdf1234 2>&1 | FileCheck --check-prefix=MCPU_UNKNOWN %s
+// MCPU_UNKNOWN: "-target-cpu" "asdf1234"
+
+/// Check for the unknown target error if an unknown mcpu option is used.
+// RUN: not %clang -c -target powerpc64 %s -mcpu=asdf1234 2>&1 | FileCheck --check-prefix=MCPU_ERR %s
+// MCPU_ERR: unknown target CPU 'asdf1234'
+
 // RUN: %clang -### -c -target powerpc64 %s -mcpu=7400 2>&1 | FileCheck --check-prefix=MCPU_7400 %s
 // MCPU_7400: "-target-cpu" "7400"
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -409,25 +409,9 @@
   case llvm::Triple::ppc:
   case llvm::Triple::ppcle:
   case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le: {
-std::string TargetCPUName = ppc::getPPCTargetCPU(Args);
-// LLVM may default to generating code for the native CPU,
-// but, like gcc, we default to a more generic option for
-// each architecture. (except on AIX)
-if (!TargetCPUName.empty())
-  return TargetCPUName;
-
-if (T.isOSAIX())
-  TargetCPUName = "pwr7";
-else if (T.getArch() == llvm::Triple::ppc64le)
-  TargetCPUName = "ppc64le";
-else if (T.getArch() == llvm::Triple::ppc64)
-  TargetCPUName = "ppc64";
-else
-  TargetCPUName = "ppc";
+  case llvm::Triple::ppc64le:
+return ppc::getPPCTargetCPU(Args, T);
 
-return TargetCPUName;
-  }
   case llvm::Triple::csky:
 if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
   return A->getValue();
Index: clang/lib/Driver/ToolChains/Arch/PPC.h
===
--- clang/lib/Driver/ToolChains/Arch/PPC.h
+++ clang/lib/Driver/ToolChains/Arch/PPC.h
@@ -35,7 +35,8 @@
 
 FloatABI getPPCFloatABI(const Driver &D, const llvm::opt::ArgList &Args);
 
-std::string getPPCTargetCPU(const llvm::opt::ArgList &Args);
+std::string getPPCTargetCPU(const llvm::opt::ArgList &Args,
+const llvm::Triple &T);
 const char *getPPCAsmModeForCPU(StringRef Name);
 ReadGOTPtrMode getPPCReadGOTPtrMode(const Driver &D, const llvm::Triple &Triple,
 const llvm::opt::ArgList &Args);
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -20,46 +20,45 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static std::string getPPCGenericTargetCPU(const llvm::Triple &T) {
+  // LLVM may default to generating code for the native CPU,
+  // but, like gcc, we default to a more generic option for
+  // each architecture. (except on AIX)
+  if (T.isOSAIX())
+return "pwr7";
+  else if (T.getArch() == llvm::Triple::ppc64le)
+return "ppc64le";
+  else if (T.getArch() == llvm::Triple::ppc64)
+return "ppc64";
+  else
+return "ppc";
+}
+
 /// getPPCTargetCPU - Get the (LLVM) name of the PowerPC cpu we are targeting.
-std::string ppc::getPPCTargetCPU(const ArgList &Args) {
+std::string ppc::getPPCTargetCPU(const ArgList &Args, const llvm::Triple &T) {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_mcpu_EQ)) {
 StringRef CPUName = A->getValue();
 
+if (CPUName == "generic")
+  return getPPCGenericTargetCPU(T);
+
 if (CPUName == "native") {
   std::string CPU = std::string(llvm::sys::getHostCPUName());
   if (!CPU.empty() && CPU != "generic")
 return CPU;
   else
-return "";
+return getPPCGenericTargetCPU(T);
 }
 
 return llvm::StringSwitch(CPUName)
 .Case("common", "generic")
-.Case("440", "440")
 .Case("440fp", "440")
-.Case("450", "450")
-.Case("601", "601")
-.Case("602", "602")
-.Case("603", "603")
-.Case("603e", "603e")
-.Case("603ev", "603ev")
-.Case("604

[clang] 1fdbe5c - [clang][PPC] Checking Unknown Values Passed to -mcpu

[Qiongsi Wu via cfe-commits]

Author: Qiongsi Wu
Date: 2022-12-13T10:18:44-05:00
New Revision: 1fdbe5c573b920f00e5bfdbcea0e837833ae77a0

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

LOG: [clang][PPC] Checking Unknown Values Passed to -mcpu

Currently `ppc::getPPCTargetCPU` returns an empty string when it encounters an 
unknown value passed to `-mcpu`. This causes `clang` to ignore unknown `-mcpu` 
values silently.

This patch changes the behaviour of `ppc::getPPCTargetCPU` so that it passes 
the unknown option to the target info, so the target info can actually check if 
the CPU string is supported, and report an error when encountering 
unknown/unsupported CPU string.

Reviewed By: jamieschmeiser

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/PPC.cpp
clang/lib/Driver/ToolChains/Arch/PPC.h
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/ppc-cpus.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/PPC.cpp 
b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
index bcaecf4b2d980..f90a772ed5a21 100644
--- a/clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -20,46 +20,45 @@ using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+static std::string getPPCGenericTargetCPU(const llvm::Triple &T) {
+  // LLVM may default to generating code for the native CPU,
+  // but, like gcc, we default to a more generic option for
+  // each architecture. (except on AIX)
+  if (T.isOSAIX())
+return "pwr7";
+  else if (T.getArch() == llvm::Triple::ppc64le)
+return "ppc64le";
+  else if (T.getArch() == llvm::Triple::ppc64)
+return "ppc64";
+  else
+return "ppc";
+}
+
 /// getPPCTargetCPU - Get the (LLVM) name of the PowerPC cpu we are targeting.
-std::string ppc::getPPCTargetCPU(const ArgList &Args) {
+std::string ppc::getPPCTargetCPU(const ArgList &Args, const llvm::Triple &T) {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_mcpu_EQ)) {
 StringRef CPUName = A->getValue();
 
+if (CPUName == "generic")
+  return getPPCGenericTargetCPU(T);
+
 if (CPUName == "native") {
   std::string CPU = std::string(llvm::sys::getHostCPUName());
   if (!CPU.empty() && CPU != "generic")
 return CPU;
   else
-return "";
+return getPPCGenericTargetCPU(T);
 }
 
 return llvm::StringSwitch(CPUName)
 .Case("common", "generic")
-.Case("440", "440")
 .Case("440fp", "440")
-.Case("450", "450")
-.Case("601", "601")
-.Case("602", "602")
-.Case("603", "603")
-.Case("603e", "603e")
-.Case("603ev", "603ev")
-.Case("604", "604")
-.Case("604e", "604e")
-.Case("620", "620")
 .Case("630", "pwr3")
 .Case("G3", "g3")
-.Case("7400", "7400")
 .Case("G4", "g4")
-.Case("7450", "7450")
 .Case("G4+", "g4+")
-.Case("750", "750")
 .Case("8548", "e500")
-.Case("970", "970")
 .Case("G5", "g5")
-.Case("a2", "a2")
-.Case("e500", "e500")
-.Case("e500mc", "e500mc")
-.Case("e5500", "e5500")
 .Case("power3", "pwr3")
 .Case("power4", "pwr4")
 .Case("power5", "pwr5")
@@ -71,23 +70,13 @@ std::string ppc::getPPCTargetCPU(const ArgList &Args) {
 .Case("power9", "pwr9")
 .Case("power10", "pwr10")
 .Case("future", "future")
-.Case("pwr3", "pwr3")
-.Case("pwr4", "pwr4")
-.Case("pwr5", "pwr5")
-.Case("pwr5x", "pwr5x")
-.Case("pwr6", "pwr6")
-.Case("pwr6x", "pwr6x")
-.Case("pwr7", "pwr7")
-.Case("pwr8", "pwr8")
-.Case("pwr9", "pwr9")
-.Case("pwr10", "pwr10")
 .Case("powerpc", "ppc")
 .Case("powerpc64", "ppc64")
 .Case("powerpc64le", "ppc64le")
-.Default("");
+.Default(CPUName.data());
   }
 
-  return "";
+  return getPPCGenericTargetCPU(T);
 }
 
 const char *ppc::getPPCAsmModeForCPU(StringRef Name) {

diff  --git a/clang/lib/Driver/ToolChains/Arch/PPC.h 
b/clang/lib/Driver/ToolChains/Arch/PPC.h
index e1c943955e812..cd2b47d392b65 100644
--- a/clang/lib/Driver/ToolChains/Arch/PPC.h
+++ b/clang/lib/Driver/ToolChains/Arch/PPC.h
@@ -35,7 +35,8 @@ enum class ReadGOTPtrMode {
 
 FloatABI getPPCFloatABI(const Driver &D, const llvm::opt::ArgList &Args);
 
-std::string getPPCTargetCPU(const llvm::opt::ArgList &Args);
+std::string getPPCTargetCPU(const llvm::opt::ArgList &Args,
+const llvm::Triple &T);
 const char *getPPCAsmModeForCPU(StringRef Name);
 ReadGOTPtrMode getPPCReadGOTPtrMode(const Driver &D, const llvm::Triple 
&Triple,
  

[PATCH] D139935: [NFC] [Doc] Fix example for AnnotateTypeDocs

[Xiang Li via Phabricator via cfe-commits]

python3kgae created this revision.
python3kgae added reviewers: mboehme, aaron.ballman, erichkeane.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Change clang::annotate into clang::annotate_type.

The example will get error like
error: 'annotate' attribute cannot be applied to types
int* [[clang::annotate("category1", "foo", 1)]] 
f(int[[clang::annotate("category2")]] *);


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139935

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6679,7 +6679,7 @@
 
 .. code-block:: c++
 
-  int* [[clang::annotate("category1", "foo", 1)]] 
f(int[[clang::annotate("category2")]] *);
+  int* [[clang::annotate_type("category1", "foo", 1)]] 
f(int[[clang::annotate_type("category2")]] *);
 
 The attribute does not have any effect on the semantics of the type system,
 neither type checking rules, nor runtime semantics. In particular:


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6679,7 +6679,7 @@
 
 .. code-block:: c++
 
-  int* [[clang::annotate("category1", "foo", 1)]] f(int[[clang::annotate("category2")]] *);
+  int* [[clang::annotate_type("category1", "foo", 1)]] f(int[[clang::annotate_type("category2")]] *);
 
 The attribute does not have any effect on the semantics of the type system,
 neither type checking rules, nor runtime semantics. In particular:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp requested changes to this revision.
carlosgalvezp added a comment.
This revision now requires changes to proceed.

Looking good! Some minor comments




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp:40
+
+  diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+}

Perhaps it would be good with a diagnostic that gives more actionable feedback? 
For example "avoid capturing coroutines in a lambda"



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h:18
+
+/// The normal usage of captures in lambdas are problematic when the lambda is 
a
+/// coroutine because the captures are destroyed after the first suspension

Start the documentation describing "what" the check warns for (here you explain 
the "why"). Make sure this text is in sync with the .rst documentation.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h:28
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;

Limit to C++20 using isLanguageVersionSupported



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:121
+
+  Adds check for cpp core guideline: "CP.51: Do not use capturing lambdas that
+  are coroutines."

C++ Core Guidelines



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.rst:23
+All of the cases above will trigger the warning. However, implicit captures
+do not trigger the warning unless the body of the lambda uses the capture.
+For example, the following do not trigger the warning.

Mention that the check implements rule CP.51 of C++ Core Guidelines, and add a 
link to the that guideline (see other C++ Core Guidelines checks as an example).



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.rst:24
+do not trigger the warning unless the body of the lambda uses the capture.
+For example, the following do not trigger the warning.
+

does



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:180
`concurrency-thread-canceltype-asynchronous 
`_,
+   `cppcoreguidelines-avoid-capturing-lambda-coroutines 
`_, "Yes"
`cppcoreguidelines-avoid-const-or-ref-data-members 
`_,

Remove, since this check does not provide fix-its.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-capturing-lambda-coroutines.cpp:1-2
+// RUN: %check_clang_tidy -std=c++20-or-later %s 
cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
+// RUN:   -isystem %S/readability/Inputs/identifier-naming/system
+

I think it's easier to read and reason about if you keep it in one line.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-capturing-lambda-coroutines.cpp:2
+// RUN: %check_clang_tidy -std=c++20-or-later %s 
cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
+// RUN:   -isystem %S/readability/Inputs/identifier-naming/system
+

It's strange to have this test depend on input data from another module - 
create a Inputs folder inside the cppcoreguidelines folder and copy 
coroutines.h there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137514

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

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp:40
+
+  diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+}

carlosgalvezp wrote:
> Perhaps it would be good with a diagnostic that gives more actionable 
> feedback? For example "avoid capturing coroutines in a lambda"
I meant to remove this comment since it's rather nitpick, please ignore :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137514

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

[PATCH] D139937: [clang-format] fix typo in doc for SLS_Inline

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght created this revision.
Backl1ght added reviewers: HazardyKnusperkeks, MyDeveloperDay.
Backl1ght added a project: clang-format.
Herald added a project: All.
Backl1ght requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139937

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -688,7 +688,7 @@
 ///   };
 /// \endcode
 SLS_Empty,
-/// Merge lambda into a single line if argument of a function.
+/// Merge lambda into a single line if argument of a function is empty.
 /// \code
 ///   auto lambda = [](int a) {
 ///   return a;
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1191,7 +1191,7 @@
   };
 
   * ``SLS_Inline`` (in configuration: ``Inline``)
-Merge lambda into a single line if argument of a function.
+Merge lambda into a single line if argument of a function is empty.
 
 .. code-block:: c++
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -688,7 +688,7 @@
 ///   };
 /// \endcode
 SLS_Empty,
-/// Merge lambda into a single line if argument of a function.
+/// Merge lambda into a single line if argument of a function is empty.
 /// \code
 ///   auto lambda = [](int a) {
 ///   return a;
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1191,7 +1191,7 @@
   };
 
   * ``SLS_Inline`` (in configuration: ``Inline``)
-Merge lambda into a single line if argument of a function.
+Merge lambda into a single line if argument of a function is empty.
 
 .. code-block:: c++
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139749: Headers: make a couple of builtins non-static

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a subscriber: STL_MSFT.
compnerd added a comment.

@fsb4000 is my reading correct that MSVC will look into trying to handle 
`static inline` even though it is a GNUism?  I wonder if we should consider 
limiting the use of `static inline` to C mode rather than including C++.  I 
also wonder if I can loosen it similarly to avoid the issue I'm running into 
(which @STL_MSFT correctly identified - modules).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139749

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

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:12
+  auto lambda1 = []  {}; // expected-error {{default 
argument references local variable x_constexpr of enclosing function}}
+  auto lambda2 = []  {}; // expected-error {{default 
argument references local variable x_static of enclosing function}}
+  auto lambda3 = []  {}; // expected-error {{default 
argument references local variable x_const of enclosing function}}

MSVC and Clang currently accept `x_static`. The resolution of [[ 
https://cplusplus.github.io/CWG/issues/2346.html | CWG 2346 ]] suggests that 
this case (and maybe `x_constexpr`) should be accepted following the adoption 
of [[ https://wg21.link/p0588r1 | P0588R1 (Simplifying implicit lambda capture) 
]]. However, it isn't obvious to me that P0588R1 actually addresses this. It 
might be worth following up with CWG to understand what the intent is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139400

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

[PATCH] D139938: [clang] Don't spuriously pass -stdlib=libc++ to CC1 on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: arphaman.
Herald added a project: All.
ldionne requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Previously, we would be passing down -stdlib=libc++ from the Driver
to CC1 whenever the default standard library on the platform was libc++,
even if -stdlib= had not been passed to the Driver. This meant that we
would pass -stdlib=libc++ in nonsensical circumstances, such as when
compiling C code.

This logic had been added in b534ce46bd40 to make sure that header
search paths were set up properly. However, since libc++ is now the
default Standard Library on Darwin, passing this explicitly is not
required anymore. Indeed, if no -stdlib= is specified, CC1 will end
up using libc++ if it queries which standard library to use, without
having to be told.

Not passing -stdlib= at all to CC1 on Darwin should become possible
once CC1 stops relying on it to set up framework search paths.

Furthermore, this commit also removes a diagnostic checking whether the
deployment target is too old to support libc++. Nowadays, all supported
deployment targets use libc++ and compiling with libstdc++ is not
supported anymore. The Driver was the wrong place to issue this
diagnostic since it doesn't know whether libc++ will actually be linked
against (e.g. C vs C++), which would lead to spurious diagnostics.
Given that these targets are not supported anymore, we simply drop
the diagnostic instead of trying to refactor it into CC1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139938

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-stdlib-dont-pass-in-c.c
  clang/test/Driver/darwin-stdlib.cpp

Index: clang/test/Driver/darwin-stdlib.cpp
===
--- clang/test/Driver/darwin-stdlib.cpp
+++ clang/test/Driver/darwin-stdlib.cpp
@@ -1,10 +1,29 @@
-// This test will fail if CLANG_DEFAULT_CXX_STDLIB is set to libstdc++.
-// XFAIL: default-cxx-stdlib=libstdc++
+// Make sure that the Driver passes down -stdlib=libc++ to CC1 when specified explicitly.
+//
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch arm64  -miphoneos-version-min=7.0 -stdlib=libc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/  -mmacosx-version-min=10.9  -stdlib=libc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 -stdlib=libc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k-stdlib=libc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBCXX %s
+// CHECK-LIBCXX: "-stdlib=libc++"
+// CHECK-LIBCXX-NOT: "-stdlib=libstdc++"
 
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch arm64 -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.9 %s -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k %s -### 2>&1 | FileCheck %s
+// Make sure that the Driver passes down -stdlib=libstdc++ to CC1 when specified explicitly. Note that this
+// shouldn't really be used on Darwin because libstdc++ is not supported anymore, but we still pin down the
+// Driver behavior for now.
+//
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch arm64  -miphoneos-version-min=7.0 -stdlib=libstdc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBSTDCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/  -mmacosx-version-min=10.9  -stdlib=libstdc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBSTDCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 -stdlib=libstdc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBSTDCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k-stdlib=libstdc++ %s -### 2>&1 | FileCheck --check-prefix=CHECK-LIBSTDCXX %s
+// CHECK-LIBSTDCXX: "-stdlib=libstdc++"
+// CHECK-LIBSTDCXX-NOT: "-

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

As this is a rather simple cleanup, I'll go ahead and merge this next week 
unless there is any objection until then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138316

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

ChuanqiXu wrote:
> dblaikie wrote:
> > tahonermann wrote:
> > > ChuanqiXu wrote:
> > > > dblaikie wrote:
> > > > > Do we do that for `-o` today? (eg: if you try to `-o` and specify the 
> > > > > input file name, such that the output would overwrite the input, what 
> > > > > happens? I'm not sure - but I guess it's OK to do whatever that is 
> > > > > for this case too)
> > > > > Do we do that for -o today? (eg: if you try to -o and specify the 
> > > > > input file name, such that the output would overwrite the input, what 
> > > > > happens? I'm not sure - but I guess it's OK to do whatever that is 
> > > > > for this case too)
> > > > 
> > > > In this case, the input file will be overwrite and no warning shows. So 
> > > > it looks like we didn't take special treatment here. So I remove the 
> > > > FIXME.
> > > Basing the location of the module output on the presence of `-o` sounds 
> > > confusing to me. Likewise, defaulting to writing next to the input file 
> > > as opposed to the current directory (where a `.o` file is written by 
> > > default) sounds wrong. I think this option should be handled similarly to 
> > > `-o`; it should accept a path and:
> > >   - If an absolute path is provided, write to that location.
> > >   - If a relative path is provided, write to that location relative to 
> > > the current working directory.
> > > Leave it up to the user or build system to ensure that the `.o` and 
> > > `.pcm` file locations coincide if that is what they want. In general, I 
> > > don't expect colocation of `.o` and `.pcm` files to be what is desired.
> > > 
> > > 
> > @tahonermann there's precedent for this with Split DWARF (.dwo files go 
> > next to the .o file) & I'd argued for possibly only providing this 
> > behavior, letting consumers move files elsewhere if they needed to, but got 
> > voted down there.
> > 
> > I do think this is a reasonable default, though. Users and build systems 
> > have the option to pass a path to place the .pcm somewhere else (in the 
> > follow-up patch to this one).
> @tahonermann here is another patch which implements the behavior you 
> described: https://reviews.llvm.org/D137059
I'm ok with a default that consistently writes the PCM relative to the location 
of the `.o` file (if not overridden with an absolute path). What I'm not ok 
with is having the default be next to the `.o` file if `-o` is specified and 
next to the input file if `-o` is not specified. I don't think writing the PCM 
relative to the input file is a good default in any case. If `-o` is not 
specified, then I think it should be written relative to the current working 
directory; which matches where the `.o` file will be written.


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

https://reviews.llvm.org/D137058

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

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 482489.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/Sema/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
  llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll

Index: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64 -mattr=+v8.9a --global-isel=1 --global-isel-abort=1 < %s | FileCheck %s
+
+@a = internal global ptr null, align 8
+@b = external global ptr, align 8
+
+define void @test(ptr %i, i32 %j) nounwind ssp {
+entry:
+  ; CHECK-LABEL: @test
+  %j.addr = alloca i32, align 4
+  store i32 %j, ptr %j.addr, align 4, !tbaa !0
+  %tmp = bitcast ptr %j.addr to ptr
+
+  %i.next = getelementptr i8, ptr %i, i64 2
+
+  ; Verify prefetching works for all the different kinds of pointers we might
+  ; want to prefetch.
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %tmp, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfum pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr %i.next, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @a, i32 0, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1keep,
+  call void @llvm.aarch64.prefetch(ptr @b, i32 0, i32 0, i32 0, i32 1)
+
+  ; Verify that we can generate every single valid prefetch value.
+
+  ; CHECK: prfm pstl1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 1, i32 0, i32 0, i32 1)
+
+  ; CHECK: prfm pldl2keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 1, i32 0, i32 1)
+
+  ; CHECK: prfm pldl3keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 2, i32 0, i32 1)
+
+  ; CHECK: prfm pldslckeep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 3, i32 0, i32 1)
+
+  ; CHECK: prfm pldl1strm,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 1, i32 1)
+
+  ; CHECK: prfm plil1keep,
+  call void @llvm.aarch64.prefetch(ptr null, i32 0, i32 0, i32 0, i32 0)
+
+  ret void
+}
+
+declare void @llvm.aarch64.prefetch(ptr readonly, i32 immarg, i32 immarg, i32 immarg, i32 immarg) #0
+
+attributes #0 = { inaccessiblemem_or_argmemonly nounwind willreturn }
+
+!0 = !{!"int", !1}
+!1 = !{!"omnipotent char", !2}
+!2 = !{!"Simple C/C++ TBAA"}
+!3 = !{!"any pointer", !1}
Index: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1063,6 +1063,24 @@
 MI.eraseFromParent();
 return true;
   }
+  case Intrinsic::aarch64_prefetch: {
+MachineIRBuilder MIB(MI);
+auto &AddrVal = MI.getOperand(1);
+
+int64_t IsWrite = MI.getOperand(2).getImm();
+int64_t Target = MI.getOperand(3).getImm();
+int64_t IsStream = MI.getOperand(4).getImm();
+int64_t IsData = MI.getOperand(5).getImm();
+
+unsigned PrfOp = (IsWrite << 4) |// Load/Store bit
+ (!IsData << 3) |// IsDataCache bit
+ (Target << 1) | // Cache level bits
+ (unsigned)IsStream; // Stream bit
+
+MIB.buildInstr(AArch64::G_PREFETCH).addImm(PrfOp).add(AddrVal);
+MI.eraseFromParent();
+return true;
+  }
   }
 
   return true;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.h
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -956,6 +956,7 @@
 
   SDValue LowerINTRINSIC_W_CHAIN(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerINTRINSIC_VOID(SDValue Op, SelectionDAG &DAG) const;
 
   bool
   isEligibleForTailCallOptimization(const CallLoweringInfo &CLI) const;
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -1189,9 +1189,6 @@
 }
   }
 
-  if (Subtarget->hasSME())
-setOperationAction(ISD::INTRINSIC_VOID, MVT::Other, Custom);
-
 

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

[Sam Elliott via Phabricator via cfe-commits]

lenary added reviewers: paquette, aemerson.
lenary added a comment.

Adding Jessica and Amara as this affects GlobalISel for AArch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139443

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

[PATCH] D139937: [clang-format] fix typo in doc for SLS_Inline

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght added a comment.

OMG! I just looked into git history and last edition of this line happened 
about 4 years ago. Am I doing it wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139937

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

[PATCH] D134130: [clangd] Add doxygen parsing for Hover [1/3]

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Hi!

Sorry for letting these series of patches sit around without any comments. We 
were having some discussions internally to both understand the value 
proposition and its implications on the infrastructure.
So it'd help a lot if you can provide some more information for use cases, that 
way we can find a nice scope for this functionality to make sure it provides 
most of the value and doesn't introduce an unnecessary complexity into rest of 
the infrastructure and also we should try not to regress indexing of projects 
that don't have doxygen comments.

So first of all, what are the exact use cases you're planning to 
address/improve with support for doxygen parsing of comments? Couple that comes 
to mind:

- obtaining docs about params & return value
- stripping doxygen commands
- treating brief/detail/warning/note differently
- formatting text within comments (bold etc)
- getting linebreaks/indent right clangd#1040 


any other use cases that you believe are important?

as you might've noticed, this list already talks about dealing with certain 
doxygen commands (but not all).
that list is gathered by counting occurrences of those commands in a codebase 
with lots of open-source third_party code. findings and some initial ideas look 
like this:

- \brief: ~70k occurrences
  - common but usefulness in practice is unclear
  - can infer for non-doxy too (e.g. first sentence of a regular documentation)
  - maybe just strip (or merge into regular documentation)?
- \return[s]: 30k occurrences
  - unclear if worth separation in hover, because it might be tied to rest of 
the documentation (re-ordering concerns)
  - can infer for non-doxy maybe?
  - probably just strip the command and keep the rest as-is.
- \param: 28k occurrences
  - useful for signature help. maybe hover on func calls
  - probably worth storing in a structured manner.
- \detail[s]: 2k
- \p: 20k
- \code: 1k
- \warning: 2k
- \note: 9k
  - (for all of the above) just introduce as formatted text?

what do you think about those conclusions? any other commands that you seem 
worth giving a thought?
One important concern we've noticed around this part is, re-ordering comment 
components might actually hinder readability. as the comments are usually 
written with the assumption that they'll be read from top to bottom, but if we 
re-order them during presentation (e.g. hover has its own layout) we might 
start referring to concepts/entities in documentation before they're 
introduced. so we believe it's important to avoid any sort of re-ordering. this 
is one of the big down sides for preserving parameter comments in a structured 
way.

another important thing to consider is trying to heuristically infer some of 
these fields for non-doxygen code bases as well. that way we can provide a 
similar experience for both.

some other things to discuss about the design overall:

- How to store the extra information?
  - Proposal from our side would be to introduce structured storage for the 
pieces we want (limited), and keep the rest as part of main documentation text 
while doing stripping/reformatting.
- What to use as a parser?
  - Clang's doxygen parser actually looks like a great piece of code to re-use, 
it's unfortunate that it can issue diagnostics, requires AST etc. It'd be great 
to refactor that into a state where we can use it without any AST or 
diagnostics, and a minimal SourceManager (this seems feasible to achieve at 
first glance, as most of these inputs seem to be optional or used in 1 or 2 
places).
  - we still need to make sure performance and behaviour on non-doxygen is 
reasonable though. do you have any numbers here?
- How to store in the index?
  - If we can strip the parser off the dependencies on an astcontext, 
diagnostics etc. the best option would be to just store as raw text and run the 
whole pipeline on demand (e.g. do the doxygen parsing and markdown-ization 
afterwards). This is the simplest approach as it keeps index interfaces the 
same.

Happy to move the discussion to some other medium as well, if you would like to 
have them in discourse/github etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134130

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

[PATCH] D139450: Warn about unsupported ibmlongdouble

[Tulio Magno Quites Machado Filho via Phabricator via cfe-commits]

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

Thanks @qiucf ! I don't have commit access yet. Could you land this patch for 
me, please?
Please use “Tulio Magno Quites Machado Filho tul...@redhat.com” to commit the 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139450

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

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

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

steakhal added a comment.

The `StaticAnalyzer` portion looks good to me AFAICT.




Comment at: clang/lib/StaticAnalyzer/Core/CallDescription.cpp:39
 ento::CallDescription::CallDescription(CallDescriptionFlags Flags,
-   ArrayRef QualifiedName,
+   ArrayRef QualifiedName,
MaybeCount RequiredArgs /*= None*/,

Maybe we could restrict it even more to `StringLiteral`. The same applies to 
the other ctor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139881

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

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

[Ben Boeckel via Phabricator via cfe-commits]

ben.boeckel added a comment.

This worked for CMake in a previous version, so is good enough on the 
functionality front. I'll retry with the current state to verify.


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

https://reviews.llvm.org/D139168

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

tahonermann wrote:
> ChuanqiXu wrote:
> > dblaikie wrote:
> > > tahonermann wrote:
> > > > ChuanqiXu wrote:
> > > > > dblaikie wrote:
> > > > > > Do we do that for `-o` today? (eg: if you try to `-o` and specify 
> > > > > > the input file name, such that the output would overwrite the 
> > > > > > input, what happens? I'm not sure - but I guess it's OK to do 
> > > > > > whatever that is for this case too)
> > > > > > Do we do that for -o today? (eg: if you try to -o and specify the 
> > > > > > input file name, such that the output would overwrite the input, 
> > > > > > what happens? I'm not sure - but I guess it's OK to do whatever 
> > > > > > that is for this case too)
> > > > > 
> > > > > In this case, the input file will be overwrite and no warning shows. 
> > > > > So it looks like we didn't take special treatment here. So I remove 
> > > > > the FIXME.
> > > > Basing the location of the module output on the presence of `-o` sounds 
> > > > confusing to me. Likewise, defaulting to writing next to the input file 
> > > > as opposed to the current directory (where a `.o` file is written by 
> > > > default) sounds wrong. I think this option should be handled similarly 
> > > > to `-o`; it should accept a path and:
> > > >   - If an absolute path is provided, write to that location.
> > > >   - If a relative path is provided, write to that location relative to 
> > > > the current working directory.
> > > > Leave it up to the user or build system to ensure that the `.o` and 
> > > > `.pcm` file locations coincide if that is what they want. In general, I 
> > > > don't expect colocation of `.o` and `.pcm` files to be what is desired.
> > > > 
> > > > 
> > > @tahonermann there's precedent for this with Split DWARF (.dwo files go 
> > > next to the .o file) & I'd argued for possibly only providing this 
> > > behavior, letting consumers move files elsewhere if they needed to, but 
> > > got voted down there.
> > > 
> > > I do think this is a reasonable default, though. Users and build systems 
> > > have the option to pass a path to place the .pcm somewhere else (in the 
> > > follow-up patch to this one).
> > @tahonermann here is another patch which implements the behavior you 
> > described: https://reviews.llvm.org/D137059
> I'm ok with a default that consistently writes the PCM relative to the 
> location of the `.o` file (if not overridden with an absolute path). What I'm 
> not ok with is having the default be next to the `.o` file if `-o` is 
> specified and next to the input file if `-o` is not specified. I don't think 
> writing the PCM relative to the input file is a good default in any case. If 
> `-o` is not specified, then I think it should be written relative to the 
> current working directory; which matches where the `.o` file will be written.
Summary of the consensus of the Clang Modules WG:
  # If `-fmodule-output=` is provided, the PCM is written to the indicated file 
(relative to the current working directory for a relative path).
  # If `-fmodule-output` is provided, the PCM is written relative to the 
location of the `.o` file (the current working directory by default and the 
location indicated by `-o` otherwise).


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

https://reviews.llvm.org/D137058

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

[PATCH] D134130: [clangd] Add doxygen parsing for Hover [1/3]

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

Thanks for the detailed feedback! Unfortunately, I’m sick right now, so I 
probably won’t be able to give a detailed answer until after Christmas.

In D134130#3992116 , @kadircet wrote:

> Happy to move the discussion to some other medium as well, if you would like 
> to have them in discourse/github etc.

Yeah let’s maybe copy your comment back to the GitHub Issue and discuss over 
there, it seems the issue has lots of subscribers already who perhaps also have 
some more ideas/usecases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134130

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Some reports can be found here 
 (if the link 
works and the data does not expire), the runs stored on 2022-12-09.

Results appeared only projects "postgres" and "curl" (from 
memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres,libwebm) from 
checkers **StdCLibraryFunctionArgs** and **Errno**.

On the postgres results, the second is one that can be fixed in the checker 
(add special cases to `StdLibraryFunctionsChecker` for zero `len` or `size` 
`fread` and `fwrite` arguments). The others are false positives because the 
error path is impossible because implicit constraints (what is not known to the 
analyzer) on variables.

These curl 

 results look faulty, the last `fclose` call looks not recognized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D139928: [clang][DebugInfo] Backport emitting DW_AT_default_value for default template arguments

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

(This should be committed in two parts - the LLVM part first, then the Clang 
part - since they can be separated, they should be - but happy to review it 
altogether)




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1856
+
+bool DwarfUnit::shouldEmitTemplateDefaultAttr() const {
+  assert(Asm != nullptr);

Maybe this could be a more general function? It could take a version and then 
have a general name "isCompatibleWithVersion(X)" and be used for other strict 
DWARF+version checks?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1857-1858
+bool DwarfUnit::shouldEmitTemplateDefaultAttr() const {
+  assert(Asm != nullptr);
+  assert(DD != nullptr);
+  return !Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= 5;

You can drop these - there's tons of non-null assumptions throughout the 
codebase, most aren't asserted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139928

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

[PATCH] D137652: Remove mandatory define of optional features macros for OpenCL C 3.0

[Finlay Marno via Phabricator via cfe-commits]

FMarno added a comment.

Thank you for your comments. Unfortunately I won't be able to implement these 
changes because of time constraints in the current project, sorry if I've 
wasted your time.
On the bright side I feel like I've learned something so thank you for that. I 
hope I can make a proper contribution in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137652

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

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

This has already been merged, but I'm commenting just to note (continued) 
approval of the most recent changes. Looks great, Corentin!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861

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

[PATCH] D139937: [clang-format] fix typo in doc for SLS_Inline

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

I'm not sure if that is correct, as that doesn't fit the example (unless the 
example is wrong)

It feels like its saying merge if its an argument of a function (sort in this 
case)

sort(a.begin(), a.end(), **[]() { return x < y; }**);

The lambda is the argument, not that the lamda has no arguments?  either way 
something isn't correct by making that change (either the description was wrong 
as well as the example or now the description doesn't match the example)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139937

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

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:1265
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;

Backl1ght wrote:
> MyDeveloperDay wrote:
> > why would the default be true, is that what happens today?
> yes
just to clarify so I'm sure (without me having to try it myself), if you hadn't 
introduce this option it would be the equivalent of true.

The only reason I say is we get complained at when we change the default from 
not doing something to doing something. Even if that means before we left it 
alone.

So mostly we normally find the options go through an evolution from   
bool->enum->struct, sometimes it can be better to introduce an enum so we can 
have "Leave" as the default

in such circumstances you let the old behaviour be the default, that way we 
know. That previously unformatted compound statements won't be touch in any 
way. 

```
else if (Style.AllowShortCompoundRequirementOnASingleLine != Leave  && ...)
{
```

Users then have to "positively" buy into your style change one way or another. 
Rather than us imposing a default even if that default seems perfectly 
reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834

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

[PATCH] D139786: [clang-format] AllowShortRequiresExpressionOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

ditto my concerns with D139834: [clang-format] 
AllowShortCompoundRequirementOnASingleLine  
as to if we should have a Leave option


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

https://reviews.llvm.org/D139786

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

[PATCH] D139937: [clang-format] fix typo in doc for SLS_Inline

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght added a comment.

Seems like I have misunderstood it, after some test I'm sure that it means 
merge if its an argument of a function.

I think there are something todo to make the doc more clearly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139937

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

[PATCH] D139937: [clang-format] make doc for SLS_Inline more clearly

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght updated this revision to Diff 482530.
Backl1ght retitled this revision from "[clang-format] fix typo in doc for 
SLS_Inline" to "[clang-format] make doc for SLS_Inline more clearly".

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

https://reviews.llvm.org/D139937

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -688,12 +688,12 @@
 ///   };
 /// \endcode
 SLS_Empty,
-/// Merge lambda into a single line if argument of a function.
+/// Merge lambda into a single line if the lambda is argument of a 
function.
 /// \code
-///   auto lambda = [](int a) {
-///   return a;
+///   auto lambda = [](int x, int y) {
+///   return x < y;
 ///   };
-///   sort(a.begin(), a.end(), []() { return x < y; });
+///   sort(a.begin(), a.end(), [](int x, int y) { return x < y; });
 /// \endcode
 SLS_Inline,
 /// Merge all lambdas fitting on a single line.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1191,14 +1191,14 @@
   };
 
   * ``SLS_Inline`` (in configuration: ``Inline``)
-Merge lambda into a single line if argument of a function.
+Merge lambda into a single line if the lambda is argument of a function.
 
 .. code-block:: c++
 
-  auto lambda = [](int a) {
-  return a;
+  auto lambda = [](int x, int y) {
+  return x < y;
   };
-  sort(a.begin(), a.end(), []() { return x < y; });
+  sort(a.begin(), a.end(), [](int x, int y) { return x < y; });
 
   * ``SLS_All`` (in configuration: ``All``)
 Merge all lambdas fitting on a single line.


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -688,12 +688,12 @@
 ///   };
 /// \endcode
 SLS_Empty,
-/// Merge lambda into a single line if argument of a function.
+/// Merge lambda into a single line if the lambda is argument of a function.
 /// \code
-///   auto lambda = [](int a) {
-///   return a;
+///   auto lambda = [](int x, int y) {
+///   return x < y;
 ///   };
-///   sort(a.begin(), a.end(), []() { return x < y; });
+///   sort(a.begin(), a.end(), [](int x, int y) { return x < y; });
 /// \endcode
 SLS_Inline,
 /// Merge all lambdas fitting on a single line.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1191,14 +1191,14 @@
   };
 
   * ``SLS_Inline`` (in configuration: ``Inline``)
-Merge lambda into a single line if argument of a function.
+Merge lambda into a single line if the lambda is argument of a function.
 
 .. code-block:: c++
 
-  auto lambda = [](int a) {
-  return a;
+  auto lambda = [](int x, int y) {
+  return x < y;
   };
-  sort(a.begin(), a.end(), []() { return x < y; });
+  sort(a.begin(), a.end(), [](int x, int y) { return x < y; });
 
   * ``SLS_All`` (in configuration: ``All``)
 Merge all lambdas fitting on a single line.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139928: [clang][DebugInfo] Backport emitting DW_AT_default_value for default template arguments

[Michael Buch via Phabricator via cfe-commits]

Michael137 updated this revision to Diff 482533.
Michael137 added a comment.

- Split clang and llvm changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139928

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-parameter.cpp


Index: clang/test/CodeGenCXX/debug-info-template-parameter.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-parameter.cpp
+++ clang/test/CodeGenCXX/debug-info-template-parameter.cpp
@@ -3,6 +3,7 @@
 
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: 
![[F1_TYPE:[0-9]+]]
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2015,7 +2015,7 @@
 } break;
 case TemplateArgument::Integral: {
   llvm::DIType *TTy = getOrCreateType(TA.getIntegralType(), Unit);
-  if (Args.TList && CGM.getCodeGenOpts().DwarfVersion >= 5)
+  if (Args.TList)
 if (auto *templateType = dyn_cast_or_null(
 Args.TList->getParam(i)))
   if (templateType->hasDefaultArgument() &&


Index: clang/test/CodeGenCXX/debug-info-template-parameter.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-parameter.cpp
+++ clang/test/CodeGenCXX/debug-info-template-parameter.cpp
@@ -3,6 +3,7 @@
 
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX17
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: ![[F1_TYPE:[0-9]+]]
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2015,7 +2015,7 @@
 } break;
 case TemplateArgument::Integral: {
   llvm::DIType *TTy = getOrCreateType(TA.getIntegralType(), Unit);
-  if (Args.TList && CGM.getCodeGenOpts().DwarfVersion >= 5)
+  if (Args.TList)
 if (auto *templateType = dyn_cast_or_null(
 Args.TList->getParam(i)))
   if (templateType->hasDefaultArgument() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139953: [llvm][DebugInfo] Backport DW_AT_default_value for template args

[Michael Buch via Phabricator via cfe-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, dblaikie.
Herald added a subscriber: hiraditya.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

**Summary**

Starting with DWARFv5, DW_AT_default_value can be used to indicate
that a template argument has a default value. With this patch LLVM
will emit the this attribute earlier versions of DWARF, unless
compiling with -gstrict-dwarf.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139953

Files:
  clang/test/CodeGenCXX/debug-info-template-parameter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h


Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -350,6 +350,10 @@
 
   virtual bool isDwoUnit() const = 0;
   const MCSymbol *getCrossSectionRelativeBaseAddress() const override;
+
+  /// Returns 'true' if the current DwarfVersion is compatible
+  /// with the specified \p Version.
+  bool isCompatibleWithVersion(uint16_t Version) const;
 };
 
 class DwarfTypeUnit final : public DwarfUnit {
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1060,7 +1060,7 @@
 addType(ParamDIE, TP->getType());
   if (!TP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, TP->getName());
-  if (TP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (TP->isDefault() && isCompatibleWithVersion(5))
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
 }
 
@@ -1074,7 +1074,7 @@
 addType(ParamDIE, VP->getType());
   if (!VP->getName().empty())
 addString(ParamDIE, dwarf::DW_AT_name, VP->getName());
-  if (VP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (VP->isDefault() && isCompatibleWithVersion(5))
 addFlag(ParamDIE, dwarf::DW_AT_default_value);
   if (Metadata *Val = VP->getValue()) {
 if (ConstantInt *CI = mdconst::dyn_extract(Val))
@@ -1852,3 +1852,7 @@
 void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) {
   DD->getAddressPool().resetUsedFlag(true);
 }
+
+bool DwarfUnit::isCompatibleWithVersion(uint16_t Version) const {
+  return !Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= Version;
+}
Index: clang/test/CodeGenCXX/debug-info-template-parameter.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-parameter.cpp
+++ clang/test/CodeGenCXX/debug-info-template-parameter.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s 
-O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s 
--check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -gstrict-dwarf -triple x86_64 
%s -O0 -disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | 
llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,STRICT
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: 
![[F1_TYPE:[0-9]+]]
@@ -21,6 +22,24 @@
 // PRE17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i8 1)
 // CXX17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, 
defaulted: true, value: i1 true)
 
+// DWARF-DUMP: DW_TAG_class_type
+// DWARF-DUMP:   DW_AT_name  ("foo")
+// DWARF-DUMP:   DW_TAG_template_type_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "char")
+// DWARF-DUMP-NEXT:DW_AT_name("T")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+// DWARF-DUMP:   DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "int")
+// DWARF-DUMP-NEXT:DW_AT_name("i")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+// DWARF-DUMP:   DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:DW_AT_type({{.*}} "bool")
+// DWARF-DUMP-NEXT:DW_AT_name("b")
+// DWARFv4-NEXT:   DW_AT_default_value   (true)
+// STRICT-NOT: DW_AT_default_value   (true)
+
 template 
 class foo {
 };


Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
===

[PATCH] D139928: [clang][DebugInfo] Backport emitting DW_AT_default_value for default template arguments

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

In D139928#3992233 , @dblaikie wrote:

> (This should be committed in two parts - the LLVM part first, then the Clang 
> part - since they can be separated, they should be - but happy to review it 
> altogether)

Split the review: https://reviews.llvm.org/D139953

Updated the version check to be a bit more general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139928

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