Re: [PATCH] D123488: [clangd] IncludeCleaner: Add filtering mechanism

[Kirill Bobyrev via cfe-commits]

@sammccall ping

On Tue, Apr 12, 2022 at 4:39 PM Kirill Bobyrev via Phabricator <
revi...@reviews.llvm.org> wrote:

> kbobyrev added a comment.
>
> Oops, sorry, I linked the wrong revision; here's the prototype: we plan to
> start rolling it out gradually https://reviews.llvm.org/D122677 and then
> have a common library that we could share between both clangd and
> Clang-Tidy (and potentially other tools).
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D123488/new/
>
> https://reviews.llvm.org/D123488
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cd149db - [NFC] Remove unused variable

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-04-19T15:19:40+08:00
New Revision: cd149dbf8ed8334e68e0f69d819bac3d400e8ba7

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

LOG: [NFC] Remove unused variable

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 410fc8285bb65..3ad44d5e2da80 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8294,7 +8294,7 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, 
const VarDecl *VD) {
 }
 
 bool LValueExprEvaluator::VisitCallExpr(const CallExpr *E) {
-  switch (unsigned BuiltinOp = E->getBuiltinCallee()) {
+  switch (E->getBuiltinCallee()) {
   case Builtin::BIas_const:
   case Builtin::BIforward:
   case Builtin::BImove:



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

[PATCH] D123682: [clang-tblgen] Automatically document options values

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

In D123682#3455793 , @aaron.ballman 
wrote:

> In D123682#3454627 , 
> @serge-sans-paille wrote:
>
>> @aaron.ballman Any thoughs on the above suggestion? I'd be happy to adopt 
>> any of those :-)
>
> I'd go with:
>
> More than two choices: ` should be 'return', 'branch', 'full', or 'none'`
> Only two choices: ` should be 'split' or 'single'.`

FWIW, I'd actually use "must" rather than "should", but otherwise I agree with 
this. "should" implies there are cases where it is okay to use a different 
value, which is obviously not the intent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123682

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

[clang] 33ec653 - [clang][lexer] Allow u8 character literal prefixes in C2x

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-04-19T09:57:51+02:00
New Revision: 33ec65305525626d5d93bd794c1c9cfa55d0ca8f

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

LOG: [clang][lexer] Allow u8 character literal prefixes in C2x

Implement N2418 for C2x.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Lex/Lexer.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Lexer/utf8-char-literal.cpp
clang/www/c_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fd4eac5458c0a..c396322f2de57 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -226,6 +226,7 @@ C2x Feature Support
 - Implemented `WG14 N2775 Literal suffixes for bit-precise integers 
`_.
 - Implemented the `*_WIDTH` macros to complete support for
   `WG14 N2412 Two's complement sign representation for C2x 
`_.
+- Implemented `WG14 N2418 Adding the u8 character prefix 
`_.
 
 C++ Language Changes in Clang
 -

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 6e8072fb1b2d9..6fc78cfa8ad95 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -3459,7 +3459,10 @@ bool Lexer::LexTokenInternal(Token &Result, bool 
TokAtPhysicalStartOfLine) {
 MIOpt.ReadToken();
 return LexNumericConstant(Result, CurPtr);
 
-  case 'u':   // Identifier (uber) or C11/C++11 UTF-8 or UTF-16 string literal
+  // Identifier (e.g., uber), or
+  // UTF-8 (C2x/C++17) or UTF-16 (C11/C++11) character literal, or
+  // UTF-8 or UTF-16 string literal (C11/C++11).
+  case 'u':
 // Notify MIOpt that we read a non-whitespace/non-comment token.
 MIOpt.ReadToken();
 
@@ -3493,7 +3496,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool 
TokAtPhysicalStartOfLine) {
ConsumeChar(ConsumeChar(CurPtr, SizeTmp, 
Result),
SizeTmp2, Result),
tok::utf8_string_literal);
-if (Char2 == '\'' && LangOpts.CPlusPlus17)
+if (Char2 == '\'' && (LangOpts.CPlusPlus17 || LangOpts.C2x))
   return LexCharConstant(
   Result, ConsumeChar(ConsumeChar(CurPtr, SizeTmp, Result),
   SizeTmp2, Result),
@@ -3517,7 +3520,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool 
TokAtPhysicalStartOfLine) {
 // treat u like the start of an identifier.
 return LexIdentifierContinue(Result, CurPtr);
 
-  case 'U':   // Identifier (Uber) or C11/C++11 UTF-32 string literal
+  case 'U': // Identifier (e.g. Uber) or C11/C++11 UTF-32 string literal
 // Notify MIOpt that we read a non-whitespace/non-comment token.
 MIOpt.ReadToken();
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3e84961a5079f..bb0bde6a31b50 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3609,6 +3609,8 @@ ExprResult Sema::ActOnCharacterConstant(const Token &Tok, 
Scope *UDLScope) {
   QualType Ty;
   if (Literal.isWide())
 Ty = Context.WideCharTy; // L'x' -> wchar_t in C and C++.
+  else if (Literal.isUTF8() && getLangOpts().C2x)
+Ty = Context.UnsignedCharTy; // u8'x' -> unsigned char in C2x
   else if (Literal.isUTF8() && getLangOpts().Char8)
 Ty = Context.Char8Ty; // u8'x' -> char8_t when it exists.
   else if (Literal.isUTF16())
@@ -3618,7 +3620,8 @@ ExprResult Sema::ActOnCharacterConstant(const Token &Tok, 
Scope *UDLScope) {
   else if (!getLangOpts().CPlusPlus || Literal.isMultiChar())
 Ty = Context.IntTy;   // 'x' -> int in C, 'wxyz' -> int in C++.
   else
-Ty = Context.CharTy;  // 'x' -> char in C++
+Ty = Context.CharTy; // 'x' -> char in C++;
+ // u8'x' -> char in C11-C17 and in C++ without 
char8_t.
 
   CharacterLiteral::CharacterKind Kind = CharacterLiteral::Ascii;
   if (Literal.isWide())

diff  --git a/clang/test/Lexer/utf8-char-literal.cpp 
b/clang/test/Lexer/utf8-char-literal.cpp
index 0ddaabc842257..ababc8b9c21bc 100644
--- a/clang/test/Lexer/utf8-char-literal.cpp
+++ b/clang/test/Lexer/utf8-char-literal.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -fsyntax-only 
-verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -fsyntax-only 
-verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -fsyntax-only 
-verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++1z -fsyntax-only 
-verify %s
 
 int array0[u'ñ' == u'\xf1'? 1 : -1];
@@ -12,4 +13,16 @@ char c = u

[PATCH] D119221: [clang][lexer] Allow u8 character literal prefixes in C2x

[Timm Bäder 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 rG33ec65305525: [clang][lexer] Allow u8 character literal 
prefixes in C2x (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119221

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/Lexer.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/utf8-char-literal.cpp
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -720,7 +720,7 @@
 
   Adding the u8 character prefix
   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2418.pdf";>N2418
-  No
+  Clang 15
 
 
   Remove support for function definitions with identifier lists
Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++1z -fsyntax-only -verify %s
 
 int array0[u'ñ' == u'\xf1'? 1 : -1];
@@ -12,4 +13,16 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
+#elif __STDC_VERSION__ > 202000L
+char a = u8'ñ';  // expected-error {{character too large for enclosing character literal type}}
+char b = u8'\x80';   // ok
+char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}}
+char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
+char e = u8'ሴ';  // expected-error {{character too large for enclosing character literal type}}
+char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
+_Static_assert(
+_Generic(u8'a',
+ default : 0,
+ unsigned char : 1),
+"Surprise!");
 #endif
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3609,6 +3609,8 @@
   QualType Ty;
   if (Literal.isWide())
 Ty = Context.WideCharTy; // L'x' -> wchar_t in C and C++.
+  else if (Literal.isUTF8() && getLangOpts().C2x)
+Ty = Context.UnsignedCharTy; // u8'x' -> unsigned char in C2x
   else if (Literal.isUTF8() && getLangOpts().Char8)
 Ty = Context.Char8Ty; // u8'x' -> char8_t when it exists.
   else if (Literal.isUTF16())
@@ -3618,7 +3620,8 @@
   else if (!getLangOpts().CPlusPlus || Literal.isMultiChar())
 Ty = Context.IntTy;   // 'x' -> int in C, 'wxyz' -> int in C++.
   else
-Ty = Context.CharTy;  // 'x' -> char in C++
+Ty = Context.CharTy; // 'x' -> char in C++;
+ // u8'x' -> char in C11-C17 and in C++ without char8_t.
 
   CharacterLiteral::CharacterKind Kind = CharacterLiteral::Ascii;
   if (Literal.isWide())
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -3459,7 +3459,10 @@
 MIOpt.ReadToken();
 return LexNumericConstant(Result, CurPtr);
 
-  case 'u':   // Identifier (uber) or C11/C++11 UTF-8 or UTF-16 string literal
+  // Identifier (e.g., uber), or
+  // UTF-8 (C2x/C++17) or UTF-16 (C11/C++11) character literal, or
+  // UTF-8 or UTF-16 string literal (C11/C++11).
+  case 'u':
 // Notify MIOpt that we read a non-whitespace/non-comment token.
 MIOpt.ReadToken();
 
@@ -3493,7 +3496,7 @@
ConsumeChar(ConsumeChar(CurPtr, SizeTmp, Result),
SizeTmp2, Result),
tok::utf8_string_literal);
-if (Char2 == '\'' && LangOpts.CPlusPlus17)
+if (Char2 == '\'' && (LangOpts.CPlusPlus17 || LangOpts.C2x))
   return LexCharConstant(
   Result, ConsumeChar(ConsumeChar(CurPtr, SizeTmp, Result),
   SizeTmp2, Result),
@@ -3517,7 +3520,7 @@
 // treat u like the start of an identifier.
 return LexIdentifierContinue(Result, CurPtr);
 
-  case 'U':   // Identifier (Uber) or C11/C++11 UTF-32 string literal
+  case 'U': // Identifier (e.g. Uber) or C11/C++11 UTF-32 string literal
 // Notify MIOpt that we read a non-whitespace/non-comment toke

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

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

steakhal added a comment.

What happens if the import fails? Or alternatively the 
`InitializeImportedDecl(FromD, ToD)` fails?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123685

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

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

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

steakhal added inline comments.



Comment at: clang/include/clang/AST/ASTImporterSharedState.h:69-75
   llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const {
 auto Pos = ImportErrors.find(ToD);
 if (Pos != ImportErrors.end())
   return Pos->second;
 else
   return Optional();
   }

Why does this API accept only non-const pointers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123685

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

[PATCH] D123957: Update the developer policy to mention release notes

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

If it's possible, maybe this could be worded in a way that strongly suggests 
the release notes should ideally be updated in the same patch as the code 
change?
I think we're generally good at doing this for tests. It would be great if we 
could treat release notes the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123957

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

The Arm/AArch64 split and the generated files check looks good to me.

Unless I'm missing an existing check, is it possible to check for the existing 
files too? I guess not nicely because along with the resource headers there are 
lots of internal headers, so we'd have to maintain some list of "non resource" 
headers which means we've just created another thing to maintain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead updated this revision to Diff 423555.
pcwang-thead added a comment.

- Disable sized deallocation for Apple targets.
- Update tests and don't use `-fno-sized-deallocation` any more.
  - With one exception: `clang/test/SemaCXX/builtin-operator-new-delete.cpp`, 
which will generate two notes without line infos if using default settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/delete-two-arg.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/new.cpp
  clang/test/CodeGenCoroutines/coro-alloc-exp-namespace.cpp
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-cleanup-exp-namespace.cpp
  clang/test/CodeGenCoroutines/coro-cleanup.cpp
  clang/test/CodeGenCoroutines/coro-gro-exp-namespace.cpp
  clang/test/CodeGenCoroutines/coro-gro.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp
  clang/test/SemaCXX/builtin-operator-new-delete.cpp
  clang/test/SemaCXX/unavailable_aligned_allocation.cpp
  clang/unittests/StaticAnalyzer/CallEventTest.cpp
  clang/www/cxx_status.html
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp
===
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp
@@ -10,9 +10,10 @@
 
 // UNSUPPORTED: sanitizer-new-delete, c++03, c++11
 
-// NOTE: Clang does not enable sized-deallocation in C++14 and beyond by
-// default. It is only enabled when -fsized-deallocation is given.
-// XFAIL: clang, apple-clang
+// XFAIL: apple-clang
+
+// Sized deallocation was added in macOS 10.12 and aligned OSes.
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
 
 #include 
 #include 
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -578,12 +578,11 @@
 
 
 
-(7): In Clang 3.7 and later, sized deallocation is only enabled
-if the user passes the -fsized-deallocation flag. The user must
-supply definitions of the sized deallocation functions, either by providing them
-explicitly or by using a C++ standard library that does. libstdc++
-added these functions in version 5.0, and libc++ added them in
-version 3.7.
+(7): The user must supply definitions of the sized deallocation
+functions, either by providing them explicitly or by using a C++ standard library
+that does. libstdc++ added these functions in version 5.0, and
+libc++ added them in version 3.7. The user can also use the
+-fno-sized-deallocation option to disable sized deallocation.
 
 
 
Index: clang/unittests/StaticAnalyzer/CallEventTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -81,7 +81,7 @@
 }
   )",
  Diags));
-  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 2\n");
 }
 
 } // namespace
Index: clang/test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- clang/test/SemaCXX/unavailable_aligned_allocation.cpp
+++ clang/test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -74,32 +74,35 @@
 // expected-note@-21 {{if you supply your own aligned allocation functions}}
 // expected-error-re@-22 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is {{only|not}} available on}}
 // expected-note@-23 {{if you supply your own aligned allocation functions}}
-
+#if !defined(ZOS)
 // expected-error-re@-24 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' is {{only|not}} available on}}
 // expected-note@-25 {{if you supply your own aligned allocation functions}}
-
-// expected-error-re@-26 {{aligned allocation function of type 'void *(std::size_t, std::align_val_t, const std::nothrow_t &) noexcept' is {{only|not}} available on}}
-// expected-note@-27 {{if you supply your o

[clang] 454d1df - [Concepts] Fix overload resolution bug with constrained candidates

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2022-04-19T04:45:28-04:00
New Revision: 454d1df9423c95e54c3a2f5cb58d864096032d09

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

LOG: [Concepts] Fix overload resolution bug with constrained candidates

When doing overload resolution, we have to check that candidates' parameter 
types are equal before trying to find a better candidate through checking which 
candidate is more constrained.
This revision adds this missing check and makes us diagnose those cases as 
ambiguous calls when the types are not equal.

Fixes GitHub issue https://github.com/llvm/llvm-project/issues/53640

Reviewed By: erichkeane

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

Added: 
clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c396322f2de57..a7a6fab5fe535 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -123,6 +123,11 @@ Bug Fixes
   a lambda expression that shares the name of a variable in a containing
   if/while/for/switch init statement as a redeclaration.
   This fixes `Issue 54913 
`_.
+- Overload resolution for constrained function templates could use the partial
+  order of constraints to select an overload, even if the parameter types of
+  the functions were 
diff erent. It now diagnoses this case correctly as an
+  ambiguous call and an error. Fixes
+  `Issue 53640 `_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e2ae02ea9f76f..fd9433cb1b786 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3590,7 +3590,8 @@ class Sema final {
 QualType& ConvertedType);
   bool FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
   const FunctionProtoType *NewType,
-  unsigned *ArgPos = nullptr);
+  unsigned *ArgPos = nullptr,
+  bool Reversed = false);
   void HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
   QualType FromType, QualType ToType);
 
@@ -8768,7 +8769,8 @@ class Sema final {
   FunctionTemplateDecl *getMoreSpecializedTemplate(
   FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
   TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-  unsigned NumCallArguments2, bool Reversed = false);
+  unsigned NumCallArguments2, bool Reversed = false,
+  bool AllowOrderingByConstraints = true);
   UnresolvedSetIterator
   getMostSpecialized(UnresolvedSetIterator SBegin, UnresolvedSetIterator SEnd,
  TemplateSpecCandidateSet &FailedCandidates,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 1add971a81a02..6e45fdfbeb089 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2945,24 +2945,30 @@ void Sema::HandleFunctionTypeMismatch(PartialDiagnostic 
&PDiag,
 }
 
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
-/// for equality of their argument types. Caller has already checked that
-/// they have same number of arguments.  If the parameters are 
diff erent,
+/// for equality of their parameter types. Caller has already checked that
+/// they have same number of parameters.  If the parameters are 
diff erent,
 /// ArgPos will have the parameter index of the first 
diff erent parameter.
+/// If `Reversed` is true, the parameters of `NewType` will be compared in
+/// reverse order. That's useful if one of the functions is being used as a 
C++20
+/// synthesized operator overload with a reversed parameter order.
 bool Sema::FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
   const FunctionProtoType *NewType,
-  unsigned *ArgPos) {
-  for (FunctionProtoType::param_type_iterator O = OldType->param_type_begin(),
-  N = NewType->param_type_begin(),
-  E = OldType->param_type_end();
-   O && (O != E); ++O, ++N) {
+  unsigned *ArgPos, bool Reversed) {
+  assert(OldType->getNumParams() == NewType->getNumParams() &&
+ "Can't compare parameters of

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Roy Jacobson 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 rG454d1df9423c: [Concepts] Fix overload resolution bug with 
constrained candidates (authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp

Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
+struct A;
+struct B;
+
+template  constexpr bool True = true;
+template  concept C = True;
+
+void f(C auto &, auto &) = delete;
+template  void f(Q &, C auto &);
+
+void g(struct A *ap, struct B *bp) {
+  f(*ap, *bp);
+}
+
+template  struct X {};
+
+template  bool operator==(X, V) = delete;
+templatebool operator==(T, X);
+
+bool h() {
+  return X{} == 0;
+}
+
+namespace PR53640 {
+
+template 
+concept C = true;
+
+template 
+void f(T t) {} // expected-note {{candidate function [with T = int]}}
+
+template 
+void f(const T &t) {} // expected-note {{candidate function [with T = int]}}
+
+int g() {
+  f(0); // expected-error {{call to 'f' is ambiguous}}
+}
+
+struct S {
+  template  explicit S(T) noexcept requires C {} // expected-note {{candidate constructor}}
+  template  explicit S(const T &) noexcept {}   // expected-note {{candidate constructor}}
+};
+
+int h() {
+  S s(4); // expected-error-re {{call to constructor of {{.*}} is ambiguous}}
+}
+
+}
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5143,18 +5143,20 @@
 /// candidate with a reversed parameter order. In this case, the corresponding
 /// P/A pairs between FT1 and FT2 are reversed.
 ///
+/// \param AllowOrderingByConstraints If \c is false, don't check whether one
+/// of the templates is more constrained than the other. Default is true.
+///
 /// \returns the more specialized function template. If neither
 /// template is more specialized, returns NULL.
-FunctionTemplateDecl *
-Sema::getMoreSpecializedTemplate(FunctionTemplateDecl *FT1,
- FunctionTemplateDecl *FT2,
- SourceLocation Loc,
- TemplatePartialOrderingContext TPOC,
- unsigned NumCallArguments1,
- unsigned NumCallArguments2,
- bool Reversed) {
-
-  auto JudgeByConstraints = [&] () -> FunctionTemplateDecl * {
+FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
+FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
+TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
+unsigned NumCallArguments2, bool Reversed,
+bool AllowOrderingByConstraints) {
+
+  auto JudgeByConstraints = [&]() -> FunctionTemplateDecl * {
+if (!AllowOrderingByConstraints)
+  return nullptr;
 llvm::SmallVector AC1, AC2;
 FT1->getAssociatedConstraints(AC1);
 FT2->getAssociatedConstraints(AC2);
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -2945,24 +2945,30 @@
 }
 
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
-/// for equality of their argument types. Caller has already checked that
-/// they have same number of arguments.  If the parameters are different,
+/// for equality of their parameter types. Caller has already checked that
+/// they have same number of parameters.  If the parameters are different,
 /// ArgPos will have the parameter index of the first different parameter.
+/// If `Reversed` is true, the parameters of `NewType` will be compared in
+/// reverse order. That's useful if one of the functions is being used as a C++20
+/// synthesized operator overload with a reversed parameter order.
 bool Sema::FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
   const FunctionProtoType *NewType,
-  unsigned *ArgPos) {
-  for (FunctionProtoType::param_type_iterator O = OldType->param_type_begin(),
-  N = NewType->param_type_begin(),
-  E = OldType->param_type_end();
-   O && (O != E); ++O, ++N) {
+  unsigned *A

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D112921#3453114 , @MaskRay wrote:

> Beside the concern raised by platform maintainers: the cc1 default switch 
> part should be made separately from the patch.
> This makes revert easy and leaves fewer churn to the test suite.
>
> If -fno-sized-deallocation is a better cc1 default (but not Driver's), I can 
> make such a change.

That will be helpful, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D122008#3456445 , @clementval 
wrote:

> Do you plan to discuss this again during the next call? Note that today is a 
> holiday in various country in Europe (maybe elsewhere too) so the one on 4/27 
> is probably better.

To be perfectly honest, I wasn't planning to.

The main reason to bring this up in one of our community calls was to increase 
the visibility of this patch. I feel that that goal has been achieved. Keeping 
the discussion here means that even people who are unable to join our calls can 
participate and share their view. As such, discussing on Phabricator is more 
inclusive.

While I appreciate that some people expressed preference for Option 3, it 
seemed to me that Option 2 is a compromise that will work for everyone. It 
hides the new functionality behind a flag and makes it very counter-intuitive 
to use. The flag itself makes it clear that this functionality is experimental. 
At the same time, Option 2 is sufficient to unblock progress in other areas.

If folks disagree and feel that Option 2 is still problematic, could you 
elaborate "why" and suggest an alternative? There's no need to wait for the 
next call to discuss this.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D123488: [clangd] IncludeCleaner: Add filtering mechanism

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Not to fix in this patch, but the move constructor of llvm::Regex is suspicious 
- there's no move assignment operator.
This putting them in a vector seems likely to copy them when the vector 
reallocates. Think you could fix this? :-)




Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:527
+diag(Warning,
+ llvm::formatv("Invalid regular expression: {0}", *HeaderPattern)
+ .str(),

missing the error message



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:242
+const Inclusion &Inc, ParsedAST &AST,
+llvm::ArrayRef> HeaderFilters) {
   if (Inc.BehindPragmaKeep)

maybe just pass in config?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123488

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

[PATCH] D123488: [clangd] IncludeCleaner: Add filtering mechanism

[Sam McCall via Phabricator via cfe-commits]

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

Oops, LG with comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123488

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

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/include/clang/Basic/Module.h:537-543
+  static StringRef getPrimaryModuleInterfaceName(StringRef Name) {
+return Name.split(':').first;
+  }
+
   /// Get the primary module interface name from a partition.
   StringRef getPrimaryModuleInterfaceName() const {
+return getPrimaryModuleInterfaceName(getTopLevelModuleName());

ChuanqiXu wrote:
> iains wrote:
> > I do not understand the purpose of this change, we already iterated over 
> > making the function more efficient.
> > 
> > For C++20 modules there cannot be more than one parent - so that the nested 
> > call to getTopLevelModule(), via getTopLevelModuleName() is redundant.
> > 
> > Is there some other case that needs special handling?
> > (If so then I think we should guard this at a higher level)
> > 
> The primary purpose of the change is that we need to get the primary module 
> name for getLangOpts().CurrentModule. So we need a static member function 
> which takes a StringRef.
> 
> Another point might be the correctness. For example, for the private module 
> fragment, the current implementation would return the right result while the 
> original wouldn't. (Although we didn't use private module fragment much...)
> 
> For the most case, I admit the current implementation would call more 
> function a little more. But I think it is negligible.
(Maybe I am worrying too much - but it does seem that we have quite some 
complexity in an operation that we expect to repeat many times per TU).



> The primary purpose of the change is that we need to get the primary module 
> name for getLangOpts().CurrentModule. So we need a static member function 
> which takes a StringRef.

That is, of course,  fine (and we are stuck in the case that we are building a 
partition, with the fact that the primary module is not available).  It seems 
that there's not a lot we can do there. 

When we have module,  we could cache the result of the split, tho - so have a 
"PrimaryModuleName" entry in each module and populate it on the first use.  We 
are still stuck with the string comparison - but at least we could avoid 
repeating the split?
As I noted, the reason we did not bother to do this in the first use was that 
that use was only once per TU).

When we are building a consumer of a module, on the other hand, we *do* have 
the complete module import graph.

> Another point might be the correctness. For example, for the private module 
> fragment, the current implementation would return the right result while the 
> original wouldn't. (Although we didn't use private module fragment much...)

Of course, we must fix correctness issues ..
Since there is only GMF and PMF, perhaps we could reasonably treat those 
specially `if (isGlobalModule())` .. and/or `if (isPrivateModule())`?

> For the most case, I admit the current implementation would call more 
> function a little more. But I think it is negligible.

It is hard to judge at the moment - perhaps we should carry on with what we 
have and then instrument some reasonable body of code?



Comment at: clang/lib/Sema/SemaLookup.cpp:1567-1568
+
+  return M->getPrimaryModuleInterfaceName() ==
+ Module::getPrimaryModuleInterfaceName(LangOpts.CurrentModule);
 }

ChuanqiXu wrote:
> iains wrote:
> > ... of course, "correctness before optimisation", but this test function 
> > seems like it would benefit from some refactoring as suggested below.
> > 
> > it seems a bit unfortunate that we are placing a string comparison in the 
> > "acceptable decl" lookup path, which might be made many times (the previous 
> > use of the string compare was in making the module decl - which only 
> > happens once in a TU).
> > 
> > * Sema has visibility of the import graph of modules (via 
> > DirectModuleImports and Exports).
> > * We also know what the module parse state is (FirstDecl, GMF etc)
> > * If were are not parsing a module (i.e. LangOpts.CurrentModule.empty()) 
> > the we could return false early?
> > * if ModuleScopes is empty we know we are not (yet) parsing a module
> > * Once we are parsing a module then we can test ModuleScopes.back() to find 
> > the current module 
> > 
> > there are only two users of isInCurrentModule() - so maybe we refactor 
> > could make more use of the higher level state information - and / or cache 
> > information on the parents of partitions  to  avoid the complexity of 
> > parsing strings each time.
> > 
> > what do you think?
> Yeah, string comparison is relatively expensive and we should avoid that. I 
> added a search cache here and I thought it should handle the most cases.
> 
> For the solution you described, I am not sure if I understood correctly. It 
> looks like a quick return path if we found we are not parsing a module? If 
> yes, I think the current check could do similar works. 
That looks good.

On the actual test itself:

1/
we have the check of  `(M->isG

[PATCH] D123957: Update the developer policy to mention release notes

[James Henderson via Phabricator via cfe-commits]

jhenderson accepted this revision.
jhenderson added a comment.

LGTM, with suggested nits.




Comment at: llvm/docs/DeveloperPolicy.rst:188
+notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to
+a project that are user-facing or users may wish to know about should be added
+to the project's release notes. Examples of changes that would typically





Comment at: llvm/docs/DeveloperPolicy.rst:192
+
+* Adding, removing, or modifying command line options.
+* Adding or removing a diagnostic.

I think official coding documentation style guides say to use "command-line" - 
at least, that's what our internal docs team has told me in the past.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123957

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

[clang] f3ee0af - [OpenCL] opencl-c.h: Add const to get_image_num_samples

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2022-04-19T10:16:44+01:00
New Revision: f3ee0afc6739bf2990f9d302ff6b28dbb0429e8d

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

LOG: [OpenCL] opencl-c.h: Add const to get_image_num_samples

Align with the `-fdeclare-opencl-builtins` option and other
get_image_* builtins which have the const attribute.

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

Added: 


Modified: 
clang/lib/Headers/opencl-c.h

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index cee3c680aff2e..7ea835ae5c2de 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -16118,21 +16118,21 @@ size_t __ovld __cnfn get_image_array_size(read_write 
image2d_array_msaa_depth_t)
 * Return the number of samples associated with image
 */
 #if defined(cl_khr_gl_msaa_sharing)
-int __ovld get_image_num_samples(read_only image2d_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_depth_t);
 
-int __ovld get_image_num_samples(write_only image2d_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_depth_t);
 
 #if defined(__opencl_c_read_write_images)
-int __ovld get_image_num_samples(read_write image2d_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_depth_t);
 #endif //defined(__opencl_c_read_write_images)
 #endif
 



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

[PATCH] D122728: [OpenCL] opencl-c.h: Add const to get_image_num_samples

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3ee0afc6739: [OpenCL] opencl-c.h: Add const to 
get_image_num_samples (authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122728

Files:
  clang/lib/Headers/opencl-c.h


Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -16118,21 +16118,21 @@
 * Return the number of samples associated with image
 */
 #if defined(cl_khr_gl_msaa_sharing)
-int __ovld get_image_num_samples(read_only image2d_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_depth_t);
 
-int __ovld get_image_num_samples(write_only image2d_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_depth_t);
 
 #if defined(__opencl_c_read_write_images)
-int __ovld get_image_num_samples(read_write image2d_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_depth_t);
 #endif //defined(__opencl_c_read_write_images)
 #endif
 


Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -16118,21 +16118,21 @@
 * Return the number of samples associated with image
 */
 #if defined(cl_khr_gl_msaa_sharing)
-int __ovld get_image_num_samples(read_only image2d_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_only image2d_array_msaa_depth_t);
 
-int __ovld get_image_num_samples(write_only image2d_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_msaa_depth_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_t);
-int __ovld get_image_num_samples(write_only image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(write_only image2d_array_msaa_depth_t);
 
 #if defined(__opencl_c_read_write_images)
-int __ovld get_image_num_samples(read_write image2d_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_msaa_depth_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_t);
-int __ovld get_image_num_samples(read_write image2d_array_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_msaa_depth_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_t);
+int __ovld __cnfn get_image_num_samples(read_write image2d_array_msaa_depth_t);
 #endif //defined(__opencl_c_read_write_images)
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, njames93, PaulkaToast.
whisperity added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, martong, gamesh411, Szelethus, dkrupp, 
rnkovacs, xazax.hun.
Herald added a project: All.
whisperity requested review of this revision.
Herald added a subscriber: cfe-commits.

The routine that facilitated symbols to be explicitly allowed asked the name of 
the called function, which resulted in a crash when the check was accidentally 
run on non-trivial C++ code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123992

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -5,6 +5,10 @@
 void nested_func() {}
 } // namespace nested
 void libc_api_func() {}
+
+struct libc_api_struct {
+  int operator()() const { return 0; }
+};
 } // namespace __llvm_libc
 
 // Emulate a function from the public headers like string.h
@@ -13,6 +17,11 @@
 // Emulate a function specifically allowed by the exception list.
 void malloc() {}
 
+// Emulate a non-trivially named symbol.
+struct global_struct {
+  int operator()() const { return 0; }
+};
+
 namespace __llvm_libc {
 void Test() {
   // Allow calls with the fully qualified name.
@@ -30,6 +39,9 @@
   void (*barePtr)(void) = __llvm_libc::libc_api_func;
   barePtr();
 
+  // Allow calling entities defined in the namespace.
+  __llvm_libc::libc_api_struct{}();
+
   // Disallow calling into global namespace for implemented entrypoints.
   ::libc_api_func();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
@@ -43,6 +55,12 @@
 
   // Allow calling into global namespace for specific functions.
   ::malloc();
+
+  // Disallow calling on entities that are not in the namespace, but make sure
+  // no crashes happen.
+  global_struct{}();
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'operator()' must resolve to a function declared within the '__llvm_libc' namespace
+  // CHECK-MESSAGES: :22:7: note: resolves to this declaration
 }
 
 } // namespace __llvm_libc
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,31 +136,39 @@
 Changes in existing checks
 ^^
 
-- Improved :doc:`performance-inefficient-vector-operation 
-  ` to work when
-  the vector is a member of a structure.
-
-- Fixed a false positive in :doc:`readability-non-const-parameter
-  ` when the parameter is referenced by an lvalue.
-
-- Fixed a crash in :doc:`readability-const-return-type
-  ` when a pure virtual function
-  overrided has a const return type. Removed the fix for a virtual function.
+- Fixed a crash in :doc:`bugprone-sizeof-expression
+  ` when `sizeof(...)` is
+  compared against a `__int128_t`.
 
-- Fixed a false positive in :doc:`misc-redundant-expression `
-  involving overloaded comparison operators.
-
-- Fixed a crash in :doc:`bugprone-sizeof-expression ` when
-  `sizeof(...)` is compared agains a `__int128_t`.
-  
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
-- Fixed a false positive in :doc:`misc-redundant-expression `
-  involving assignments in conditions. This fixes `Issue 35853 `_.
+- Fixed a crash in :doc:`llvmlibc-callee-namespace
+  ` when executing for C++ code
+  that contain calls to advanced constructs, e.g. overloaded operators.
+
+- Fixed a false positive in :doc:`misc-redundant-expression
+  ` involving overloaded
+  comparison operators.
+
+- Fixed a false positive in :doc:`misc-redundant-expression
+  ` involving assignments in
+  conditions. This fixes `Issue 35853 `_.
+
+- Fixed a crash in :doc:`readability-const-return-type
+  ` when a pure virtual function
+  overrided has a const return type. Removed the fix for a virtual function.
+
+- Fixed a false positive in :doc:`readability-non-const-parameter
+  ` when the parameter is
+  referenced by an lvalue.
+
+- Improved :doc:`performance-inefficient-vector-operation
+  ` to work when
+  the vector is a member of a structure.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:204-207
   // Compare by number of times the location was visited first (negated
   // to prefer less often visited locations), then by insertion time (prefer
   // expanding nodes inserted sooner first).
+  using QueuePriority = std::pair;

Please leave a note there, of what the negative values are used for.



Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:216-218
   // Number of inserted nodes, used to emulate DFS ordering in the priority
   // queue when insertions are equal.
+  long Counter = 0;

It seems like you are not exploiting the signedness of this variable. And a 
counter should never be negative anyway.



Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:255
+class CTUWorkList : public UnexploredFirstPriorityQueue {
+  enum class SecondaryOrder { DFS = 1, ReverseDFS };
+  SecondaryOrder ScndOrd = SecondaryOrder::ReverseDFS;

Raw enum would make it more convenient, and the scope is already private - thus 
it won't affect readability elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123784

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

[PATCH] D114292: [clang-tidy] Fix `altera-struct-pack-align` check for empty structs

[Whisperity via Phabricator via cfe-commits]

whisperity accepted this revision.
whisperity added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

@aaron.ballman Alright, I think this can go. The `ReleaseNotes.rst` needs a 
rebase anyway.


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

https://reviews.llvm.org/D114292

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

[PATCH] D123464: [analyzer] Clean checker options from bool to DefaultBool (NFC)

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

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

> A recent review emphasized the preference to use DefaultBool instead of bool 
> for checker options.

What I wanted to highlight is that we should aim for consistency. We should 
either use that everywhere or nowhere.

To be fair, we should probably use raw bools instead.
They could not only default to `false`, but to `true` as well.

In addition to this, we should prefer the c++ way of initializing these at the 
declaration directly to either of those values.
IMO we don't need this `DefaultBool` abstraction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123464

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

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 423572.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D123837

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CXX/module/module.import/p2.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp

Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -56,7 +56,14 @@
 int &c = n; // expected-error {{use of undeclared identifier}}
 
 //--- std10-1-ex2-tu7.cpp
+// expected-no-diagnostics
 module B:X3; // does not implicitly import B
-import :X2;  // X2 is an implementation so exports nothing.
- // error: n not visible here.
-int &c = n;  // expected-error {{use of undeclared identifier }}
+import : X2; // X2 is an implementation unit import B.
+// According to [module.import]p7:
+//   Additionally, when a module-import-declaration in a module unit of some
+//   module M imports another module unit U of M, it also imports all
+//   translation units imported by non-exported module-import-declarations in
+//   the module unit purview of U.
+//
+// So B is imported in B:X3 due to B:X2 imported B. So n is visible here.
+int &c = n;
Index: clang/test/CXX/module/module.import/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.import/p2.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c++20 %t/impl.cppm -emit-module-interface -o %t/M-impl.pcm
+// RUN: %clang_cc1 -std=c++20 %t/M.cppm -emit-module-interface -fprebuilt-module-path=%t -o %t/M.pcm
+// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/UseInPartA.cppm -fprebuilt-module-path=%t -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/UseInAnotherModule.cppm -fprebuilt-module-path=%t -verify -fsyntax-only
+
+//--- impl.cppm
+module M : impl;
+class A {};
+
+//--- M.cppm
+export module M;
+import : impl;
+export A f();
+
+//--- Use.cpp
+import M;
+void test() {
+  A a; // expected-error {{unknown type name 'A'}}
+}
+
+//--- UseInPartA.cppm
+// expected-no-diagnostics
+export module M : partA;
+import : impl;
+void test() {
+  A a;
+}
+
+//--- UseInAnotherModule.cppm
+export module B;
+import M;
+void test() {
+  A a; // expected-error {{unknown type name 'A'}}
+}
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1556,17 +1556,32 @@
   return LookupModulesCache;
 }
 
-/// Determine whether the module M is part of the current module from the
-/// perspective of a module-private visibility check.
-static bool isInCurrentModule(const Module *M, const LangOptions &LangOpts) {
+/// Determine if we could use all the declarations in the module.
+bool Sema::isUsableModule(const Module *M) {
+  assert(M && "We shouldn't check nullness for module here");
+  // Return quickly if we cached the result.
+  if (CurrentModuleUnitsCache.count(M))
+return true;
+
   // If M is the global module fragment of a module that we've not yet finished
-  // parsing, then it must be part of the current module.
-  // If it's a partition, then it must be visible to an importer (since only
-  // another partition or the named module can import it).
-  return M->getTopLevelModuleName() == LangOpts.CurrentModule ||
- (M->Kind == Module::GlobalModuleFragment && !M->Parent) ||
- M->Kind == Module::ModulePartitionInterface ||
- M->Kind == Module::ModulePartitionImplementation;
+  // parsing, then M should be the global module fragment in current TU. So it
+  // should be usable.
+  // [module.global.frag]p1:
+  //   The global module fragment can be used to provide declarations that are
+  //   attached to the global module and usable within the module unit.
+  if ((M->isGlobalModule() && !M->Parent) ||
+  // The module unit which is in the same module with the current module
+  // unit is usable.
+  //
+  // FIXME: Here we judge if they are in the same module by comparing the
+  // string. Is there any better solution?
+  (M->getPrimaryModuleInterfaceName() ==
+   llvm::StringRef(getLangOpts().CurrentModule).split(':').first)) {
+CurrentModuleUnitsCache.insert(M);
+return true;
+  }
+
+  return false;
 }
 
 bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
@@ -1578,7 +1593,7 @@
 
 bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) {
   for (const Module *Merged : Context.getModulesWithMergedDefinition(Def))
-if (isInCurrentModule(Merged, getLangOpts()))
+if (isUsableModule(Merged))
   return true;
   return false;
 }
@@ -1760,7 +177

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

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

steakhal added a subscriber: whisperity.
steakhal added a comment.

In D123352#3439649 , @MosheBerman 
wrote:

> In D123352#3439390 , @steakhal 
> wrote:
>
>> tldr; static-analyzer fixits are not completely implemented.
>
> Where can I learn more about this?

Grep through the code and look for references to the variable. It's not used 
widely.
In fact, there are no checkers facilitating fixits.

> Would it be possible and idiomatically/architecturally sounds to write a 
> clang-tidy that processes output from this checker?

I don't believe that `clang-tidy` should be involved in this. It would be 
better to keep it in-house (in the analyzer).

>> When I passed the `apply-fixits`, it modified the input source file - as I 
>> expected.
>
> Did you test this diff, or an existing checker? Would you please share the 
> command you used to test?

Sort of. I tried to apply some parts of it by hand, and check the output 
difference in my command.

I cannot immediately recall, but I cannot remember any major obstacles, which 
suggests that I had to set the `apply-fixits=true` analyzer config option and 
that's it.
Since there were no checkers emitting fixit hints, I modified an existing one 
to emit a dummy fixithint. Nothing fancy there.

>> Then I tried the `-analyzer-output=text` and suddenly it inserted the fixit 
>> 2 times xD, which is less than ideal and we should fix this.
>> And I'm expecting many more bugs with this feature.
>
> This is why it's gated. xD.

What do you mean by 'gated'? It seems to be a bug, which we should fix prior to 
this.

PS: I'm also inviting @whisperity since he is more experienced with fixits and 
that sort of stuff.




Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:162-164
+  /// 2. If we are annotating an Objective-C method, and not a function, we
+  ///want to use the `nullable` form instead of `_Nullable`.
+  ///When \p SyntaxSugar is true, we handle the second case.

You could pass the owning Decl to this function and directly figure out if it's 
an objc method or not.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:675
 OS << " returned from a " << C.getDeclDescription(D) <<
-  " that is expected to return a non-null value";
+  " that is expected to return a non-null value foo";
+

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123352

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

[clang] b7c9888 - [analyzer][NFC] Introduce the checker package separator character

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-04-19T12:14:27+02:00
New Revision: b7c988811d50f0f068a375292fb5f2b6df384400

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

LOG: [analyzer][NFC] Introduce the checker package separator character

Reviewed By: martong, ASDenysPetrov

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

Added: 


Modified: 
clang/utils/TableGen/ClangSACheckersEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangSACheckersEmitter.cpp 
b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
index 00d88274fc385..586cc95071cf5 100644
--- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -24,28 +24,29 @@ using namespace llvm;
 // Static Analyzer Checkers Tables generation
 
//===--===//
 
-static std::string getPackageFullName(const Record *R);
+static std::string getPackageFullName(const Record *R, StringRef Sep = ".");
 
-static std::string getParentPackageFullName(const Record *R) {
+static std::string getParentPackageFullName(const Record *R,
+StringRef Sep = ".") {
   std::string name;
   if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage")))
-name = getPackageFullName(DI->getDef());
+name = getPackageFullName(DI->getDef(), Sep);
   return name;
 }
 
-static std::string getPackageFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getPackageFullName(const Record *R, StringRef Sep) {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("PackageName").empty());
   name += R->getValueAsString("PackageName");
   return name;
 }
 
-static std::string getCheckerFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("CheckerName").empty());
   name += R->getValueAsString("CheckerName");
   return name;



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

[clang] 63c4ca9 - [analyzer] Turn missing tablegen doc entry of a checker into fatal error

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-04-19T12:14:27+02:00
New Revision: 63c4ca9d14bad909f92ef019ec85f7a1c42311f2

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

LOG: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

It turns out all checkers explicitly mention the `Documentation<>`.
It makes sense to demand this, so emit a fatal tablegen error if such
happens.

Reviewed By: martong, Szelethus

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

Added: 


Modified: 
clang/utils/TableGen/ClangSACheckersEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangSACheckersEmitter.cpp 
b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
index 586cc95071cf5..61a47ee61fac4 100644
--- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -76,14 +76,17 @@ static inline uint64_t getValueFromBitsInit(const BitsInit 
*B, const Record &R)
 
 static std::string getCheckerDocs(const Record &R) {
   StringRef LandingPage;
-  if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) {
-uint64_t V = getValueFromBitsInit(BI, R);
-if (V == 1)
-  LandingPage = "available_checks.html";
-else if (V == 2)
-  LandingPage = "alpha_checks.html";
-  }
-  
+  const BitsInit *BI = R.getValueAsBitsInit("Documentation");
+  if (!BI)
+PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
+getCheckerFullName(&R));
+
+  uint64_t V = getValueFromBitsInit(BI, R);
+  if (V == 1)
+LandingPage = "available_checks.html";
+  else if (V == 2)
+LandingPage = "alpha_checks.html";
+
   if (LandingPage.empty())
 return "";
 



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

[clang] 744e2a3 - [analyzer] ClangSA should tablegen doc urls refering to the main doc page

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-04-19T12:14:27+02:00
New Revision: 744e2a3e2232db87cb68b201739956ca205dc946

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

LOG: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

AFAIK we should prefer
https://clang.llvm.org/docs/analyzer/checkers.html to
https://clang-analyzer.llvm.org/{available_checks,alpha_checks}.html

This patch will ensure that the doc urls produced by tablegen for the
ClangSA, will use the new url. Nothing else will be changed.

Reviewed By: martong, Szelethus, ASDenysPetrov

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

Added: 


Modified: 

clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
clang/utils/TableGen/ClangSACheckersEmitter.cpp

Removed: 




diff  --git 
a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
 
b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
index 0f47941849500..9005a3654ad0e 100644
--- 
a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -451,7 +451,7 @@
   "fullDescription": {
 "text": "Check for logical errors for function calls and 
Objective-C message expressions (e.g., uninitialized arguments, null function 
pointers)"
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#core.CallAndMessage";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#core-callandmessage";,
   "id": "core.CallAndMessage",
   "name": "core.CallAndMessage"
 },
@@ -459,7 +459,7 @@
   "fullDescription": {
 "text": "Check for division by zero"
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#core.DivideZero";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#core-dividezero";,
   "id": "core.DivideZero",
   "name": "core.DivideZero"
 },
@@ -467,7 +467,7 @@
   "fullDescription": {
 "text": "Check for memory leaks, double free, and 
use-after-free problems. Traces memory managed by malloc()/free()."
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#unix-malloc";,
   "id": "unix.Malloc",
   "name": "unix.Malloc"
 }

diff  --git a/clang/utils/TableGen/ClangSACheckersEmitter.cpp 
b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
index 61a47ee61fac4..22bec37bc1fa5 100644
--- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -75,23 +75,18 @@ static inline uint64_t getValueFromBitsInit(const BitsInit 
*B, const Record &R)
 }
 
 static std::string getCheckerDocs(const Record &R) {
-  StringRef LandingPage;
   const BitsInit *BI = R.getValueAsBitsInit("Documentation");
   if (!BI)
 PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
 getCheckerFullName(&R));
 
-  uint64_t V = getValueFromBitsInit(BI, R);
-  if (V == 1)
-LandingPage = "available_checks.html";
-  else if (V == 2)
-LandingPage = "alpha_checks.html";
-
-  if (LandingPage.empty())
+  // Ignore 'Documentation' checkers.
+  if (getValueFromBitsInit(BI, R) == 0)
 return "";
 
-  return (llvm::Twine("https://clang-analyzer.llvm.org/";) + LandingPage + "#" +
-  getCheckerFullName(&R))
+  std::string CheckerFullName = StringRef(getCheckerFullName(&R, "-")).lower();
+  return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#";) +
+  CheckerFullName)
   .str();
 }
 



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

[clang] 7984189 - [analyzer] Remove HasAlphaDocumentation tablegen enum value

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-04-19T12:14:27+02:00
New Revision: 798418982630b5e3acdfc7ba398993aef2071f32

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

LOG: [analyzer] Remove HasAlphaDocumentation tablegen enum value

D121387 simplified the doc url generation process, so we no longer need
the HasAlphaDocumentation enum entry. This patch removes that.

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td 
b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
index 98d26aaa637d1..bc1da9bb3f904 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
@@ -86,15 +86,14 @@ class ParentPackage { Package ParentPackage = P; 
}
 class HelpText { string HelpText = text; }
 
 /// Describes what kind of documentation exists for the checker.
-class DocumentationEnum val> {
-  bits<2> Documentation = val;
+class DocumentationEnum val> {
+  bits<1> Documentation = val;
 }
 def NotDocumented : DocumentationEnum<0>;
 def HasDocumentation : DocumentationEnum<1>;
-def HasAlphaDocumentation : DocumentationEnum<2>;
 
 class Documentation {
-  bits<2> Documentation = val.Documentation;
+  bits<1> Documentation = val.Documentation;
 }
 
 /// Describes a checker. Every builtin checker has to be registered with the 
use
@@ -114,7 +113,7 @@ class Checker {
   list   Dependencies;
   // This field is optional.
   list   WeakDependencies;
-  bits<2> Documentation;
+  bits<1> Documentation;
   Package ParentPackage;
   bit Hidden = 0;
 }

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9340a114ac456..b5ce1eda200c7 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -236,57 +236,57 @@ let ParentPackage = CoreAlpha in {
 
 def BoolAssignmentChecker : Checker<"BoolAssignment">,
   HelpText<"Warn about assigning non-{0,1} values to Boolean variables">,
-  Documentation;
+  Documentation;
 
 def CastSizeChecker : Checker<"CastSize">,
   HelpText<"Check when casting a malloc'ed type T, whether the size is a "
"multiple of the size of T">,
-  Documentation;
+  Documentation;
 
 def CastToStructChecker : Checker<"CastToStruct">,
   HelpText<"Check for cast from non-struct pointer to struct pointer">,
-  Documentation;
+  Documentation;
 
 def ConversionChecker : Checker<"Conversion">,
   HelpText<"Loss of sign/precision in implicit conversions">,
-  Documentation;
+  Documentation;
 
 def IdenticalExprChecker : Checker<"IdenticalExpr">,
   HelpText<"Warn about unintended use of identical expressions in operators">,
-  Documentation;
+  Documentation;
 
 def FixedAddressChecker : Checker<"FixedAddr">,
   HelpText<"Check for assignment of a fixed address to a pointer">,
-  Documentation;
+  Documentation;
 
 def PointerArithChecker : Checker<"PointerArithm">,
   HelpText<"Check for pointer arithmetic on locations other than array "
"elements">,
-  Documentation;
+  Documentation;
 
 def PointerSubChecker : Checker<"PointerSub">,
   HelpText<"Check for pointer subtractions on two pointers pointing to "
"
diff erent memory chunks">,
-  Documentation;
+  Documentation;
 
 def SizeofPointerChecker : Checker<"SizeofPtr">,
   HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
-  Documentation;
+  Documentation;
 
 def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
   HelpText<"Check for division by variable that is later compared against 0. "
"Either the comparison is useless or there is division by zero.">,
-  Documentation;
+  Documentation;
 
 def DynamicTypeChecker : Checker<"DynamicTypeChecker">,
   HelpText<"Check for cases where the dynamic and the static type of an object 
"
"are unrelated.">,
-  Documentation;
+  Documentation;
 
 def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
   HelpText<"Check that addresses to stack memory do not escape the function">,
   Dependencies<[StackAddrEscapeBase]>,
-  Documentation;
+  Documentation;
 
 def PthreadLockBase : Checker<"PthreadLockBase">,
   HelpText<"Helper registering multiple checks.">,
@@ -296,7 +296,7 @@ def PthreadLockBase : Checker<"PthreadLockBase">,
 def C11LockChecker : Checker<"C11Lock">,
   HelpText<"Simple lock -> unlock checker">,
   Dependencies<[PthreadLockBase]>,
-  Documentat

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG63c4ca9d14ba: [analyzer] Turn missing tablegen doc entry of 
a checker into fatal error (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122244

Files:
  clang/utils/TableGen/ClangSACheckersEmitter.cpp


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -76,14 +76,17 @@
 
 static std::string getCheckerDocs(const Record &R) {
   StringRef LandingPage;
-  if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) {
-uint64_t V = getValueFromBitsInit(BI, R);
-if (V == 1)
-  LandingPage = "available_checks.html";
-else if (V == 2)
-  LandingPage = "alpha_checks.html";
-  }
-  
+  const BitsInit *BI = R.getValueAsBitsInit("Documentation");
+  if (!BI)
+PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
+getCheckerFullName(&R));
+
+  uint64_t V = getValueFromBitsInit(BI, R);
+  if (V == 1)
+LandingPage = "available_checks.html";
+  else if (V == 2)
+LandingPage = "alpha_checks.html";
+
   if (LandingPage.empty())
 return "";
 


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -76,14 +76,17 @@
 
 static std::string getCheckerDocs(const Record &R) {
   StringRef LandingPage;
-  if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) {
-uint64_t V = getValueFromBitsInit(BI, R);
-if (V == 1)
-  LandingPage = "available_checks.html";
-else if (V == 2)
-  LandingPage = "alpha_checks.html";
-  }
-  
+  const BitsInit *BI = R.getValueAsBitsInit("Documentation");
+  if (!BI)
+PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
+getCheckerFullName(&R));
+
+  uint64_t V = getValueFromBitsInit(BI, R);
+  if (V == 1)
+LandingPage = "available_checks.html";
+  else if (V == 2)
+LandingPage = "alpha_checks.html";
+
   if (LandingPage.empty())
 return "";
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7c988811d50: [analyzer][NFC] Introduce the checker package 
separator character (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122243

Files:
  clang/utils/TableGen/ClangSACheckersEmitter.cpp


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -24,28 +24,29 @@
 // Static Analyzer Checkers Tables generation
 
//===--===//
 
-static std::string getPackageFullName(const Record *R);
+static std::string getPackageFullName(const Record *R, StringRef Sep = ".");
 
-static std::string getParentPackageFullName(const Record *R) {
+static std::string getParentPackageFullName(const Record *R,
+StringRef Sep = ".") {
   std::string name;
   if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage")))
-name = getPackageFullName(DI->getDef());
+name = getPackageFullName(DI->getDef(), Sep);
   return name;
 }
 
-static std::string getPackageFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getPackageFullName(const Record *R, StringRef Sep) {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("PackageName").empty());
   name += R->getValueAsString("PackageName");
   return name;
 }
 
-static std::string getCheckerFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("CheckerName").empty());
   name += R->getValueAsString("CheckerName");
   return name;


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -24,28 +24,29 @@
 // Static Analyzer Checkers Tables generation
 //===--===//
 
-static std::string getPackageFullName(const Record *R);
+static std::string getPackageFullName(const Record *R, StringRef Sep = ".");
 
-static std::string getParentPackageFullName(const Record *R) {
+static std::string getParentPackageFullName(const Record *R,
+StringRef Sep = ".") {
   std::string name;
   if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage")))
-name = getPackageFullName(DI->getDef());
+name = getPackageFullName(DI->getDef(), Sep);
   return name;
 }
 
-static std::string getPackageFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getPackageFullName(const Record *R, StringRef Sep) {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("PackageName").empty());
   name += R->getValueAsString("PackageName");
   return name;
 }
 
-static std::string getCheckerFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
+static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") {
+  std::string name = getParentPackageFullName(R, Sep);
   if (!name.empty())
-name += ".";
+name += Sep;
   assert(!R->getValueAsString("CheckerName").empty());
   name += R->getValueAsString("CheckerName");
   return name;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

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

This revision was automatically updated to reflect the committed changes.
steakhal marked an inline comment as done.
Closed by commit rG744e2a3e2232: [analyzer] ClangSA should tablegen doc urls 
refering to the main doc page (authored by steakhal).

Changed prior to commit:
  https://reviews.llvm.org/D121387?vs=417344&id=423577#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121387

Files:
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/utils/TableGen/ClangSACheckersEmitter.cpp


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -75,23 +75,18 @@
 }
 
 static std::string getCheckerDocs(const Record &R) {
-  StringRef LandingPage;
   const BitsInit *BI = R.getValueAsBitsInit("Documentation");
   if (!BI)
 PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
 getCheckerFullName(&R));
 
-  uint64_t V = getValueFromBitsInit(BI, R);
-  if (V == 1)
-LandingPage = "available_checks.html";
-  else if (V == 2)
-LandingPage = "alpha_checks.html";
-
-  if (LandingPage.empty())
+  // Ignore 'Documentation' checkers.
+  if (getValueFromBitsInit(BI, R) == 0)
 return "";
 
-  return (llvm::Twine("https://clang-analyzer.llvm.org/";) + LandingPage + "#" +
-  getCheckerFullName(&R))
+  std::string CheckerFullName = StringRef(getCheckerFullName(&R, "-")).lower();
+  return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#";) +
+  CheckerFullName)
   .str();
 }
 
Index: 
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- 
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -451,7 +451,7 @@
   "fullDescription": {
 "text": "Check for logical errors for function calls and 
Objective-C message expressions (e.g., uninitialized arguments, null function 
pointers)"
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#core.CallAndMessage";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#core-callandmessage";,
   "id": "core.CallAndMessage",
   "name": "core.CallAndMessage"
 },
@@ -459,7 +459,7 @@
   "fullDescription": {
 "text": "Check for division by zero"
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#core.DivideZero";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#core-dividezero";,
   "id": "core.DivideZero",
   "name": "core.DivideZero"
 },
@@ -467,7 +467,7 @@
   "fullDescription": {
 "text": "Check for memory leaks, double free, and 
use-after-free problems. Traces memory managed by malloc()/free()."
   },
-  "helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc";,
+  "helpUri": 
"https://clang.llvm.org/docs/analyzer/checkers.html#unix-malloc";,
   "id": "unix.Malloc",
   "name": "unix.Malloc"
 }


Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -75,23 +75,18 @@
 }
 
 static std::string getCheckerDocs(const Record &R) {
-  StringRef LandingPage;
   const BitsInit *BI = R.getValueAsBitsInit("Documentation");
   if (!BI)
 PrintFatalError(R.getLoc(), "missing Documentation<...> member for " +
 getCheckerFullName(&R));
 
-  uint64_t V = getValueFromBitsInit(BI, R);
-  if (V == 1)
-LandingPage = "available_checks.html";
-  else if (V == 2)
-LandingPage = "alpha_checks.html";
-
-  if (LandingPage.empty())
+  // Ignore 'Documentation' checkers.
+  if (getValueFromBitsInit(BI, R) == 0)
 return "";
 
-  return (llvm::Twine("https://clang-analyzer.llvm.org/";) + LandingPage + "#" +
-  getCheckerFullName(&R))
+  std::string CheckerFullName = StringRef(getCheckerFullName(&R, "-")).lower();
+  return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#";) +
+  CheckerFullName)
   .str();
 }
 
Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-s

[PATCH] D121459: [analyzer] Remove HasAlphaDocumentation tablegen enum value

[Balázs Benics 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 rG798418982630: [analyzer] Remove HasAlphaDocumentation 
tablegen enum value (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121459

Files:
  clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -236,57 +236,57 @@
 
 def BoolAssignmentChecker : Checker<"BoolAssignment">,
   HelpText<"Warn about assigning non-{0,1} values to Boolean variables">,
-  Documentation;
+  Documentation;
 
 def CastSizeChecker : Checker<"CastSize">,
   HelpText<"Check when casting a malloc'ed type T, whether the size is a "
"multiple of the size of T">,
-  Documentation;
+  Documentation;
 
 def CastToStructChecker : Checker<"CastToStruct">,
   HelpText<"Check for cast from non-struct pointer to struct pointer">,
-  Documentation;
+  Documentation;
 
 def ConversionChecker : Checker<"Conversion">,
   HelpText<"Loss of sign/precision in implicit conversions">,
-  Documentation;
+  Documentation;
 
 def IdenticalExprChecker : Checker<"IdenticalExpr">,
   HelpText<"Warn about unintended use of identical expressions in operators">,
-  Documentation;
+  Documentation;
 
 def FixedAddressChecker : Checker<"FixedAddr">,
   HelpText<"Check for assignment of a fixed address to a pointer">,
-  Documentation;
+  Documentation;
 
 def PointerArithChecker : Checker<"PointerArithm">,
   HelpText<"Check for pointer arithmetic on locations other than array "
"elements">,
-  Documentation;
+  Documentation;
 
 def PointerSubChecker : Checker<"PointerSub">,
   HelpText<"Check for pointer subtractions on two pointers pointing to "
"different memory chunks">,
-  Documentation;
+  Documentation;
 
 def SizeofPointerChecker : Checker<"SizeofPtr">,
   HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
-  Documentation;
+  Documentation;
 
 def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
   HelpText<"Check for division by variable that is later compared against 0. "
"Either the comparison is useless or there is division by zero.">,
-  Documentation;
+  Documentation;
 
 def DynamicTypeChecker : Checker<"DynamicTypeChecker">,
   HelpText<"Check for cases where the dynamic and the static type of an object "
"are unrelated.">,
-  Documentation;
+  Documentation;
 
 def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
   HelpText<"Check that addresses to stack memory do not escape the function">,
   Dependencies<[StackAddrEscapeBase]>,
-  Documentation;
+  Documentation;
 
 def PthreadLockBase : Checker<"PthreadLockBase">,
   HelpText<"Helper registering multiple checks.">,
@@ -296,7 +296,7 @@
 def C11LockChecker : Checker<"C11Lock">,
   HelpText<"Simple lock -> unlock checker">,
   Dependencies<[PthreadLockBase]>,
-  Documentation;
+  Documentation;
 
 } // end "alpha.core"
 
@@ -464,22 +464,22 @@
 def CStringOutOfBounds : Checker<"OutOfBounds">,
   HelpText<"Check for out-of-bounds access in string functions">,
   Dependencies<[CStringModeling]>,
-  Documentation;
+  Documentation;
 
 def CStringBufferOverlap : Checker<"BufferOverlap">,
   HelpText<"Checks for overlap in two buffer arguments">,
   Dependencies<[CStringModeling]>,
-  Documentation;
+  Documentation;
 
 def CStringNotNullTerm : Checker<"NotNullTerminated">,
   HelpText<"Check for arguments which are not null-terminating strings">,
   Dependencies<[CStringModeling]>,
-  Documentation;
+  Documentation;
  
 def CStringUninitializedRead : Checker<"UninitializedRead">,
   HelpText<"Checks if the string manipulation function would read uninitialized bytes">,
   Dependencies<[CStringModeling]>,
-  Documentation;
+  Documentation;
   
 } // end "alpha.unix.cstring"
 
@@ -541,24 +541,24 @@
 
 def ChrootChecker : Checker<"Chroot">,
   HelpText<"Check improper use of chroot">,
-  Documentation;
+  Documentation;
 
 def PthreadLockChecker : Checker<"PthreadLock">,
   HelpText<"Simple lock -> unlock checker">,
   Dependencies<[PthreadLockBase]>,
-  Documentation;
+  Documentation;
 
 def StreamChecker : Checker<"Stream">,
   HelpText<"Check stream handling functions">,
-  Documentation;
+  Documentation;
 
 def SimpleStreamChecker : Checker<"SimpleStream">,
   HelpText<"Check for misuses of stream APIs">,
-  Documentation;
+  Documentation;
 
 def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
   HelpText<"Check for calls to blocking functions inside a critical section">,
-  Documentation;
+  Documentation;
 
 

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/Basic/Module.h:547-548
+//  is the default name showed in module map.
+if (isGlobalModule())
+  return "";
+

I thought to add an assertion here. But I feel like it is not so necessary and 
friendly.



Comment at: clang/include/clang/Basic/Module.h:537-543
+  static StringRef getPrimaryModuleInterfaceName(StringRef Name) {
+return Name.split(':').first;
+  }
+
   /// Get the primary module interface name from a partition.
   StringRef getPrimaryModuleInterfaceName() const {
+return getPrimaryModuleInterfaceName(getTopLevelModuleName());

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > I do not understand the purpose of this change, we already iterated over 
> > > making the function more efficient.
> > > 
> > > For C++20 modules there cannot be more than one parent - so that the 
> > > nested call to getTopLevelModule(), via getTopLevelModuleName() is 
> > > redundant.
> > > 
> > > Is there some other case that needs special handling?
> > > (If so then I think we should guard this at a higher level)
> > > 
> > The primary purpose of the change is that we need to get the primary module 
> > name for getLangOpts().CurrentModule. So we need a static member function 
> > which takes a StringRef.
> > 
> > Another point might be the correctness. For example, for the private module 
> > fragment, the current implementation would return the right result while 
> > the original wouldn't. (Although we didn't use private module fragment 
> > much...)
> > 
> > For the most case, I admit the current implementation would call more 
> > function a little more. But I think it is negligible.
> (Maybe I am worrying too much - but it does seem that we have quite some 
> complexity in an operation that we expect to repeat many times per TU).
> 
> 
> 
> > The primary purpose of the change is that we need to get the primary module 
> > name for getLangOpts().CurrentModule. So we need a static member function 
> > which takes a StringRef.
> 
> That is, of course,  fine (and we are stuck in the case that we are building 
> a partition, with the fact that the primary module is not available).  It 
> seems that there's not a lot we can do there. 
> 
> When we have module,  we could cache the result of the split, tho - so have a 
> "PrimaryModuleName" entry in each module and populate it on the first use.  
> We are still stuck with the string comparison - but at least we could avoid 
> repeating the split?
> As I noted, the reason we did not bother to do this in the first use was that 
> that use was only once per TU).
> 
> When we are building a consumer of a module, on the other hand, we *do* have 
> the complete module import graph.
> 
> > Another point might be the correctness. For example, for the private module 
> > fragment, the current implementation would return the right result while 
> > the original wouldn't. (Although we didn't use private module fragment 
> > much...)
> 
> Of course, we must fix correctness issues ..
> Since there is only GMF and PMF, perhaps we could reasonably treat those 
> specially `if (isGlobalModule())` .. and/or `if (isPrivateModule())`?
> 
> > For the most case, I admit the current implementation would call more 
> > function a little more. But I think it is negligible.
> 
> It is hard to judge at the moment - perhaps we should carry on with what we 
> have and then instrument some reasonable body of code?
> (Maybe I am worrying too much - but it does seem that we have quite some 
> complexity in an operation that we expect to repeat many times per TU).

Never mind! This is the meaning of reviewing.

> Of course, we must fix correctness issues ..
Since there is only GMF and PMF, perhaps we could reasonably treat those 
specially if (isGlobalModule()) .. and/or if (isPrivateModule())?

Done.

After consideration, I think what you here makes sense. Especially now 
Module::getPrimaryModuleInterfaceName is only used for 
Sema.getLangOpts().CurrentModule. So it might be better to handle this in Sema 
or LangOptions.



Comment at: clang/lib/Sema/SemaLookup.cpp:1567-1568
+
+  return M->getPrimaryModuleInterfaceName() ==
+ Module::getPrimaryModuleInterfaceName(LangOpts.CurrentModule);
 }

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ... of course, "correctness before optimisation", but this test function 
> > > seems like it would benefit from some refactoring as suggested below.
> > > 
> > > it seems a bit unfortunate that we are placing a string comparison in the 
> > > "acceptable decl" lookup path, which might be made many times (the 
> > > previous use of the string compare was in making the module decl - which 
> > > only happens once in a TU).
> > > 
> > > * Sema has visibility of the import graph of modules (via 
> > > DirectModuleImports and Exports).
> > > * We also know what the

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 423579.
ChuanqiXu added a comment.

Rename Sema::CurrentModuleUnitsCache to Sema::UsableModuleUnitsCache


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

https://reviews.llvm.org/D123837

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CXX/module/module.import/p2.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp

Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -56,7 +56,14 @@
 int &c = n; // expected-error {{use of undeclared identifier}}
 
 //--- std10-1-ex2-tu7.cpp
+// expected-no-diagnostics
 module B:X3; // does not implicitly import B
-import :X2;  // X2 is an implementation so exports nothing.
- // error: n not visible here.
-int &c = n;  // expected-error {{use of undeclared identifier }}
+import : X2; // X2 is an implementation unit import B.
+// According to [module.import]p7:
+//   Additionally, when a module-import-declaration in a module unit of some
+//   module M imports another module unit U of M, it also imports all
+//   translation units imported by non-exported module-import-declarations in
+//   the module unit purview of U.
+//
+// So B is imported in B:X3 due to B:X2 imported B. So n is visible here.
+int &c = n;
Index: clang/test/CXX/module/module.import/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.import/p2.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c++20 %t/impl.cppm -emit-module-interface -o %t/M-impl.pcm
+// RUN: %clang_cc1 -std=c++20 %t/M.cppm -emit-module-interface -fprebuilt-module-path=%t -o %t/M.pcm
+// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/UseInPartA.cppm -fprebuilt-module-path=%t -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/UseInAnotherModule.cppm -fprebuilt-module-path=%t -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/Private.cppm -emit-module-interface -fprebuilt-module-path=%t -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 %t/TryUseFromPrivate.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
+
+//--- impl.cppm
+module M : impl;
+class A {};
+
+//--- M.cppm
+export module M;
+import : impl;
+export A f();
+
+//--- Use.cpp
+import M;
+void test() {
+  A a; // expected-error {{unknown type name 'A'}}
+}
+
+//--- UseInPartA.cppm
+// expected-no-diagnostics
+export module M : partA;
+import : impl;
+void test() {
+  A a;
+}
+
+//--- UseInAnotherModule.cppm
+export module B;
+import M;
+void test() {
+  A a; // expected-error {{unknown type name 'A'}}
+}
+
+//--- Private.cppm
+export module A;
+module : private;
+
+class A {};
+void test() {
+  A a;
+}
+
+//--- TryUseFromPrivate.cpp
+
+import A;
+void test() {
+  A a; // expected-error {{unknown type name 'A'}}
+}
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1556,17 +1556,32 @@
   return LookupModulesCache;
 }
 
-/// Determine whether the module M is part of the current module from the
-/// perspective of a module-private visibility check.
-static bool isInCurrentModule(const Module *M, const LangOptions &LangOpts) {
+/// Determine if we could use all the declarations in the module.
+bool Sema::isUsableModule(const Module *M) {
+  assert(M && "We shouldn't check nullness for module here");
+  // Return quickly if we cached the result.
+  if (UsableModuleUnitsCache.count(M))
+return true;
+
   // If M is the global module fragment of a module that we've not yet finished
-  // parsing, then it must be part of the current module.
-  // If it's a partition, then it must be visible to an importer (since only
-  // another partition or the named module can import it).
-  return M->getTopLevelModuleName() == LangOpts.CurrentModule ||
- (M->Kind == Module::GlobalModuleFragment && !M->Parent) ||
- M->Kind == Module::ModulePartitionInterface ||
- M->Kind == Module::ModulePartitionImplementation;
+  // parsing, then M should be the global module fragment in current TU. So it
+  // should be usable.
+  // [module.global.frag]p1:
+  //   The global module fragment can be used to provide declarations that are
+  //   attached to the global module and usable within the module unit.
+  if ((M->isGlobalModule() && !M->Parent) ||
+  // The module unit which is in the same module with the current module
+  // unit is usable.
+  //
+  // FIXME: Here we judge if they are in the same module by comparing the
+  // string. Is there any better solution?
+  (M->getPrimaryModuleInterfaceName() ==
+   llvm::StringRef(getLangOpts().CurrentM

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaLookup.cpp:1579
+  (M->getPrimaryModuleInterfaceName() ==
+   llvm::StringRef(getLangOpts().CurrentModule).split(':').first)) {
+CurrentModuleUnitsCache.insert(M);

I thought to add a comment for this. But I feel like it might be too wordy...


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

https://reviews.llvm.org/D123837

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

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

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

steakhal added a comment.

In D123155#3433715 , @tomrittervg 
wrote:

> Just a note - I was able to test this finally and it works for what I was 
> trying to do.

Do you have commit access? Or we shall commit this on your behalf?
If the latter, please tell us what to use for the `--author`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123155

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

[PATCH] D123837: [C++20] [Modules] Judge isInCurrentModule currently

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

it would be good to add tests for GMF and PMF cases?




Comment at: clang/include/clang/Basic/Module.h:550-553
 if (isModulePartition()) {
   auto pos = Name.find(':');
   return StringRef(Name.data(), pos);
 }

if we find this is repeated too many times, we could cache it in the module as 
PrimaryModuleName.



Comment at: clang/include/clang/Basic/Module.h:537-543
+  static StringRef getPrimaryModuleInterfaceName(StringRef Name) {
+return Name.split(':').first;
+  }
+
   /// Get the primary module interface name from a partition.
   StringRef getPrimaryModuleInterfaceName() const {
+return getPrimaryModuleInterfaceName(getTopLevelModuleName());

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > I do not understand the purpose of this change, we already iterated 
> > > > over making the function more efficient.
> > > > 
> > > > For C++20 modules there cannot be more than one parent - so that the 
> > > > nested call to getTopLevelModule(), via getTopLevelModuleName() is 
> > > > redundant.
> > > > 
> > > > Is there some other case that needs special handling?
> > > > (If so then I think we should guard this at a higher level)
> > > > 
> > > The primary purpose of the change is that we need to get the primary 
> > > module name for getLangOpts().CurrentModule. So we need a static member 
> > > function which takes a StringRef.
> > > 
> > > Another point might be the correctness. For example, for the private 
> > > module fragment, the current implementation would return the right result 
> > > while the original wouldn't. (Although we didn't use private module 
> > > fragment much...)
> > > 
> > > For the most case, I admit the current implementation would call more 
> > > function a little more. But I think it is negligible.
> > (Maybe I am worrying too much - but it does seem that we have quite some 
> > complexity in an operation that we expect to repeat many times per TU).
> > 
> > 
> > 
> > > The primary purpose of the change is that we need to get the primary 
> > > module name for getLangOpts().CurrentModule. So we need a static member 
> > > function which takes a StringRef.
> > 
> > That is, of course,  fine (and we are stuck in the case that we are 
> > building a partition, with the fact that the primary module is not 
> > available).  It seems that there's not a lot we can do there. 
> > 
> > When we have module,  we could cache the result of the split, tho - so have 
> > a "PrimaryModuleName" entry in each module and populate it on the first 
> > use.  We are still stuck with the string comparison - but at least we could 
> > avoid repeating the split?
> > As I noted, the reason we did not bother to do this in the first use was 
> > that that use was only once per TU).
> > 
> > When we are building a consumer of a module, on the other hand, we *do* 
> > have the complete module import graph.
> > 
> > > Another point might be the correctness. For example, for the private 
> > > module fragment, the current implementation would return the right result 
> > > while the original wouldn't. (Although we didn't use private module 
> > > fragment much...)
> > 
> > Of course, we must fix correctness issues ..
> > Since there is only GMF and PMF, perhaps we could reasonably treat those 
> > specially `if (isGlobalModule())` .. and/or `if (isPrivateModule())`?
> > 
> > > For the most case, I admit the current implementation would call more 
> > > function a little more. But I think it is negligible.
> > 
> > It is hard to judge at the moment - perhaps we should carry on with what we 
> > have and then instrument some reasonable body of code?
> > (Maybe I am worrying too much - but it does seem that we have quite some 
> > complexity in an operation that we expect to repeat many times per TU).
> 
> Never mind! This is the meaning of reviewing.
> 
> > Of course, we must fix correctness issues ..
> Since there is only GMF and PMF, perhaps we could reasonably treat those 
> specially if (isGlobalModule()) .. and/or if (isPrivateModule())?
> 
> Done.
> 
> After consideration, I think what you here makes sense. Especially now 
> Module::getPrimaryModuleInterfaceName is only used for 
> Sema.getLangOpts().CurrentModule. So it might be better to handle this in 
> Sema or LangOptions.
 
>  Especially now Module::getPrimaryModuleInterfaceName is only used for 
> Sema.getLangOpts().CurrentModule. So it might be better to handle this in 
> Sema or LangOptions.

Yeah, perhaps cache that split in Sema - adding another LangOption does not 
feel quite right (unless we find we need this information more widely - but I 
didn't see that yet).



Comment at: clang/lib/Sema/SemaLookup.cpp:1567-1568
+
+  return M->getPrimaryModuleInterfaceName() ==
+ Module::getPrimaryModuleInterfaceName(LangOpts.CurrentModule);
 }

ChuanqiXu wrote:
> iains w

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 423582.
kwk added a comment.

- Make functions lowercase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

Files:
  clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp

Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -169,24 +169,6 @@
   });
 }
 
-inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>;");
-}
-
-const char IncludeRegexPattern[] =
-R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
-
-// Returns the last match group in the above regex (IncludeRegexPattern) that
-// is not empty.
-StringRef getIncludeNameFromMatches(const SmallVectorImpl &Matches) {
-  for (int i = Matches.size() - 1; i > 0; i--) {
-if (!Matches[i].empty())
-  return Matches[i];
-  }
-  llvm_unreachable("No non-empty match group found in list of matches");
-  return StringRef();
-}
-
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
 //  - in names with multiple dots (foo.cu.cc) it terminates at the *first*
@@ -285,8 +267,7 @@
   MaxInsertOffset(MinInsertOffset +
   getMaxHeaderInsertionOffset(
   FileName, Code.drop_front(MinInsertOffset), Style)),
-  Categories(Style, FileName),
-  IncludeRegex(llvm::Regex(IncludeRegexPattern)) {
+  Categories(Style, FileName), IncludeRegex(getCppIncludeRegex()) {
   // Add 0 for main header and INT_MAX for headers that are not in any
   // category.
   Priorities = {0, INT_MAX};
@@ -301,7 +282,7 @@
   for (auto Line : Lines) {
 NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
 if (IncludeRegex.match(Line, &Matches)) {
-  StringRef IncludeName = getIncludeNameFromMatches(Matches);
+  StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
   // If this is the last line without trailing newline, we need to make
   // sure we don't delete across the file boundary.
   addExistingInclude(
@@ -415,5 +396,25 @@
   return Result;
 }
 
+llvm::Regex getCppIncludeRegex() {
+  static const char CppIncludeRegexPattern[] =
+  R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
+  return llvm::Regex(CppIncludeRegexPattern);
+}
+
+llvm::StringRef getIncludeNameFromMatches(
+const llvm::SmallVectorImpl &Matches) {
+  for (auto Match : llvm::reverse(Matches)) {
+if (!Match.empty())
+  return Match;
+  }
+  llvm_unreachable("No non-empty match group found in list of matches");
+  return llvm::StringRef();
+}
+
+llvm::StringRef trimInclude(llvm::StringRef IncludeName) {
+  return IncludeName.trim("\"<>;");
+}
+
 } // namespace tooling
 } // namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2722,23 +2722,6 @@
   }
 }
 
-namespace {
-
-const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
-} // anonymous namespace
-
-// Returns the last match group in the above regex (CppIncludeRegexPattern) that
-// is not empty.
-StringRef getIncludeNameFromMatches(const SmallVectorImpl &Matches) {
-  for (int i = Matches.size() - 1; i > 0; i--) {
-if (!Matches[i].empty())
-  return Matches[i];
-  }
-  llvm_unreachable("No non-empty match group found in list of matches");
-  return StringRef();
-}
-
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
   ArrayRef Ranges,
   StringRef FileName,
@@ -2748,7 +2731,7 @@
   .StartsWith("\xEF\xBB\xBF", 3) // UTF-8 BOM
   .Default(0);
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(CppIncludeRegexPattern);
+  llvm::Regex IncludeRegex(tooling::getCppIncludeRegex());
   SmallVector Matches;
   SmallVector IncludesInBlock;
 
@@ -2804,7 +2787,7 @@
 bool MergeWithNextLine = Trimmed.endswith("\\");
 if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
-StringRef IncludeName = getIncludeNameFromMatches(Matches);
+StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
 // This addresses https://github.com/llvm/llvm-project/issues/38995
 bool WithSemicolon = false;
 if (!IncludeName.startswith("\"") && !IncludeName.startswith("<") &&
@@ -3087,20 +3070,15 @@
 
 inline bool isHeaderInsertion(const tooling::Replacement

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk marked 2 inline comments as done.
kwk added a comment.

In D121370#3456223 , @owenpan wrote:

> Should we handle `#import` and `@import` for Object-C only so as to simply 
> the regex for C++?

@owenpan my counter question probably reveals how few things I know about 
clang-format and how much I still have to learn. I only see `sortCppIncludes` 
but no `sortObjCIncludes` in the function list in `Format.h`. That said, do you 
want to introduce that or do you want to put logic in `sortCppIncludes` that 
knows about the programming language at hand and decides which regex to pick?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 423584.
kwk added a comment.

- typo: @include bar; -> @import bar;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

Files:
  clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp

Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -169,24 +169,6 @@
   });
 }
 
-inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>;");
-}
-
-const char IncludeRegexPattern[] =
-R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
-
-// Returns the last match group in the above regex (IncludeRegexPattern) that
-// is not empty.
-StringRef getIncludeNameFromMatches(const SmallVectorImpl &Matches) {
-  for (int i = Matches.size() - 1; i > 0; i--) {
-if (!Matches[i].empty())
-  return Matches[i];
-  }
-  llvm_unreachable("No non-empty match group found in list of matches");
-  return StringRef();
-}
-
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
 //  - in names with multiple dots (foo.cu.cc) it terminates at the *first*
@@ -285,8 +267,7 @@
   MaxInsertOffset(MinInsertOffset +
   getMaxHeaderInsertionOffset(
   FileName, Code.drop_front(MinInsertOffset), Style)),
-  Categories(Style, FileName),
-  IncludeRegex(llvm::Regex(IncludeRegexPattern)) {
+  Categories(Style, FileName), IncludeRegex(getCppIncludeRegex()) {
   // Add 0 for main header and INT_MAX for headers that are not in any
   // category.
   Priorities = {0, INT_MAX};
@@ -301,7 +282,7 @@
   for (auto Line : Lines) {
 NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
 if (IncludeRegex.match(Line, &Matches)) {
-  StringRef IncludeName = getIncludeNameFromMatches(Matches);
+  StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
   // If this is the last line without trailing newline, we need to make
   // sure we don't delete across the file boundary.
   addExistingInclude(
@@ -415,5 +396,25 @@
   return Result;
 }
 
+llvm::Regex getCppIncludeRegex() {
+  static const char CppIncludeRegexPattern[] =
+  R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
+  return llvm::Regex(CppIncludeRegexPattern);
+}
+
+llvm::StringRef getIncludeNameFromMatches(
+const llvm::SmallVectorImpl &Matches) {
+  for (auto Match : llvm::reverse(Matches)) {
+if (!Match.empty())
+  return Match;
+  }
+  llvm_unreachable("No non-empty match group found in list of matches");
+  return llvm::StringRef();
+}
+
+llvm::StringRef trimInclude(llvm::StringRef IncludeName) {
+  return IncludeName.trim("\"<>;");
+}
+
 } // namespace tooling
 } // namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2722,23 +2722,6 @@
   }
 }
 
-namespace {
-
-const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
-} // anonymous namespace
-
-// Returns the last match group in the above regex (CppIncludeRegexPattern) that
-// is not empty.
-StringRef getIncludeNameFromMatches(const SmallVectorImpl &Matches) {
-  for (int i = Matches.size() - 1; i > 0; i--) {
-if (!Matches[i].empty())
-  return Matches[i];
-  }
-  llvm_unreachable("No non-empty match group found in list of matches");
-  return StringRef();
-}
-
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
   ArrayRef Ranges,
   StringRef FileName,
@@ -2748,7 +2731,7 @@
   .StartsWith("\xEF\xBB\xBF", 3) // UTF-8 BOM
   .Default(0);
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(CppIncludeRegexPattern);
+  llvm::Regex IncludeRegex(tooling::getCppIncludeRegex());
   SmallVector Matches;
   SmallVector IncludesInBlock;
 
@@ -2804,7 +2787,7 @@
 bool MergeWithNextLine = Trimmed.endswith("\\");
 if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
-StringRef IncludeName = getIncludeNameFromMatches(Matches);
+StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
 // This addresses https://github.com/llvm/llvm-project/issues/38995
 bool WithSemicolon = false;
 if (!IncludeName.startswith("\"") && !IncludeName.startswith("<") &&
@@ -3087,20 +3070,15 @@
 
 inline bool isHeaderInsertion(const tooling::

[PATCH] D123961: [clang][dataflow] Do not crash on missing `Value` for struct-typed variable init.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:184-186
-  // FIXME: The initializer expression must always be assigned a value.
-  // Replace this with an assert when we have sufficient coverage of
-  // language features.

The patch makes sense to me in the current state, but it's unclear why is this 
not something that we'd like to do in the long term.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123961

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

[PATCH] D123858: [clang][dataflow] Ensure well-formed flow conditions.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:112-113
 cast_or_null(Env.getValue(Cond, SkipPast::Reference));
-if (Val == nullptr)
-  return;
+// Value merging depends on flow conditions from different environments
+// being mutually exclusive. So, we need *some* value for the condition
+// expression, even if just an atom.

What does it mean for flow conditions to be mutually exclusive? Why is this a 
requirement?



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:123-136
+  } else if (auto *CondVal = cast_or_null(
+ Env.getValue(Cond, SkipPast::None))) {
+Env.setValue(CondVal->getPointeeLoc(), *Val);
+  } else {
+QualType PointeeType = Type->castAs()->getPointeeType();
+StorageLocation &PointeeLoc = Env.createStorageLocation(PointeeType);
+Env.setValue(PointeeLoc, *Val);

Let's also add tests for these two paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123858

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

[PATCH] D123957: Update the developer policy to mention release notes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 423593.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Updating based on review feedback.

Fixed some grammar issues, added a bullet for fixing bugs, modified the bullet 
about diagnostics to include regrouping under a new diagnostic name.


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

https://reviews.llvm.org/D123957

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -185,12 +185,14 @@
 
 Many projects in LLVM communicate important changes to users through release
 notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to
-a project that are user-facing or users may wish to know about should be added
-to the project's release notes. Examples of changes that would typically
-warrant adding a release note (this list is not exhaustive):
-
-* Adding, removing, or modifying command line options.
-* Adding or removing a diagnostic.
+a project that are user-facing, or that users may wish to know about, should be
+added to the project's release notes, preferably as part of the commit landing
+the changes. Examples of changes that would typically warrant adding a release
+note (this list is not exhaustive):
+
+* Adding, removing, or modifying command-line options.
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
 * Adding or removing an optimization.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -185,12 +185,14 @@
 
 Many projects in LLVM communicate important changes to users through release
 notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to
-a project that are user-facing or users may wish to know about should be added
-to the project's release notes. Examples of changes that would typically
-warrant adding a release note (this list is not exhaustive):
-
-* Adding, removing, or modifying command line options.
-* Adding or removing a diagnostic.
+a project that are user-facing, or that users may wish to know about, should be
+added to the project's release notes, preferably as part of the commit landing
+the changes. Examples of changes that would typically warrant adding a release
+note (this list is not exhaustive):
+
+* Adding, removing, or modifying command-line options.
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
 * Adding or removing an optimization.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123957: Update the developer policy to mention release notes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 423594.
aaron.ballman added a comment.

Something went funny during a rebase, so this is the full update, not just 
changes since last time.


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

https://reviews.llvm.org/D123957

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -180,6 +180,27 @@
 for coverage (correctness, performance, etc) testing, not feature or regression
 testing.
 
+Release Notes
+-
+
+Many projects in LLVM communicate important changes to users through release
+notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to
+a project that are user-facing, or that users may wish to know about, should be
+added to the project's release notes, preferably as part of the commit landing
+the changes. Examples of changes that would typically warrant adding a release
+note (this list is not exhaustive):
+
+* Adding, removing, or modifying command-line options.
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
+* Adding or removing an optimization.
+* Modifying a C stable API.
+* Notifying users about a potentially disruptive change expected to be made in
+  a future release, such as removal of a deprecated feature.
+
+Code reviewers are encouraged to request a release note if they think one is
+warranted when performing a code review.
+
 Quality
 ---
 


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -180,6 +180,27 @@
 for coverage (correctness, performance, etc) testing, not feature or regression
 testing.
 
+Release Notes
+-
+
+Many projects in LLVM communicate important changes to users through release
+notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to
+a project that are user-facing, or that users may wish to know about, should be
+added to the project's release notes, preferably as part of the commit landing
+the changes. Examples of changes that would typically warrant adding a release
+note (this list is not exhaustive):
+
+* Adding, removing, or modifying command-line options.
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
+* Adding or removing an optimization.
+* Modifying a C stable API.
+* Notifying users about a potentially disruptive change expected to be made in
+  a future release, such as removal of a deprecated feature.
+
+Code reviewers are encouraged to request a release note if they think one is
+warranted when performing a code review.
+
 Quality
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123957: Update the developer policy to mention release notes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: xbolva00.
aaron.ballman added a comment.

In D123957#3458354 , @hans wrote:

> If it's possible, maybe this could be worded in a way that strongly suggests 
> the release notes should ideally be updated in the same patch as the code 
> change?
> I think we're generally good at doing this for tests. It would be great if we 
> could treat release notes the same.

That's a good idea! This was also independently suggested by @xbolva00 on the 
RFC thread.


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

https://reviews.llvm.org/D123957

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

[PATCH] D123957: Update the developer policy to mention release notes

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Looks great to me!


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

https://reviews.llvm.org/D123957

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

[clang] 7641004 - Revert "[Concepts] Fix overload resolution bug with constrained candidates"

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2022-04-19T07:51:21-04:00
New Revision: 76410040b9f391185c7df48c14519860e1cf75e5

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

LOG: Revert "[Concepts] Fix overload resolution bug with constrained candidates"

This reverts commit 454d1df9423c95e54c3a2f5cb58d864096032d09.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 
clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a7a6fab5fe535..c396322f2de57 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -123,11 +123,6 @@ Bug Fixes
   a lambda expression that shares the name of a variable in a containing
   if/while/for/switch init statement as a redeclaration.
   This fixes `Issue 54913 
`_.
-- Overload resolution for constrained function templates could use the partial
-  order of constraints to select an overload, even if the parameter types of
-  the functions were 
diff erent. It now diagnoses this case correctly as an
-  ambiguous call and an error. Fixes
-  `Issue 53640 `_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fd9433cb1b786..e2ae02ea9f76f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3590,8 +3590,7 @@ class Sema final {
 QualType& ConvertedType);
   bool FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
   const FunctionProtoType *NewType,
-  unsigned *ArgPos = nullptr,
-  bool Reversed = false);
+  unsigned *ArgPos = nullptr);
   void HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
   QualType FromType, QualType ToType);
 
@@ -8769,8 +8768,7 @@ class Sema final {
   FunctionTemplateDecl *getMoreSpecializedTemplate(
   FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
   TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-  unsigned NumCallArguments2, bool Reversed = false,
-  bool AllowOrderingByConstraints = true);
+  unsigned NumCallArguments2, bool Reversed = false);
   UnresolvedSetIterator
   getMostSpecialized(UnresolvedSetIterator SBegin, UnresolvedSetIterator SEnd,
  TemplateSpecCandidateSet &FailedCandidates,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 6e45fdfbeb089..1add971a81a02 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2945,30 +2945,24 @@ void Sema::HandleFunctionTypeMismatch(PartialDiagnostic 
&PDiag,
 }
 
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
-/// for equality of their parameter types. Caller has already checked that
-/// they have same number of parameters.  If the parameters are 
diff erent,
+/// for equality of their argument types. Caller has already checked that
+/// they have same number of arguments.  If the parameters are 
diff erent,
 /// ArgPos will have the parameter index of the first 
diff erent parameter.
-/// If `Reversed` is true, the parameters of `NewType` will be compared in
-/// reverse order. That's useful if one of the functions is being used as a 
C++20
-/// synthesized operator overload with a reversed parameter order.
 bool Sema::FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
   const FunctionProtoType *NewType,
-  unsigned *ArgPos, bool Reversed) {
-  assert(OldType->getNumParams() == NewType->getNumParams() &&
- "Can't compare parameters of functions with 
diff erent number of "
- "parameters!");
-  for (size_t I = 0; I < OldType->getNumParams(); I++) {
-// Reverse iterate over the parameters of `OldType` if `Reversed` is true.
-size_t J = Reversed ? (OldType->getNumParams() - I - 1) : I;
-
+  unsigned *ArgPos) {
+  for (FunctionProtoType::param_type_iterator O = OldType->param_type_begin(),
+  N = NewType->param_type_begin(),
+  E = OldType->param_type_end();
+   O && (O != E); ++O, ++N) {
 // Ignore address spaces in pointee type. This is to disallow overloading
 // on __ptr32/__ptr64 address spaces

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a subscriber: Mordante.
royjacobson added a comment.

So, it seems like this broke one of `basic_arg_format`'s private constructors.. 
Sorry for that.

  //format_arg.h:167
  explicit basic_format_arg(_Tp __v) noexcept
requires(same_as<_Tp, char_type> ||
 (same_as<_Tp, char> && same_as)) []
  
  //format_arg.h:248
  explicit basic_format_arg(const _Tp& __v) [...]

This is now ambiguous when calling `basic_format_arg(char)` because the 
argument type is different and comparing constraints is not allowed. @Mordante 
could you maybe take a look? I've reverted in the meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D123955: [C2x] Disallow functions without prototypes/functions with identifier lists

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as not done.
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:124
 BENIGN_LANGOPT(ImplicitInt, 1, 0, "C89 implicit 'int'")
+LANGOPT(StrictPrototypes  , 1, 0, "require function types to have a prototype")
 LANGOPT(Digraphs  , 1, 0, "digraphs")

aaron.ballman wrote:
> rsmith wrote:
> > This makes me think we should have some declarative way of specifying 
> > dependencies between `LANGOPT`s. It's presumably sufficiently obvious to a 
> > library user that they shouldn't enable (eg) `CPlusPlus20` unless they 
> > enable all the previous `CPlusPlusXY` modes and `CPlusPlus`, but I doubt 
> > it's obvious that enabling `CPlusPlus` requires also enabling 
> > `StrictPrototypes`.
> > 
> > In fact, after this change, I think a lot of existing library users of 
> > Clang that invent their own `LangOptions` will silently start getting this 
> > wrong. That's concerning. Maybe we should consider prototypes to be 
> > required if either `StrictPrototypes` or `CPlusPlus` is enabled?
> > This makes me think we should have some declarative way of specifying 
> > dependencies between LANGOPTs.
> 
> Tee hee: 
> https://github.com/llvm/llvm-project/commit/be9371659380388a693ec99624e1f3d02f07047f
> 
> I was caught by the same issues that I ended up fixing in that commit -- I 
> tried making `StrictPrototypes` dependent and was very surprised when it 
> doesn't work. The issue was 
> `Invocation->getLangOpts()->resetNonModularOptions();` in 
> `compileModuleImpl()` IIRC -- it would clear all of the language options 
> (including ones like digraphs and wchar support).
> 
> > In fact, after this change, I think a lot of existing library users of 
> > Clang that invent their own LangOptions will silently start getting this 
> > wrong. That's concerning. Maybe we should consider prototypes to be 
> > required if either StrictPrototypes or CPlusPlus is enabled?
> 
> @cor3ntin also shared this same concern -- my goal with this language option 
> was:
> 
> * Make it easy to reenable functions without prototypes in C2x mode if we 
> find some major breakage in the wild (hopefully we don't)
> * Make it easy to add `-fstrict-prototypes` as a language flag to opt *into* 
> strict prototypes (there would be no option to opt *out* of strict 
> prototypes).
> * Stop repeating `CPlusPlus || C2x` in a bunch of places and give it a named 
> option.
> 
> So I'm not keen on adding `StrictPrototypes || CPlusPlus` everywhere -- C++ 
> requires strict prototypes. However, the fears are still valid -- what if I 
> found someplace nice to add an assert that `StrictPrototypes` cannot be false 
> when `CPlusPlus` is true?
>> This makes me think we should have some declarative way of specifying 
>> dependencies between LANGOPTs.
> Tee hee: 
> https://github.com/llvm/llvm-project/commit/be9371659380388a693ec99624e1f3d02f07047f
>
> I was caught by the same issues that I ended up fixing in that commit -- I 
> tried making StrictPrototypes dependent and was very surprised when it 
> doesn't work. The issue was 
> Invocation->getLangOpts()->resetNonModularOptions(); in compileModuleImpl() 
> IIRC -- it would clear all of the language options (including ones like 
> digraphs and wchar support).

There's actually a secondary issue, which is that users can set language 
options directly -- there's not a setter function for them. This means that 
places where we create a `LangOptions` object and manually set it up (like our 
unit tests) have no way to automatically set `StrictPrototypes` when setting 
other language options.

However, this speaks to the importance of having an assert to ensure that by 
the time the compiler instance is invoked, we have language options that are 
correct (on the assumption that later stages will not be modifying the language 
options particularly often).


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

https://reviews.llvm.org/D123955

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

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 423595.
whisperity added a comment.

**[NFC]** Fix the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123992

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -5,6 +5,10 @@
 void nested_func() {}
 } // namespace nested
 void libc_api_func() {}
+
+struct libc_api_struct {
+  int operator()() const { return 0; }
+};
 } // namespace __llvm_libc
 
 // Emulate a function from the public headers like string.h
@@ -13,6 +17,11 @@
 // Emulate a function specifically allowed by the exception list.
 void malloc() {}
 
+// Emulate a non-trivially named symbol.
+struct global_struct {
+  int operator()() const { return 0; }
+};
+
 namespace __llvm_libc {
 void Test() {
   // Allow calls with the fully qualified name.
@@ -30,19 +39,28 @@
   void (*barePtr)(void) = __llvm_libc::libc_api_func;
   barePtr();
 
+  // Allow calling entities defined in the namespace.
+  __llvm_libc::libc_api_struct{}();
+
   // Disallow calling into global namespace for implemented entrypoints.
   ::libc_api_func();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
-  // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+  // CHECK-MESSAGES: :15:6: note: resolves to this declaration
 
   // Disallow indirect references to functions in global namespace.
   void (*badPtr)(void) = ::libc_api_func;
   badPtr();
   // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
-  // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+  // CHECK-MESSAGES: :15:6: note: resolves to this declaration
 
   // Allow calling into global namespace for specific functions.
   ::malloc();
+
+  // Disallow calling on entities that are not in the namespace, but make sure
+  // no crashes happen.
+  global_struct{}();
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'operator()' must resolve to a function declared within the '__llvm_libc' namespace
+  // CHECK-MESSAGES: :22:7: note: resolves to this declaration
 }
 
 } // namespace __llvm_libc
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,31 +136,39 @@
 Changes in existing checks
 ^^
 
-- Improved :doc:`performance-inefficient-vector-operation 
-  ` to work when
-  the vector is a member of a structure.
-
-- Fixed a false positive in :doc:`readability-non-const-parameter
-  ` when the parameter is referenced by an lvalue.
-
-- Fixed a crash in :doc:`readability-const-return-type
-  ` when a pure virtual function
-  overrided has a const return type. Removed the fix for a virtual function.
+- Fixed a crash in :doc:`bugprone-sizeof-expression
+  ` when `sizeof(...)` is
+  compared against a `__int128_t`.
 
-- Fixed a false positive in :doc:`misc-redundant-expression `
-  involving overloaded comparison operators.
-
-- Fixed a crash in :doc:`bugprone-sizeof-expression ` when
-  `sizeof(...)` is compared agains a `__int128_t`.
-  
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
-- Fixed a false positive in :doc:`misc-redundant-expression `
-  involving assignments in conditions. This fixes `Issue 35853 `_.
+- Fixed a crash in :doc:`llvmlibc-callee-namespace
+  ` when executing for C++ code
+  that contain calls to advanced constructs, e.g. overloaded operators.
+
+- Fixed a false positive in :doc:`misc-redundant-expression
+  ` involving overloaded
+  comparison operators.
+
+- Fixed a false positive in :doc:`misc-redundant-expression
+  ` involving assignments in
+  conditions. This fixes `Issue 35853 `_.
+
+- Fixed a crash in :doc:`readability-const-return-type
+  ` when a pure virtual function
+  overrided has a const return type. Removed the fix for a virtual function.
+
+- Fixed a false positive in :doc:`readability-non-const-parameter
+  ` when the parameter is
+  referenced by an lvalue.
+
+- Improved :doc:`performance-inefficient-vector-operation
+  ` to work when
+  the vector is a member of a structure.
 
 Re

[PATCH] D123957: Update the developer policy to mention release notes

[Nikita Popov via Phabricator via cfe-commits]

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



Comment at: llvm/docs/DeveloperPolicy.rst:195
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
+* Adding or removing an optimization.

I disagree with this point. Bug fixes should not make it into the release notes 
as a matter of course -- there may be specific high-impact bug fixes that may 
be worth mentioning, but bug fixes should not be included in the release notes 
as a matter of policy.

I agree that release notes for Clang/LLVM are currently insufficient, but we 
should also be careful not to over-compensate in the other direction, but 
including too much irrelevant noise.


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

https://reviews.llvm.org/D123957

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

[PATCH] D123957: Update the developer policy to mention release notes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:195
+* Adding, removing, or regrouping a diagnostic.
+* Fixing a bug (please link to the issue fixed in the bug database).
+* Adding or removing an optimization.

nikic wrote:
> I disagree with this point. Bug fixes should not make it into the release 
> notes as a matter of course -- there may be specific high-impact bug fixes 
> that may be worth mentioning, but bug fixes should not be included in the 
> release notes as a matter of policy.
> 
> I agree that release notes for Clang/LLVM are currently insufficient, but we 
> should also be careful not to over-compensate in the other direction, but 
> including too much irrelevant noise.
> I disagree with this point. Bug fixes should not make it into the release 
> notes as a matter of course -- there may be specific high-impact bug fixes 
> that may be worth mentioning, but bug fixes should not be included in the 
> release notes as a matter of policy.

I disagree (quite strongly) with this and would point out that users have 
explicitly requested this information be included: 
https://discourse.llvm.org/t/rfc-update-developer-policy-on-release-notes/61856/3

> I agree that release notes for Clang/LLVM are currently insufficient, but we 
> should also be careful not to over-compensate in the other direction, but 
> including too much irrelevant noise.

Bug fixes are generally far from irrelevant, but I agree that reviewer and 
author discretion is advised (as with any release note). For example, if you 
introduce a bug in version N and fix the bug in version N, there's no need for 
a release note in that situation because there's no user-facing change as far 
as users care about. But I don't want to try to spell that out in precise 
detail because there will always be edge cases.


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

https://reviews.llvm.org/D123957

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

[clang] 944b25a - [OpenMP] Make Xopenmp-target args compile-only to silence warnings

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-04-19T08:42:43-04:00
New Revision: 944b25aee393ceb31d847fb4d3b98695852eecc9

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

LOG: [OpenMP] Make Xopenmp-target args compile-only to silence warnings

Summary:
Previously we needed the `Xopenmp-target=` option during the linking
phase so the old offloading driver knew which items to extract and link
for the device. Now that the new driver has become the default this is
no longer necessary and will cause a warning to be emitted for the
unused argument. This should be silenced to avoid noise.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a1f4075ad9590..6ec7ddb331ecb 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -812,9 +812,9 @@ def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass  to fatbinary invocation">, MetaVarName<"">;
 def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">,
   HelpText<"Pass  to the ptxas assembler">, MetaVarName<"">;
-def Xopenmp_target : Separate<["-"], "Xopenmp-target">,
+def Xopenmp_target : Separate<["-"], "Xopenmp-target">, 
Group,
   HelpText<"Pass  to the target offloading toolchain.">, 
MetaVarName<"">;
-def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">,
+def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">, 
Group,
   HelpText<"Pass  to the target offloading toolchain identified by 
.">,
   MetaVarName<" ">;
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,



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

[clang] 0f8b8d7 - [OpenMP][Docs] Remove old 14.0 release information

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-04-19T08:45:51-04:00
New Revision: 0f8b8d79af8bf97a414986cdebfa728ce5018f80

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

LOG: [OpenMP][Docs] Remove old 14.0 release information

Summary:
This patch removes the OpenMP sections in the release notes. These will
be filled once the release is close and implementations are finalized.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c396322f2de57..ef4211204a905 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -282,18 +282,7 @@ ABI Changes in Clang
 OpenMP Support in Clang
 ---
 
-- ``clang-nvlink-wrapper`` tool introduced to support linking of cubin files
-  archived in an archive. See :doc:`ClangNvlinkWrapper`.
-- ``clang-linker-wrapper`` tool introduced to support linking using a new 
OpenMP
-  target offloading method. See :doc:`ClangLinkerWrapper`.
-- Support for a new driver for OpenMP target offloading has been added as an
-  opt-in feature. The new driver can be selected using ``-fopenmp-new-driver``
-  with clang. Device-side LTO can also be enabled using the new driver by
-  passing ``-foffload-lto=`` as well. The new driver supports the following
-  features:
-  - Linking AMDGPU and NVPTX offloading targets.
-  - Static linking using archive files.
-  - Device-side LTO.
+...
 
 CUDA Support in Clang
 -



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

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D123831#3455048 , @cishida wrote:

>> we might not always want to transform an absolute path because the resulting 
>> relative include name might get remapped in a headermap, for example in test 
>> known_files_only_hmap.c. But how does it work with modules where we need 
>> relative includes? Is the setup in known_files_only_hmap even valid?
>
> I think, in most cases, this shouldn't matter because if the header path 
> input doesn't match the location stored in the header map, they should still 
> have the same source content. The same should be true with header search 
> resolution with modules & vfsoverlay

Agreed, I think it would be classified as a user error to remap to different 
source content via headermap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123831

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

[PATCH] D123488: [clangd] IncludeCleaner: Add filtering mechanism

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 423601.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Resolve comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123488

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1738,6 +1738,8 @@
 ]]
   #include "used.h"
 
+  #include "ignore.h"
+
   #include 
 
   void foo() {
@@ -1754,12 +1756,19 @@
 #pragma once
 void used() {}
   )cpp";
+  TU.AdditionalFiles["ignore.h"] = R"cpp(
+#pragma once
+void ignore() {}
+  )cpp";
   TU.AdditionalFiles["system/system_header.h"] = "";
   TU.ExtraArgs = {"-isystem" + testPath("system")};
   // Off by default.
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
   Config Cfg;
   Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  // Set filtering.
+  Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back(
+  [](llvm::StringRef Header) { return Header.endswith("ignore.h"); });
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -263,6 +263,24 @@
   EXPECT_TRUE(compileAndApply());
   EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
 Config::UnusedIncludesPolicy::Strict);
+
+  Frag = {};
+  EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
+  << Conf.Diagnostics.Includes.IgnoreHeader.size();
+  Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+  Located("foo.h"));
+  Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+  Located(".*inc"));
+  EXPECT_TRUE(compileAndApply());
+  auto HeaderFilter = [this](llvm::StringRef Path) {
+for (auto &Filter : Conf.Diagnostics.Includes.IgnoreHeader) {
+  if (Filter(Path))
+return true;
+}
+return false;
+  };
+  EXPECT_TRUE(HeaderFilter("foo.h"));
+  EXPECT_FALSE(HeaderFilter("bar.h"));
 }
 
 TEST_F(ConfigCompileTests, DiagnosticSuppression) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -23,10 +23,14 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -233,7 +237,8 @@
   }
 }
 
-static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST) {
+static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
+  const Config &Cfg) {
   if (Inc.BehindPragmaKeep)
 return false;
 
@@ -258,6 +263,15 @@
  FE->getName());
 return false;
   }
+  for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
+// Convert the path to Unix slashes and try to match aginast the fiilter.
+llvm::SmallString<64> Path(Inc.Resolved);
+llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+if (Filter(Inc.Resolved)) {
+  dlog("{0} header is filtered out by the configuration", FE->getName());
+  return false;
+}
+  }
   return true;
 }
 
@@ -369,6 +383,7 @@
   const llvm::DenseSet &ReferencedFiles,
   const llvm::StringSet<> &ReferencedPublicHeaders) {
   trace::Span Tracer("IncludeCleaner::getUnused");
+  const Config &Cfg = Config::current();
   std::vector Unused;
   for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
 if (!MFI.HeaderID)
@@ -377,7 +392,7 @@
   continue;
 auto IncludeID = static_cast(*MFI.HeaderID);
 bool Used = ReferencedFiles.contains(IncludeID);
-if (!Used && !mayConsiderUnused(MFI, AST)) {
+if (!Used && !mayConsiderUnused(MFI, AST, Cfg)) {
   dlog("{0} was not used, but is not eligible to be diagnosed as unused",
MFI.Written);
   continue;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/Confi

[clang-tools-extra] bdf0b75 - [clangd] IncludeCleaner: Add filtering mechanism

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2022-04-19T14:56:27+02:00
New Revision: bdf0b757d5938a9f774d17e81be226da3229d3e5

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

LOG: [clangd] IncludeCleaner: Add filtering mechanism

This introduces filtering out inclusions based on the resolved path. This
mechanism will be important for disabling warnings for headers that we can not
diagnose correctly yet.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index c693aee56ce57..734dce43c5873 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -29,6 +29,8 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Regex.h"
+#include 
 #include 
 #include 
 
@@ -100,6 +102,12 @@ struct Config {
 } ClangTidy;
 
 UnusedIncludesPolicy UnusedIncludes = None;
+
+/// IncludeCleaner will not diagnose usages of these headers matched by
+/// these regexes.
+struct {
+  std::vector> IgnoreHeader;
+} Includes;
   } Diagnostics;
 
   /// Style of the codebase.

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 4438a29d56089..a4d7904781e4f 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -45,7 +45,9 @@
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -432,6 +434,7 @@ struct FragmentCompiler {
 Out.Apply.push_back([Val](const Params &, Config &C) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
+compile(std::move(F.Includes));
 
 compile(std::move(F.ClangTidy));
   }
@@ -507,6 +510,41 @@ struct FragmentCompiler {
 }
   }
 
+  void compile(Fragment::DiagnosticsBlock::IncludesBlock &&F) {
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+auto Filters = std::make_shared>();
+for (auto &HeaderPattern : F.IgnoreHeader) {
+  // Anchor on the right.
+  std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+  llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+  std::string RegexError;
+  if (!CompiledRegex.isValid(RegexError)) {
+diag(Warning,
+ llvm::formatv("Invalid regular expression '{0}': {1}",
+   *HeaderPattern, RegexError)
+ .str(),
+ HeaderPattern.Range);
+continue;
+  }
+  Filters->push_back(std::move(CompiledRegex));
+}
+if (Filters->empty())
+  return;
+auto Filter = [Filters](llvm::StringRef Path) {
+  for (auto &Regex : *Filters)
+if (Regex.match(Path))
+  return true;
+  return false;
+};
+Out.Apply.push_back([Filter](const Params &, Config &C) {
+  C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter);
+});
+  }
+
   void compile(Fragment::CompletionBlock &&F) {
 if (F.AllScopes) {
   Out.Apply.push_back(

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 3c574bace2221..34bff844cb26c 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -232,6 +232,15 @@ struct Fragment {
 /// - None
 llvm::Optional> UnusedIncludes;
 
+/// Controls IncludeCleaner diagnostics.
+struct IncludesBlock {
+  /// Regexes that will be used to avoid diagnosing certain includes as
+  /// unused or missing. These can match any suffix of the header file in
+  /// question.
+  std::vector> IgnoreHeader;
+};
+IncludesBlock Includes;
+
 /// Controls how clang-tidy will run over the code base.
 ///
 /// The settings are merged with any settings found in .clang-tidy

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 1e6e3cac8a484..e6c8d9e76ed67 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -23,10 +23,14 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/T

[PATCH] D123488: [clangd] IncludeCleaner: Add filtering mechanism

[Kirill Bobyrev 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 rGbdf0b757d593: [clangd] IncludeCleaner: Add filtering 
mechanism (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123488

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1738,6 +1738,8 @@
 ]]
   #include "used.h"
 
+  #include "ignore.h"
+
   #include 
 
   void foo() {
@@ -1754,12 +1756,19 @@
 #pragma once
 void used() {}
   )cpp";
+  TU.AdditionalFiles["ignore.h"] = R"cpp(
+#pragma once
+void ignore() {}
+  )cpp";
   TU.AdditionalFiles["system/system_header.h"] = "";
   TU.ExtraArgs = {"-isystem" + testPath("system")};
   // Off by default.
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
   Config Cfg;
   Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  // Set filtering.
+  Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back(
+  [](llvm::StringRef Header) { return Header.endswith("ignore.h"); });
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -263,6 +263,24 @@
   EXPECT_TRUE(compileAndApply());
   EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
 Config::UnusedIncludesPolicy::Strict);
+
+  Frag = {};
+  EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
+  << Conf.Diagnostics.Includes.IgnoreHeader.size();
+  Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+  Located("foo.h"));
+  Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+  Located(".*inc"));
+  EXPECT_TRUE(compileAndApply());
+  auto HeaderFilter = [this](llvm::StringRef Path) {
+for (auto &Filter : Conf.Diagnostics.Includes.IgnoreHeader) {
+  if (Filter(Path))
+return true;
+}
+return false;
+  };
+  EXPECT_TRUE(HeaderFilter("foo.h"));
+  EXPECT_FALSE(HeaderFilter("bar.h"));
 }
 
 TEST_F(ConfigCompileTests, DiagnosticSuppression) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -23,10 +23,14 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -233,7 +237,8 @@
   }
 }
 
-static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST) {
+static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
+  const Config &Cfg) {
   if (Inc.BehindPragmaKeep)
 return false;
 
@@ -258,6 +263,15 @@
  FE->getName());
 return false;
   }
+  for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
+// Convert the path to Unix slashes and try to match aginast the fiilter.
+llvm::SmallString<64> Path(Inc.Resolved);
+llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+if (Filter(Inc.Resolved)) {
+  dlog("{0} header is filtered out by the configuration", FE->getName());
+  return false;
+}
+  }
   return true;
 }
 
@@ -369,6 +383,7 @@
   const llvm::DenseSet &ReferencedFiles,
   const llvm::StringSet<> &ReferencedPublicHeaders) {
   trace::Span Tracer("IncludeCleaner::getUnused");
+  const Config &Cfg = Config::current();
   std::vector Unused;
   for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
 if (!MFI.HeaderID)
@@ -377,7 +392,7 @@
   continue;
 auto IncludeID = static_cast(*MFI.HeaderID);
 bool Used = ReferencedFiles.contains(IncludeID);
-if (!Used && !mayConsiderUnused(MFI, AST)) {
+if (!Used && !mayConsiderUnused(MFI, AST, Cfg)) {
   dlog("{0} was not used, but is not eligible to be diagnosed as unused",
MFI.Written);
   continue;
Index: clang-tools-extra/clangd/ConfigFragment.h
=

[PATCH] D123961: [clang][dataflow] Do not crash on missing `Value` for struct-typed variable init.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:184-186
-  // FIXME: The initializer expression must always be assigned a value.
-  // Replace this with an assert when we have sufficient coverage of
-  // language features.

sgatev wrote:
> The patch makes sense to me in the current state, but it's unclear why is 
> this not something that we'd like to do in the long term.
Because of uninterpreted fields. If we consider those a temporary fix, then I 
agree about the long term. I'd thought those were here to stay.  That said, if 
we find a way to either interperet fields lazily or only interpret those needed 
in the current function scope, we may be able to avoid uninterpreted 
expressions.

This also begs the question as to why we insist on initializing the variable. 
Arguably, if the expresssion is uninterpreted, so should be the variable. So, 
we should at least explain why we're distinguishing. Thoughts on my adding a 
comment to that effect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123961

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I went to commit this, and found that a recently lit test now fails with a 
crash during constraint instantiation!  I'll be looking into that.  The example 
reduces to:

  template
  constexpr bool constraint = true;
   
  template 
  void dependent(U&& u) {
[]() requires constraint {}();
  }
   
  void test_dependent() {
int v   = 0;
dependent(v);
  }




Comment at: clang/lib/Sema/SemaTemplate.cpp:4705
   CheckConstraintSatisfaction(
-  NamedConcept, {NamedConcept->getConstraintExpr()}, Converted,
+  NamedConcept, {NamedConcept->getConstraintExpr()}, MLTAL,
   SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),

ChuanqiXu wrote:
> erichkeane wrote:
> > ChuanqiXu wrote:
> > > I would feel better if we could write:
> > > ```
> > > CheckConstraintSatisfaction(
> > >   NamedConcept, {NamedConcept->getConstraintExpr()}, {MLTAL},
> > >   SourceRange(SS.isSet() ? SS.getBeginLoc() : 
> > > ConceptNameInfo.getLoc(),
> > >   TemplateArgs->getRAngleLoc()),
> > >   Satisfaction)
> > > ```
> > > 
> > > But it looks unimplementable.
> > I'm not sure I get the suggestion?  Why would you want to put the 
> > `MultiLevelTemplateArgumentList` in curleys?  
> I just feel like the style is more cleaner. But I found the constructor might 
> not allow us to do so... So this one might not be a suggestion.
Ah, you mean to pass 'converted' directly in, so:
```
CheckConstraintSatisfaction(
  NamedConcept, {NamedConcept->getConstraintExpr()}, {Converted},
  SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
  TemplateArgs->getRAngleLoc()),
  Satisfaction)
```

(notice 'Converted' instead of MLTAL).  I agree with you, that WOULD be nicer, 
but unfortunately I think the constructor for that type was created explicitly 
to avoid us doing that :)


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

https://reviews.llvm.org/D119544

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks, LGTM! With minor revisions.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:749
+// E.g. char{1,3,5,127} -> uint{1,3,5,127}
+// Interrupt the first phase and go to second one when casted values start
+// go in descending order. That means that we crossed over the middle of





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687
+
+  if (IsConversion && (!IsPromotion || !What.isUnsigned()))
+return makePersistent(convertTo(What, Ty));

ASDenysPetrov wrote:
> martong wrote:
> > Could you please explain why do we need the `|| !What.isUnsigned()` part of 
> > the condition?
> > 
> > This would make much more sense to me:
> > ```
> > if (IsConversion && !IsPromotion)
> > return makePersistent(convertTo(What, Ty));
> > ```
> Look. Here we handle 2 cases:
> 
>   - `IsConversion && !IsPromotion`. In this case we handle changing a sign 
> with same bitwidth: char -> uchar, uint -> int. Here we convert //negatives 
> //to //positives //and //positives which is out of range// to //negatives//. 
> We use `convertTo` function for that.
>  
>   - `IsConversion && IsPromotion && !What.isUnsigned()`. In this case we 
> handle changing a sign from signeds to unsigneds with higher bitwidth: char 
> -> uint, int-> uint64. The point is that we also need convert //negatives 
> //to //positives //and use `convertTo` function as well. For example, we 
> don't need such a convertion when converting //unsigned //to //signed with 
> higher bitwidth//, because all the values of //unsigned //is valid for the 
> such //signed//.
>  
> 
> 
> 
Ok, makes sense. Could you please attach your explanation as a comment then 
into the code?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:690
+
+  // Promotion from unsigneds to signeds/unsigneds left.
+

ASDenysPetrov wrote:
> ASDenysPetrov wrote:
> > martong wrote:
> > > martong wrote:
> > > > I think it would be more consistent with the other operations (truncate 
> > > > and convert) if we had this logic in its own separate function: 
> > > > `promoteTo`. And this function should be called only if `isPromotion` 
> > > > is set (?)
> > > This comment is confusing, since we can get here also if
> > > `isConversion` is false and
> > > `isPromotion` is false 
> > Nothing confusing is here.
> > We have 7 main cases:
> > NOOP: **u -> u, s -> s**
> > Conversion: **u -> s, s -> u**
> > Truncation: **U-> u, S -> s**
> > Promotion: `u->U`, `s -> S`
> > Truncation + Conversion: **U -> s, S -> u**
> > Promotion + Conversion: **s -> U**, `u -> S`
> > 
> > As you can see, we've handled all the **bolds** above this line .
> > So only promotions from //unsigneds// left. That means, `isPromotion` never 
> > should be `false` here. We could add an assertion here if you want.
> That makes sense. I'll do.
> So only promotions from unsigneds left. That means, isPromotion never should 
> be false here. We could add an assertion here if you want.

Yeah, I think that would be useful, to enforce that the precondition for 
`promoteTo` holds all the time (even after future changes).



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:827
+  if (is_signed_v) {
+this->template checkCastTo({{MIN, MIN}, {MAX, MAX}}, {{MAX, MIN}});
+this->template checkCastTo({{MID, MID}, {MAX, MAX}}, {{MAX, MID}});

ASDenysPetrov wrote:
> martong wrote:
> > I am getting lost. Why don't you check against `ToMIN` and `ToMAX` here? 
> > Could you explain e.g. with int16->int8?
> > 
> > It is confusing that at many places you test against `MIN`, `MAX`, `A`, ... 
> > and the conversion is happening automatically by the `checkCastTo` 
> > template. Would it be more explicit to use everywhere `ToMIN`, `ToA`, 
> > `ToB`, ... and check against them?
> We can't use ToMIN, ToMAX, ... everywhere. That would be incorrect:
> **int16(-32768, 32767)** -> **int8(-128, 127)**, aka `(MIN, MAX) -> (ToMIN, 
> ToMAX) // OK`.
> **int16(-32768, -32768)** -> **int8(-128, -128)**, aka `(MIN, MIN) -> (ToMIN, 
> ToMIN) // NOK`.
> **int16(-32768,-32768)** -> **int8(0, 0)**, aka `(MIN, MIN) -> ((int8)MIN, 
> (int8)MIN) // OK`.
> 
Okay makes sense, thanks for the explanation. Nevertheless, I think this 
explanatory comment would be useful to have in the code.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:874-879
+  constexpr auto FromA = TV::FromA;
+  constexpr auto ToA = TV::ToA;
+  constexpr auto FromB = TV::FromB;
+  constexpr auto ToB = TV::ToB;
+  this->template checkCastTo({{FromA, ToA}, {FromB, ToB}},
+   {{FromA, ToA}});

ASDenysPetrov wrote:
> martong wrote:
> > Could you please elaborate what do we test here?
> E.g. 
> ```
> 

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

[Tom Ritter via Phabricator via cfe-commits]

tomrittervg added a comment.

Please commit on my behalf; you can use "Tom Ritter "


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123155

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

In D123498#3458357 , @DavidSpickett 
wrote:

> Unless I'm missing an existing check, is it possible to check for the 
> existing files too? I guess not nicely because along with the resource 
> headers there are lots of internal headers, so we'd have to maintain some 
> list of "non resource" headers which means we've just created another thing 
> to maintain.

This is a good point. Having such checks will be beneficial, but I concur that 
it is not easy to check for all the existing files given what we have now (the 
current way headers are organized etc) and I agree that we may end up 
maintaining more lists. Therefore I am keeping this patch in its current shape 
at this stage. I can come back and revisit once I find a clean way to check for 
the necessity of the header file. Maybe I can ask `cmake` to check for 
architecture/targets during configuration and select the headers automatically, 
but that is beyond the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> Therefore I am keeping this patch in its current shape at this stage.

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[clang] 218b5c8 - [clang][AArch64] Remove BTI after setjmp from release notes

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2022-04-19T13:49:55Z
New Revision: 218b5c83940d469424564ba6f1ec488c4970a5e5

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

LOG: [clang][AArch64] Remove BTI after setjmp from release notes

This is now going into 14.0.2 as
571c7d8f6dae1a8797ae3271c0c09fc648b1940b so will not be
new in clang-15.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ef4211204a905..4a0e2ff6170a2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -298,11 +298,6 @@ DWARF Support in Clang
 Arm and AArch64 Support in Clang
 
 
-- When using ``-mbranch-protection=bti`` with AArch64, calls to setjmp will
-  now be followed by a BTI instruction. This is done to be compatible with
-  setjmp implementations that return with a br instead of a ret. You can
-  disable this behaviour using the ``-mno-bti-at-return-twice`` option.
-
 Floating Point Support in Clang
 ---
 



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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The hard part about doing 2 separate cases is where we miss-interpret a .hpp or 
.h file as being the wrong language (C/C++/ObjC/ObjC++), I don't think I have a 
problem with a single regex (its not like we change it that often).

Is there a use case for having them separate or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[MyDeveloperDay via Phabricator via cfe-commits]

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

Accepting but please wait for @owenpan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D124004: Define __FLT_EVAL_METHOD__ when input source is stdin.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
zahiraam added reviewers: aaron.ballman, andrew.w.kaylor.
Herald added a subscriber: dschuff.
Herald added a project: All.
zahiraam requested review of this revision.
Herald added a subscriber: aheejin.
Herald added a project: clang.

When the input source is stdin, the __FLT_EVAL_METHOD__ macro is not  set. 
This patch defines it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124004

Files:
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Preprocessor/flt_eval_macro.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1571,6 +1571,7 @@
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
 // WEBASSEMBLY-NEXT:#define __FLT_EPSILON__ 1.19209290e-7F
+// WEBASSEMBLY-NEXT:#define __FLT_EVAL_METHOD__ 0
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_DENORM__ 1
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_INFINITY__ 1
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_QUIET_NAN__ 1
Index: clang/test/Preprocessor/flt_eval_macro.cpp
===
--- clang/test/Preprocessor/flt_eval_macro.cpp
+++ clang/test/Preprocessor/flt_eval_macro.cpp
@@ -23,6 +23,31 @@
 // RUN:   -target-feature -sse %s -o - | FileCheck -check-prefix=EXT %s \
 // RUN:   -strict-whitespace
 
+// RUN: %clang_cc1 -E -dM -triple=x86_64-none-none  < /dev/null \
+// RUN:   | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple=x86_64-none-none -target-feature -sse \
+// RUN:   < /dev/null | FileCheck %s  --check-prefix=CHECK-FEM-EXT
+
+// RUN: %clang_cc1 -E -dM -triple=arm64e-apple-ios -target-feature -sse \
+// RUN:   < /dev/null | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple=arm64e-apple-ios -target-feature +sse \
+// RUN:   < /dev/null | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple=arm64_32-apple-ios < /dev/null \
+// RUN:   | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple=arm64_32-apple-ios -target-feature -sse \
+// RUN:   < /dev/null | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 \
+// RUN:   < /dev/null | FileCheck %s  --check-prefix=CHECK-FEM
+
+// RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 \
+// RUN:   -target-feature -sse < /dev/null \
+// RUN:   | FileCheck %s  --check-prefix=CHECK-FEM-EXT
+
 #ifdef __FLT_EVAL_METHOD__
 #if __FLT_EVAL_METHOD__ == 3
 #define __GLIBC_FLT_EVAL_METHOD 2
@@ -80,3 +105,6 @@
   // EXT: #define Val "val2"
   return Val;
 }
+
+// CHECK-FEM: #define __FLT_EVAL_METHOD__ 0
+// CHECK-FEM-EXT: #define __FLT_EVAL_METHOD__ 2
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/DeclGroup.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Basic/MacroBuilder.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -840,6 +841,19 @@
 CI.getLangOpts().CurrentModule = CI.getLangOpts().ModuleName;
   }
 
+  // Set the __FLT_EVAL_METHOD__ when the input source is stdin.
+  if (Input.isFile() && Input.getFile() == "-") {
+Preprocessor &PP = CI.getPreprocessor();
+std::string PredefineBuffer;
+PredefineBuffer.reserve(4080);
+llvm::raw_string_ostream Predefines(PredefineBuffer);
+Predefines << PP.getPredefines();
+MacroBuilder Builder(Predefines);
+Builder.append("# 1 \"\" 3");
+Builder.defineMacro("__FLT_EVAL_METHOD__", Twine(PP.getTUFPEvalMethod()));
+PP.setPredefines(Predefines.str());
+  }
+
   if (!CI.InitializeSourceManager(Input))
 return false;
 


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1571,6 +1571,7 @@
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
 // WEBASSEMBLY-NEXT:#define __FLT_EPSILON__ 1.19209290e-7F
+// WEBASSEMBLY-NEXT:#define __FLT_EVAL_METHOD__ 0
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_DENORM__ 1
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_INFINITY__ 1
 // WEBASSEMBLY-NEXT:#define __FLT_HAS_QUIET_NAN__ 1
Index: clang/test/Preprocessor/flt_eval_macro.cpp
===
--- clang/test/Preprocessor/flt_eval_macro.cpp
+++ clang/test/Preprocessor/flt_eval_macro.cpp
@@ -23,6 +23,31 @@
 // RUN:   -target-feature -sse %s -o - | FileCheck -check-prefix=EXT %s \
 // RUN:   -strict-whitespace
 
+// RUN: %clang_cc1 -E -dM -triple=x86_

[clang] 2512a87 - [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[David Tenty via cfe-commits]

Author: Qiongsi Wu
Date: 2022-04-19T10:10:07-04:00
New Revision: 2512a875ccac158bc9b654b09e3347db167e33df

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

LOG: [clang] Adding Platform/Architecture Specific Resource Header Installation 
Targets

The goal of this patch is to improve distribution build's flexibility to 
include only applicable header files.

Currently, the clang-resource-headers target contains nearly all the files in 
clang/lib/Headers. Most of these files are platform specific (e.g. immintrin.h 
is x86 specific). A distribution build will have to either include all the 
headers for all the platforms, or not include any headers. For example, if a 
distribution build for powerpc includes the clang-resource-headers target, it 
will include all the x86 specific headers, even-though the x86 specific headers 
cannot be used.

This patch breaks up the clang-resource-headers list to a core list and 
platform specific lists. With the patch, a distribution build can now include 
the ppc-resource-headers to include the headers applicable to the powerpc 
platform.

Specifically, one can now have

cmake ... LLVM_DISTRIBUTION_COMPONENTS="clang;ppc-resource-headers" ... ../llvm
ninja install-distribution then installs the powerpc headers.

Similarly, one can do

cmake ... LLVM_DISTRIBUTION_COMPONENTS="clang;x86-resource-headers" ... ../llvm
to include headers applicable to the x86 platform in a distribution 
installation.

To implement this behaviour, the patch does two things:
* It breaks up the long files header file list to a core list and platform 
specific lists.
* It adds numerous platform specific installation targets.

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

Added: 


Modified: 
clang/lib/Headers/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index f1106b97bb382..e506ecb7a8c0e 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -1,19 +1,107 @@
-set(files
-  adxintrin.h
-  altivec.h
-  ammintrin.h
-  amxintrin.h
+# core_files list contains the headers shared by all platforms.
+# Please consider adding new platform specific headers
+# to platform specific lists below.
+set(core_files
+  builtins.h
+  float.h
+  inttypes.h
+  iso646.h
+  limits.h
+  module.modulemap
+  stdalign.h
+  stdarg.h
+  stdatomic.h
+  stdbool.h
+  stddef.h
+  __stddef_max_align_t.h
+  stdint.h
+  stdnoreturn.h
+  tgmath.h
+  unwind.h
+  varargs.h
+  )
+
+set(arm_common_files
+  # Headers shared by Arm and AArch64
   arm_acle.h
+  )
+
+set(arm_only_files
   arm_cmse.h
   armintr.h
+  )
+
+set(aarch64_only_files
   arm64intr.h
+  )
+
+set(cuda_files
+  __clang_cuda_builtin_vars.h
+  __clang_cuda_math.h
+  __clang_cuda_cmath.h
+  __clang_cuda_complex_builtins.h
+  __clang_cuda_device_functions.h
+  __clang_cuda_intrinsics.h
+  __clang_cuda_texture_intrinsics.h
+  __clang_cuda_libdevice_declares.h
+  __clang_cuda_math_forward_declares.h
+  __clang_cuda_runtime_wrapper.h
+  )
+
+set(hexagon_files
+  hexagon_circ_brev_intrinsics.h
+  hexagon_protos.h
+  hexagon_types.h
+  hvx_hexagon_protos.h
+  )
+
+set(hip_files
+  __clang_hip_libdevice_declares.h
+  __clang_hip_cmath.h
+  __clang_hip_math.h
+  __clang_hip_runtime_wrapper.h
+  )
+
+set(mips_msa_files
+  msa.h
+  )
+
+set(opencl_files
+  opencl-c.h
+  opencl-c-base.h
+  )
+
+set(ppc_files
+  altivec.h
+  htmintrin.h
+  htmxlintrin.h
+  )
+
+set(systemz_files
+  s390intrin.h
+  vecintrin.h
+  )
+
+set(ve_files
+  velintrin.h
+  velintrin_gen.h
+  velintrin_approx.h
+  )
+
+set(webassembly_files
+  wasm_simd128.h
+  )
+
+set(x86_files
+# Intrinsics
+  adxintrin.h
+  ammintrin.h
+  amxintrin.h
   avx2intrin.h
   avx512bf16intrin.h
-  avx512bwintrin.h
   avx512bitalgintrin.h
-  avx512vlbitalgintrin.h
+  avx512bwintrin.h
   avx512cdintrin.h
-  avx512vpopcntdqintrin.h
   avx512dqintrin.h
   avx512erintrin.h
   avx512fintrin.h
@@ -21,86 +109,55 @@ set(files
   avx512ifmaintrin.h
   avx512ifmavlintrin.h
   avx512pfintrin.h
+  avx512vbmi2intrin.h
   avx512vbmiintrin.h
   avx512vbmivlintrin.h
-  avx512vbmi2intrin.h
-  avx512vlvbmi2intrin.h
   avx512vlbf16intrin.h
+  avx512vlbitalgintrin.h
   avx512vlbwintrin.h
   avx512vlcdintrin.h
   avx512vldqintrin.h
   avx512vlfp16intrin.h
   avx512vlintrin.h
-  avx512vp2intersectintrin.h
+  avx512vlvbmi2intrin.h
+  avx512vlvnniintrin.h
   avx512vlvp2intersectintrin.h
-  avx512vpopcntdqvlintrin.h
   avx512vnniintrin.h
-  avx512vlvnniintrin.h
+  avx512vp2intersectintrin.h
+  avx512vpopcntdqintrin.h
+  avx512vpopcntdqvlintrin.h
   avxintrin.h
   avxvnniintrin.h
   bmi2intrin.h
   bmiintrin.h
-  builtins.h
-  __clang_cuda_builtin_vars.h
-  __clang_cuda_math.h
-  __clang_cuda_cmath.h
-  __clang_cuda_complex_bui

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[David Tenty 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 rG2512a875ccac: [clang] Adding Platform/Architecture Specific 
Resource Header Installation… (authored by qiongsiwu1, committed by daltenty).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

Files:
  clang/lib/Headers/CMakeLists.txt

Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -1,19 +1,107 @@
-set(files
-  adxintrin.h
-  altivec.h
-  ammintrin.h
-  amxintrin.h
+# core_files list contains the headers shared by all platforms.
+# Please consider adding new platform specific headers
+# to platform specific lists below.
+set(core_files
+  builtins.h
+  float.h
+  inttypes.h
+  iso646.h
+  limits.h
+  module.modulemap
+  stdalign.h
+  stdarg.h
+  stdatomic.h
+  stdbool.h
+  stddef.h
+  __stddef_max_align_t.h
+  stdint.h
+  stdnoreturn.h
+  tgmath.h
+  unwind.h
+  varargs.h
+  )
+
+set(arm_common_files
+  # Headers shared by Arm and AArch64
   arm_acle.h
+  )
+
+set(arm_only_files
   arm_cmse.h
   armintr.h
+  )
+
+set(aarch64_only_files
   arm64intr.h
+  )
+
+set(cuda_files
+  __clang_cuda_builtin_vars.h
+  __clang_cuda_math.h
+  __clang_cuda_cmath.h
+  __clang_cuda_complex_builtins.h
+  __clang_cuda_device_functions.h
+  __clang_cuda_intrinsics.h
+  __clang_cuda_texture_intrinsics.h
+  __clang_cuda_libdevice_declares.h
+  __clang_cuda_math_forward_declares.h
+  __clang_cuda_runtime_wrapper.h
+  )
+
+set(hexagon_files
+  hexagon_circ_brev_intrinsics.h
+  hexagon_protos.h
+  hexagon_types.h
+  hvx_hexagon_protos.h
+  )
+
+set(hip_files
+  __clang_hip_libdevice_declares.h
+  __clang_hip_cmath.h
+  __clang_hip_math.h
+  __clang_hip_runtime_wrapper.h
+  )
+
+set(mips_msa_files
+  msa.h
+  )
+
+set(opencl_files
+  opencl-c.h
+  opencl-c-base.h
+  )
+
+set(ppc_files
+  altivec.h
+  htmintrin.h
+  htmxlintrin.h
+  )
+
+set(systemz_files
+  s390intrin.h
+  vecintrin.h
+  )
+
+set(ve_files
+  velintrin.h
+  velintrin_gen.h
+  velintrin_approx.h
+  )
+
+set(webassembly_files
+  wasm_simd128.h
+  )
+
+set(x86_files
+# Intrinsics
+  adxintrin.h
+  ammintrin.h
+  amxintrin.h
   avx2intrin.h
   avx512bf16intrin.h
-  avx512bwintrin.h
   avx512bitalgintrin.h
-  avx512vlbitalgintrin.h
+  avx512bwintrin.h
   avx512cdintrin.h
-  avx512vpopcntdqintrin.h
   avx512dqintrin.h
   avx512erintrin.h
   avx512fintrin.h
@@ -21,86 +109,55 @@
   avx512ifmaintrin.h
   avx512ifmavlintrin.h
   avx512pfintrin.h
+  avx512vbmi2intrin.h
   avx512vbmiintrin.h
   avx512vbmivlintrin.h
-  avx512vbmi2intrin.h
-  avx512vlvbmi2intrin.h
   avx512vlbf16intrin.h
+  avx512vlbitalgintrin.h
   avx512vlbwintrin.h
   avx512vlcdintrin.h
   avx512vldqintrin.h
   avx512vlfp16intrin.h
   avx512vlintrin.h
-  avx512vp2intersectintrin.h
+  avx512vlvbmi2intrin.h
+  avx512vlvnniintrin.h
   avx512vlvp2intersectintrin.h
-  avx512vpopcntdqvlintrin.h
   avx512vnniintrin.h
-  avx512vlvnniintrin.h
+  avx512vp2intersectintrin.h
+  avx512vpopcntdqintrin.h
+  avx512vpopcntdqvlintrin.h
   avxintrin.h
   avxvnniintrin.h
   bmi2intrin.h
   bmiintrin.h
-  builtins.h
-  __clang_cuda_builtin_vars.h
-  __clang_cuda_math.h
-  __clang_cuda_cmath.h
-  __clang_cuda_complex_builtins.h
-  __clang_cuda_device_functions.h
-  __clang_cuda_intrinsics.h
-  __clang_cuda_texture_intrinsics.h
-  __clang_cuda_libdevice_declares.h
-  __clang_cuda_math_forward_declares.h
-  __clang_cuda_runtime_wrapper.h
-  __clang_hip_libdevice_declares.h
-  __clang_hip_cmath.h
-  __clang_hip_math.h
-  __clang_hip_runtime_wrapper.h
   cetintrin.h
-  cet.h
   cldemoteintrin.h
-  clzerointrin.h
-  crc32intrin.h
-  cpuid.h
   clflushoptintrin.h
   clwbintrin.h
+  clzerointrin.h
+  crc32intrin.h
   emmintrin.h
   enqcmdintrin.h
   f16cintrin.h
-  float.h
   fma4intrin.h
   fmaintrin.h
   fxsrintrin.h
   gfniintrin.h
-  hexagon_circ_brev_intrinsics.h
-  hexagon_protos.h
-  hexagon_types.h
-  hvx_hexagon_protos.h
   hresetintrin.h
-  htmintrin.h
-  htmxlintrin.h
   ia32intrin.h
   immintrin.h
-  intrin.h
-  inttypes.h
   invpcidintrin.h
-  iso646.h
   keylockerintrin.h
-  limits.h
   lwpintrin.h
   lzcntintrin.h
   mm3dnow.h
   mmintrin.h
-  mm_malloc.h
-  module.modulemap
   movdirintrin.h
-  msa.h
   mwaitxintrin.h
   nmmintrin.h
-  opencl-c.h
-  opencl-c-base.h
+  pconfigintrin.h
   pkuintrin.h
   pmmintrin.h
-  pconfigintrin.h
   popcntintrin.h
   prfchwintrin.h
   ptwriteintrin.h
@@ -108,33 +165,18 @@
   rtmintrin.h
   serializeintrin.h
   sgxintrin.h
-  s390intrin.h
   shaintrin.h
   smmintrin.h
-  stdalign.h
-  stdarg.h
-  stdatomic.h
-  stdbool.h
-  stddef.h
-  __stddef_max_align_t.h
-  stdint.h
-  stdnoreturn.h
   tbmintrin.h
-  tgmath.h
   tmmintrin.h
   tsxldtrkintrin.h
   uintrintrin.h
-  unwind.h
-  vadefs.h
   vaesintrin.h
-  varargs.h
-  v

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D119544#3458848 , @erichkeane 
wrote:

> I went to commit this, and found that a recently lit test now fails with a 
> crash during constraint instantiation!  I'll be looking into that.  The 
> example reduces to:
>
>   template
>   constexpr bool constraint = true;
>
>   template < typename U>
>   void dependent(U&& u) {
> []() requires constraint {}();
>   }
>
>   void test_dependent() {
> int v   = 0;
> dependent(v);
>   }

I suspect this may be related to my recent lambda changes - not 100% certain 
but I'm looking into it too


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

https://reviews.llvm.org/D119544

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> Maybe I can ask cmake to check for architecture/targets during configuration 
> and select the headers automatically, but that is beyond the scope of this 
> patch.

I'm not familar with cmake, but I guess it might be doable. I once verified the 
X86 headers by command `echo '#include ' | clang -x c -E - | grep 
'#.*clang.*h' | grep -o '[^\/]*\.h' |sort|uniq`.
Notice, targets like X86 doesn't have a single entry. But it still available 
with a bit more work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D119544#3458966 , @cor3ntin wrote:

> In D119544#3458848 , @erichkeane 
> wrote:
>
>> I went to commit this, and found that a recently lit test now fails with a 
>> crash during constraint instantiation!  I'll be looking into that.  The 
>> example reduces to:
>>
>>   template
>>   constexpr bool constraint = true;
>>
>>   template < typename U>
>>   void dependent(U&& u) {
>> []() requires constraint {}();
>>   }
>>
>>   void test_dependent() {
>> int v   = 0;
>> dependent(v);
>>   }
>
> I suspect this may be related to my recent lambda changes - not 100% certain 
> but I'm looking into it too

I've got it down to the `SetupConstraintCheckingTemplateArgumentsAndScope` that 
I added.  This is a case where the function is a template instantiation but 
does NOT have a primary template, so I have to figure out what THAT means/what 
I should be using instead.


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

https://reviews.llvm.org/D119544

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

[PATCH] D123300: [Clang] Enable opaque pointers by default

[Markus Lavin via Phabricator via cfe-commits]

markus added a comment.

With

  $ ./bin/clang -cc1 -emit-llvm BBI-68673.c

the IR contains the following

  @a2_ldm_i64 = global [4 x [18 x { i64, i64 }]] zeroinitializer, align 16
  %arrayidx = getelementptr inbounds [4 x [18 x { i64, i64 }]], ptr 
@a2_ldm_i64, i64 0, i64 %idxprom
  %.real = load volatile i64, ptr getelementptr inbounds ([18 x { i64, i64 }], 
ptr @a2_ldm_i64, i64 0, i64 1), align 16

but I would expect both GEPs to have the same first argument type (as the 
global symbol)

If I on the other hand compile with

  $ ./bin/clang -cc1 -emit-llvm -no-opaque-pointers BBI-68673.c

I instead get

  @a2_ldm_i64 = global [4 x [18 x { i64, i64 }]] zeroinitializer, align 16
  %arrayidx = getelementptr inbounds [4 x [18 x { i64, i64 }]], [4 x [18 x { 
i64, i64 }]]* @a2_ldm_i64, i64 0, i64 %idxprom
  %.real = load volatile i64, i64* getelementptr inbounds ([4 x [18 x { i64, 
i64 }]], [4 x [18 x { i64, i64 }]]* @a2_ldm_i64, i64 0, i64 0, i64 1, i32 0), 
align 16

which looks more consistent.

Any idea what is going on here?

F22817439: BBI-68673.c 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123300

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

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123992

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

[PATCH] D114292: [clang-tidy] Fix `altera-struct-pack-align` check for empty structs

[Aaron Ballman via Phabricator via cfe-commits]

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

Oh, yeah, this fell off my radar. Agreed, LGTM!


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

https://reviews.llvm.org/D114292

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

[PATCH] D123300: [Clang] Enable opaque pointers by default

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@markus Without tracing through it in detail, I'd guess that without opaque 
pointers this creates two getelementptr constant expressions that get folded 
together. With opaque pointers, the first one (which would be a zero-index GEP) 
is omitted, and only the second one is left. ConstantFolding will later 
canonicalize the GEP source type, but this only happens when InstCombine runs, 
while you're looking at unoptimized IR.

Are you encountering some particular issue relating to this? For well-written 
optimizations, the exact GEP representation shouldn't matter either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123300

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

[clang] 82f3ed9 - [analyzer] Expose Taint.h to plugins

[Balazs Benics via cfe-commits]

Author: Tom Ritter
Date: 2022-04-19T16:55:01+02:00
New Revision: 82f3ed99045de9a2f06981621a485c5a47546270

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

LOG: [analyzer] Expose Taint.h to plugins

Reviewed By: NoQ, xazax.hun, steakhal

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

Added: 
clang/include/clang/StaticAnalyzer/Checkers/Taint.h

Modified: 
clang/docs/tools/clang-formatted-files.txt
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/lib/StaticAnalyzer/Checkers/Taint.cpp
clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

Removed: 
clang/lib/StaticAnalyzer/Checkers/Taint.h



diff  --git a/clang/docs/tools/clang-formatted-files.txt 
b/clang/docs/tools/clang-formatted-files.txt
index e344c8faa7da2..61d4b71586201 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -530,7 +530,6 @@ 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
 clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp
 clang/lib/StaticAnalyzer/Checkers/Taint.cpp
-clang/lib/StaticAnalyzer/Checkers/Taint.h
 clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
 clang/lib/StaticAnalyzer/Checkers/Yaml.h
 clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp

diff  --git a/clang/lib/StaticAnalyzer/Checkers/Taint.h 
b/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
similarity index 100%
rename from clang/lib/StaticAnalyzer/Checkers/Taint.h
rename to clang/include/clang/StaticAnalyzer/Checkers/Taint.h

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 2a5fe9d8ed926..ba15abbdca718 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -11,9 +11,9 @@
 //
 
//===--===//
 
-#include "Taint.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"

diff  --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
index 2b3164ba4a2c8..bfd1d4c162ec3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
@@ -11,8 +11,8 @@
 //
 
//===--===//
 
-#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 2ce1bef6d228d..9253e9e6abfee 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -6,10 +6,10 @@
 //
 
//===--===//
 
-#include "Taint.h"
 #include "clang/Analysis/IssueHash.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"

diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 518a4eb4d7759..7e237b471def5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -14,11 +14,11 @@
 //
 
//===--===//
 
-#include "Taint.h"
 #include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f3ed99045d: [analyzer] Expose Taint.h to plugins (authored 
by tomrittervg, committed by steakhal).

Changed prior to commit:
  https://reviews.llvm.org/D123155?vs=420616&id=423622#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123155

Files:
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -13,9 +13,9 @@
 //
 //===--===//
 
-#include "Taint.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
Index: clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
@@ -10,8 +10,8 @@
 //
 //===--===//
 
-#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
Index: clang/lib/StaticAnalyzer/Checkers/Taint.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/Taint.cpp
+++ clang/lib/StaticAnalyzer/Checkers/Taint.cpp
@@ -10,7 +10,7 @@
 //
 //===--===//
 
-#include "Taint.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -14,11 +14,11 @@
 //
 //===--===//
 
-#include "Taint.h"
 #include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -6,10 +6,10 @@
 //
 //===--===//
 
-#include "Taint.h"
 #include "clang/Analysis/IssueHash.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
Index: clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
@@ -11,8 +11,8 @@
 //
 //===--===//
 
-#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/Static

[clang] 4aa5dc1 - [SystemZ] Handle SystemZ specific inline assembly address operands.

[Jonas Paulsson via cfe-commits]

Author: Jonas Paulsson
Date: 2022-04-19T16:55:45+02:00
New Revision: 4aa5dc15f0869f8a5fb6fe760c517d2d5d4c710e

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

LOG: [SystemZ] Handle SystemZ specific inline assembly address operands.

Handle ZQ, ZR, ZS and ZT inline assembly operand constraints.

Review: Ulrich Weigand

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

Added: 


Modified: 
clang/lib/Basic/Targets/SystemZ.cpp
clang/lib/Basic/Targets/SystemZ.h
clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c
llvm/include/llvm/IR/InlineAsm.h
llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
llvm/lib/Target/SystemZ/SystemZISelLowering.h
llvm/test/CodeGen/SystemZ/inline-asm-addr.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/SystemZ.cpp 
b/clang/lib/Basic/Targets/SystemZ.cpp
index e3e0da21f8d5f..3af9216315132 100644
--- a/clang/lib/Basic/Targets/SystemZ.cpp
+++ b/clang/lib/Basic/Targets/SystemZ.cpp
@@ -59,6 +59,17 @@ bool SystemZTargetInfo::validateAsmConstraint(
   default:
 return false;
 
+  case 'Z':
+switch (Name[1]) {
+default:
+  return false;
+case 'Q': // Address with base and unsigned 12-bit displacement
+case 'R': // Likewise, plus an index
+case 'S': // Address with base and signed 20-bit displacement
+case 'T': // Likewise, plus an index
+  break;
+}
+LLVM_FALLTHROUGH;
   case 'a': // Address register
   case 'd': // Data register (equivalent to 'r')
   case 'f': // Floating-point register

diff  --git a/clang/lib/Basic/Targets/SystemZ.h 
b/clang/lib/Basic/Targets/SystemZ.h
index f0306642a6656..d12045c756c1f 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -86,6 +86,20 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public 
TargetInfo {
 switch (Constraint[0]) {
 case 'p': // Keep 'p' constraint.
   return std::string("p");
+case 'Z':
+  switch (Constraint[1]) {
+  case 'Q': // Address with base and unsigned 12-bit displacement
+  case 'R': // Likewise, plus an index
+  case 'S': // Address with base and signed 20-bit displacement
+  case 'T': // Likewise, plus an index
+// "^" hints llvm that this is a 2 letter constraint.
+// "Constraint++" is used to promote the string iterator
+// to the next constraint.
+return std::string("^") + std::string(Constraint++, 2);
+  default:
+break;
+  }
+  break;
 default:
   break;
 }

diff  --git a/clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c 
b/clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c
index 3157d0ef62416..fd91abcc75954 100644
--- a/clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c
+++ b/clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c
@@ -6,6 +6,34 @@ long *A;
 long Idx;
 unsigned long Addr;
 
+unsigned long fun_BD12_Q() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BD12_Q()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZQ"(i64* nonnull %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZQ" (&A[100]));
+  return Addr;
+}
+
+unsigned long fun_BD12_R() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BD12_R()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZR"(i64* nonnull %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZR" (&A[100]));
+  return Addr;
+}
+
+unsigned long fun_BD12_S() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BD12_S()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZS"(i64* nonnull %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZS" (&A[100]));
+  return Addr;
+}
+
+unsigned long fun_BD12_T() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BD12_T()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZT"(i64* nonnull %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZT" (&A[100]));
+  return Addr;
+}
+
 unsigned long fun_BD12_p() {
 // CHECK-LABEL: define{{.*}} i64 @fun_BD12_p()
 // CHECK: call i64 asm "lay $0, $1", "=r,p"(i64* nonnull %arrayidx)
@@ -13,6 +41,34 @@ unsigned long fun_BD12_p() {
   return Addr;
 }
 
+unsigned long fun_BDX12_Q() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BDX12_Q()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZQ"(i64* %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZQ" (&A[Idx + 100]));
+  return Addr;
+}
+
+unsigned long fun_BDX12_R() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BDX12_R()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZR"(i64* %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZR" (&A[Idx + 100]));
+  return Addr;
+}
+
+unsigned long fun_BDX12_S() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BDX12_S()
+// CHECK: call i64 asm "lay $0, $1", "=r,^ZS"(i64* %arrayidx)
+  asm("lay %0, %1" : "=r" (Addr) : "ZS" (&A[Idx + 100]));
+  return Addr;
+}
+
+unsigned long fun_BDX12_T() {
+// CHECK-LABEL: define{{.*}} i64 @fun_BDX12_T()
+// CHECK: call i64 asm

[PATCH] D110267: [InlineAsm, SystemZ] Handle inline assembly address operands.

[Jonas Paulsson 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 rG4aa5dc15f086: [SystemZ] Handle SystemZ specific inline 
assembly address operands. (authored by jonpa).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110267

Files:
  clang/lib/Basic/Targets/SystemZ.cpp
  clang/lib/Basic/Targets/SystemZ.h
  clang/test/CodeGen/SystemZ/systemz-inline-asm-03.c
  llvm/include/llvm/IR/InlineAsm.h
  llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.h
  llvm/test/CodeGen/SystemZ/inline-asm-addr.ll

Index: llvm/test/CodeGen/SystemZ/inline-asm-addr.ll
===
--- llvm/test/CodeGen/SystemZ/inline-asm-addr.ll
+++ llvm/test/CodeGen/SystemZ/inline-asm-addr.ll
@@ -4,6 +4,54 @@
 @A = global i64* null, align 8
 @Idx = global i64 0, align 8
 
+define i64 @fun_BD12_Q() {
+; CHECK-LABEL: fun_BD12_Q:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 100
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZQ"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
+define i64 @fun_BD12_R() {
+; CHECK-LABEL: fun_BD12_R:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 100
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZR"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
+define i64 @fun_BD12_S() {
+; CHECK-LABEL: fun_BD12_S:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 100
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZS"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
+define i64 @fun_BD12_T() {
+; CHECK-LABEL: fun_BD12_T:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 100
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZT"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
 define i64 @fun_BD12_p() {
 ; CHECK-LABEL: fun_BD12_p:
 ; CHECK: #APP
@@ -16,6 +64,62 @@
   ret i64 %1
 }
 
+define i64 @fun_BDX12_Q() {
+; CHECK-LABEL: fun_BDX12_Q:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %1 = load i64, i64* @Idx
+  %add = add nsw i64 %1, 100
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 %add
+  %2 = tail call i64 asm "lay $0, $1", "=r,^ZQ"(i64* %arrayidx)
+  store i64 %2, i64* @Addr
+  ret i64 %2
+}
+
+define i64 @fun_BDX12_R() {
+; CHECK-LABEL: fun_BDX12_R:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1,%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %1 = load i64, i64* @Idx
+  %add = add nsw i64 %1, 100
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 %add
+  %2 = tail call i64 asm "lay $0, $1", "=r,^ZR"(i64* %arrayidx)
+  store i64 %2, i64* @Addr
+  ret i64 %2
+}
+
+define i64 @fun_BDX12_S() {
+; CHECK-LABEL: fun_BDX12_S:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %1 = load i64, i64* @Idx
+  %add = add nsw i64 %1, 100
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 %add
+  %2 = tail call i64 asm "lay $0, $1", "=r,^ZS"(i64* %arrayidx)
+  store i64 %2, i64* @Addr
+  ret i64 %2
+}
+
+define i64 @fun_BDX12_T() {
+; CHECK-LABEL: fun_BDX12_T:
+; CHECK: #APP
+; CHECK: lay	%r2, 800(%r1,%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %1 = load i64, i64* @Idx
+  %add = add nsw i64 %1, 100
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 %add
+  %2 = tail call i64 asm "lay $0, $1", "=r,^ZT"(i64* %arrayidx)
+  store i64 %2, i64* @Addr
+  ret i64 %2
+}
+
 define i64 @fun_BDX12_p() {
 ; CHECK-LABEL: fun_BDX12_p:
 ; CHECK: #APP
@@ -30,6 +134,54 @@
   ret i64 %2
 }
 
+define i64 @fun_BD20_Q() {
+; CHECK-LABEL: fun_BD20_Q:
+; CHECK: #APP
+; CHECK: lay	%r2, 0(%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 1000
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZQ"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
+define i64 @fun_BD20_R() {
+; CHECK-LABEL: fun_BD20_R:
+; CHECK: #APP
+; CHECK: lay	%r2, 0(%r2)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 1000
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZR"(i64* nonnull %arrayidx)
+  store i64 %1, i64* @Addr
+  ret i64 %1
+}
+
+define i64 @fun_BD20_S() {
+; CHECK-LABEL: fun_BD20_S:
+; CHECK: #APP
+; CHECK: lay	%r2, 8000(%r1)
+entry:
+  %0 = load i64*, i64** @A
+  %arrayidx = getelementptr inbounds i64, i64* %0, i64 1000
+  %1 = tail call i64 asm "lay $0, $1", "=r,^ZS"(i64* nonnull %arrayidx)
+  store i6

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> This is a case where the function is a template instantiation but does NOT 
> have a primary template, so I have to figure out what THAT means/what I 
> should be using instead.

I think that is not supposed to be possible. For example, 
`FunctionDecl::isFunctionTemplateSpecialization()` will return `false` if there 
is no primary template.


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

https://reviews.llvm.org/D119544

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D119544#3459045 , @tahonermann 
wrote:

>> This is a case where the function is a template instantiation but does NOT 
>> have a primary template, so I have to figure out what THAT means/what I 
>> should be using instead.
>
> I think that is not supposed to be possible. For example, 
> `FunctionDecl::isFunctionTemplateSpecialization()` will return `false` if 
> there is no primary template.

I wouldn't think so either?  In this case the problem is that 'u' is not in the 
re-manufactured scope, I think there is a bit of work to make sure that lambdas 
ALSO get the scope of their containing function, if they are in a functiondecl.


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

https://reviews.llvm.org/D119544

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Gabor Marton via Phabricator via cfe-commits]

martong added a subscriber: vabridgers.
martong added a comment.

First of all, thanks Denys for working on this, nice work! Here are my concerns 
and remarks.

I think this fixed set of types in `NominalTypeList` is too rigid. I don't like 
the fact that we have to iterate over all four types whenever we set a new 
constraint or when we try to infer. Also, I am thinking about downstream 
hardware architectures, where there might be integers with different bit-widths 
(@vabridgers). Also, at some point people will pursue us to support integers 
with arbitrary bitwidth (see _ExtInt 
)

Thus, I am proposing an alternative approach. We should have a SymExpr -> Set 
of SymExpr mapping in the State that represents the relation of symbols that 
are connected via some cast operations (see REGISTER_MAP_WITH_PROGRAMSTATE). 
Let's call this mapping as `CastMap`. The key should be the root symbol, i.e 
the symbol that is being declared first before all cast operations.

E.g.  Let's have

  int16 a = 128;

then we have a constraint [128,128] stored for `$a`. Then

  if ((int8)a < 0)

creates a new symbol `$a2` (SymbolCast) that has a new constraint [-128,-128] 
assigned to it. And we also keep track in the State, that `$a` and `$a2` refers 
the same root symbol (`a`). We now have in the CastMap $a -> [$a2].

Now, let's say we have

  if ((_ExtInt(7))a > 64)

then we can dig up the existing contraints from CastMap to check for the 
State's validity and we can update all the constraints of $a and $a2 as needed. 
Also, CastMap is updated: $a -> [$a2, $a3].




Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:421-426
+  // Unwrap symbolic expression to skip argument casts on function call.
+  // This is useful when there is no way for overloading function in C
+  // but we need to pass different types of arguments and
+  // implicit cast occures.
+  Sym = Sym->ignoreCasts();
+

Does it really matter? I mean, why do we need this change?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2212
+
+bool ConstraintAssignor::assignSymExprToRangeSet(const SymExpr *Sym,
+ RangeSet Constraint) {

Could you please rebase? The "simplification" code part had been merged already 
to llvm/main and it is not part of this change.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2227-2251
+  llvm::SmallSet SimplifiedClasses;
+  // Iterate over all equivalence classes and try to simplify them.
+  ClassMembersTy Members = State->get();
+  for (std::pair ClassToSymbolSet : Members) {
+EquivalenceClass Class = ClassToSymbolSet.first;
+State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class);
+if (!State)

I think this hunk should remain in `assignSymExprToConst`. Why did you move it?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2280-2290
+/// Return a symbol which is the best canidate to save it in the constraint
+/// map. We should correct symbol because in case of truncation cast we can
+/// only reason about truncated bytes but not the whole value. E.g. (char)(int
+/// x), we can store constraints for the first lower byte but we still don't
+/// know the root value. Also in case of promotion or converion we should
+/// store the root value instead of cast symbol, because we can always get
+/// a correct range using `castTo` metho. And we are not intrested in any

I think, we should definitely store the constraints as they appear in the 
analyzed code. This way we mix the infer logic into the constraint setting, 
which is bad.
I mean, we should simply store the constraint directly for the symbol as it is. 
And then only in `VisitSymbolCast` should we infer the proper value from the 
stored constraint (if we can).

(Of course, if we have related symbols (casts of the original symbol) then 
their constraints must be updated.)



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2281
+/// Return a symbol which is the best canidate to save it in the constraint
+/// map. We should correct symbol because in case of truncation cast we can
+/// only reason about truncated bytes but not the whole value. E.g. (char)(int





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2284
+/// x), we can store constraints for the first lower byte but we still don't
+/// know the root value. Also in case of promotion or converion we should
+/// store the root value instead of cast symbol, because we can always get





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2285
+/// know the root value. Also in case of promotion or converion we should
+/// store the ro

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

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

steakhal updated this revision to Diff 423624.
steakhal edited the summary of this revision.
steakhal added a comment.
Herald added a subscriber: manas.
Herald added a project: All.

rebased; I'm still interested in this. WDYT @martong
BTW we have this downstream for about two years now without any problems.


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

https://reviews.llvm.org/D85528

Files:
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/z3-refute-enum-crash.cpp


Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -verify %s
+//
+// REQUIRES: z3
+//
+// Requires z3 only for refutation. Works with both constraint managers.
+
+void clang_analyzer_dump(int);
+
+using sugar_t = unsigned char;
+
+// Enum types
+enum class ScopedSugared : sugar_t {};
+enum class ScopedPrimitive : unsigned char {};
+enum UnscopedSugared : sugar_t {};
+enum UnscopedPrimitive : unsigned char {};
+
+template 
+T conjure();
+
+void test_enum_types() {
+  int sym1 = static_cast(conjure()) & 0x0F;
+  int sym2 = static_cast(conjure()) & 0x0F;
+  int sym3 = static_cast(conjure()) & 0x0F;
+  int sym4 = static_cast(conjure()) & 0x0F;
+
+  if (sym1 && sym2 && sym3 && sym4) {
+// no-crash on these dumps
+clang_analyzer_dump(sym1); // expected-warning{{((unsigned char) (conj_}}
+clang_analyzer_dump(sym2); // expected-warning{{((unsigned char) (conj_}}
+clang_analyzer_dump(sym3); // expected-warning{{(conj_}}
+clang_analyzer_dump(sym4); // expected-warning{{(conj_}}
+  }
+}
+
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -983,8 +983,8 @@
 
 // Produce SymbolCast if CastTy and T are different integers.
 // NOTE: In the end the type of SymbolCast shall be equal to CastTy.
-if (T->isIntegralOrEnumerationType() &&
-CastTy->isIntegralOrEnumerationType()) {
+if (T->isIntegralOrUnscopedEnumerationType() &&
+CastTy->isIntegralOrUnscopedEnumerationType()) {
   AnalyzerOptions &Opts =
   StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
   // If appropriate option is disabled, ignore the cast.


Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -verify %s
+//
+// REQUIRES: z3
+//
+// Requires z3 only for refutation. Works with both constraint managers.
+
+void clang_analyzer_dump(int);
+
+using sugar_t = unsigned char;
+
+// Enum types
+enum class ScopedSugared : sugar_t {};
+enum class ScopedPrimitive : unsigned char {};
+enum UnscopedSugared : sugar_t {};
+enum UnscopedPrimitive : unsigned char {};
+
+template 
+T conjure();
+
+void test_enum_types() {
+  int sym1 = static_cast(conjure()) & 0x0F;
+  int sym2 = static_cast(conjure()) & 0x0F;
+  int sym3 = static_cast(conjure()) & 0x0F;
+  int sym4 = static_cast(conjure()) & 0x0F;
+
+  if (sym1 && sym2 && sym3 && sym4) {
+// no-crash on these dumps
+clang_analyzer_dump(sym1); // expected-warning{{((unsigned char) (conj_}}
+clang_analyzer_dump(sym2); // expected-warning{{((unsigned char) (conj_}}
+clang_analyzer_dump(sym3); // expected-warning{{(conj_}}
+clang_analyzer_dump(sym4); // expected-warning{{(conj_}}
+  }
+}
+
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -983,8 +983,8 @@
 
 // Produce SymbolCast if CastTy and T are different integers.
 // NOTE: In the end the type of SymbolCast shall be equal to CastTy.
-if (T->isIntegralOrEnumerationType() &&
-CastTy->isIntegralOrEnumerationType()) {
+if (T->isIntegralOrUnscopedEnumerationType() &&
+CastTy->isIntegralOrUnscopedEnumerationType()) {
   AnalyzerOptions &Opts =
   StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
   // If appropriate option is disabled, ignore the cast.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123955: [C2x] Disallow functions without prototypes/functions with identifier lists

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 423629.
aaron.ballman marked an inline comment as not done.
aaron.ballman added a comment.

Updated based on review feedback.

- Added `-fstrict-prototypes` command line option (but there is NO 
`-fno-strict-prototypes` support)
- Changed the way the language option works to be a little less fragile
- Fixed the static analyzer to create a function with a prototype instead


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

https://reviews.llvm.org/D123955

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Driver/strict-prototypes.c
  clang/test/Frontend/strict-prototypes.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-func-prototype.c
  clang/test/Sema/attr-c2x.c
  clang/test/Sema/c2x-func-prototype.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -725,7 +725,7 @@
 
   Remove support for function definitions with identifier lists
   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2432.pdf";>N2432
-  No
+  Clang 15
 
 
 
Index: clang/test/Sema/c2x-func-prototype.c
===
--- /dev/null
+++ clang/test/Sema/c2x-func-prototype.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=c2x -std=c2x %s
+// RUN: %clang_cc1 -Wno-strict-prototypes -fsyntax-only -verify -std=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -fstrict-prototypes -std=c99 -verify=c2x %s
+// expected-no-diagnostics
+
+void func(); // c2x-note {{'func' declared here}}
+typedef void (*fp)();
+
+void other_func(int i);
+
+void call(void) {
+  func(1, 2, 3); // c2x-error {{too many arguments to function call, expected 0, have 3}}
+  fp call_me = func;
+  call_me(1, 2, 3); // c2x-error {{too many arguments to function call, expected 0, have 3}}
+
+  fp nope = other_func; // c2x-warning {{incompatible function pointer types initializing 'fp' (aka 'void (*)(void)') with an expression of type 'void (int)'}}
+}
+
+// Ensure these function declarations do not merge in C2x.
+void redecl1();  // c2x-note {{previous declaration is here}}
+void redecl1(int i); // c2x-error {{conflicting types for 'redecl1'}}
+
+void redecl2(int i); // c2x-note {{previous declaration is here}}
+void redecl2();  // c2x-error {{conflicting types for 'redecl2'}}
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -20,7 +20,7 @@
 void context_async_okay2(void *context [[clang::swift_async_context]], void *selfType, char **selfWitnessTable) [[clang::swiftasynccall]];
 
 [[clang::ownership_returns(foo)]] void *f1(void);
-[[clang::ownership_returns(foo)]] void *f2(); // expected-warning {{'ownership_returns' attribute only applies to non-K&R-style functions}}
+[[clang::ownership_returns(foo)]] void *f2();
 
 [[clang::unavailable("not available - replaced")]] void foo2(void); // expected-note {{'foo2' has been explicitly marked unavailable here}}
 void bar(void) {
Index: clang/test/Parser/c2x-func-prototype.c
===
--- /dev/null
+++ clang/test/Parser/c2x-func-prototype.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=c2x -std=c2x %s
+// RUN: %clang_cc1 -Wno-strict-prototypes -fsyntax-only -verify -std=c17 %s
+// expected-no-diagnostics
+
+// Functions with an identifier list are not supported in C2x.
+void ident_list(a) // c2x-error {{expected ';' after top level declarator}} \
+  c2x-warning {{type specifier missing, defaults to 'int'}}
+  int a;
+{} // c2x-error {{expected identifier or '('}}
+
+// Functions with an empty parameter list are supported as though the function
+// was declared with a parameter list of (void). Ensure they still parse.
+void no_param_decl();
+void no_param_defn() {}
+void (*var_of_type_with_no_param)();
+typedef void fn();
Index: clang/test/Parser/c2x-attributes.c
===
--- clang/test/Parser/c2x-attributes.c
+++ clang/test/Parser/c2x-attributes.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -fdouble-square-bracket-attributes -verify -Wno-strict-prototypes %s
-// RUN: %clang_cc1 -fsyntax-only -std=gnu2x -verify -Wno-strict-prototypes %s
+// RUN: %clang_cc1 -fsyntax-only -fdouble-square-bracket-attributes -verify=expected,notc2x -Wno-strict-prototypes %s
+// RUN: %clang_cc1 -fsyntax-only -std=gnu2x -verify=expected,c2x %s
 
 enum [[]] E {
   One [[]],
@@ -59,7 +59,10 @@

[PATCH] D123955: [C2x] Disallow functions without prototypes/functions with identifier lists

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 5 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:124
 BENIGN_LANGOPT(ImplicitInt, 1, 0, "C89 implicit 'int'")
+LANGOPT(StrictPrototypes  , 1, 0, "require function types to have a prototype")
 LANGOPT(Digraphs  , 1, 0, "digraphs")

aaron.ballman wrote:
> aaron.ballman wrote:
> > rsmith wrote:
> > > This makes me think we should have some declarative way of specifying 
> > > dependencies between `LANGOPT`s. It's presumably sufficiently obvious to 
> > > a library user that they shouldn't enable (eg) `CPlusPlus20` unless they 
> > > enable all the previous `CPlusPlusXY` modes and `CPlusPlus`, but I doubt 
> > > it's obvious that enabling `CPlusPlus` requires also enabling 
> > > `StrictPrototypes`.
> > > 
> > > In fact, after this change, I think a lot of existing library users of 
> > > Clang that invent their own `LangOptions` will silently start getting 
> > > this wrong. That's concerning. Maybe we should consider prototypes to be 
> > > required if either `StrictPrototypes` or `CPlusPlus` is enabled?
> > > This makes me think we should have some declarative way of specifying 
> > > dependencies between LANGOPTs.
> > 
> > Tee hee: 
> > https://github.com/llvm/llvm-project/commit/be9371659380388a693ec99624e1f3d02f07047f
> > 
> > I was caught by the same issues that I ended up fixing in that commit -- I 
> > tried making `StrictPrototypes` dependent and was very surprised when it 
> > doesn't work. The issue was 
> > `Invocation->getLangOpts()->resetNonModularOptions();` in 
> > `compileModuleImpl()` IIRC -- it would clear all of the language options 
> > (including ones like digraphs and wchar support).
> > 
> > > In fact, after this change, I think a lot of existing library users of 
> > > Clang that invent their own LangOptions will silently start getting this 
> > > wrong. That's concerning. Maybe we should consider prototypes to be 
> > > required if either StrictPrototypes or CPlusPlus is enabled?
> > 
> > @cor3ntin also shared this same concern -- my goal with this language 
> > option was:
> > 
> > * Make it easy to reenable functions without prototypes in C2x mode if we 
> > find some major breakage in the wild (hopefully we don't)
> > * Make it easy to add `-fstrict-prototypes` as a language flag to opt 
> > *into* strict prototypes (there would be no option to opt *out* of strict 
> > prototypes).
> > * Stop repeating `CPlusPlus || C2x` in a bunch of places and give it a 
> > named option.
> > 
> > So I'm not keen on adding `StrictPrototypes || CPlusPlus` everywhere -- C++ 
> > requires strict prototypes. However, the fears are still valid -- what if I 
> > found someplace nice to add an assert that `StrictPrototypes` cannot be 
> > false when `CPlusPlus` is true?
> >> This makes me think we should have some declarative way of specifying 
> >> dependencies between LANGOPTs.
> > Tee hee: 
> > https://github.com/llvm/llvm-project/commit/be9371659380388a693ec99624e1f3d02f07047f
> >
> > I was caught by the same issues that I ended up fixing in that commit -- I 
> > tried making StrictPrototypes dependent and was very surprised when it 
> > doesn't work. The issue was 
> > Invocation->getLangOpts()->resetNonModularOptions(); in compileModuleImpl() 
> > IIRC -- it would clear all of the language options (including ones like 
> > digraphs and wchar support).
> 
> There's actually a secondary issue, which is that users can set language 
> options directly -- there's not a setter function for them. This means that 
> places where we create a `LangOptions` object and manually set it up (like 
> our unit tests) have no way to automatically set `StrictPrototypes` when 
> setting other language options.
> 
> However, this speaks to the importance of having an assert to ensure that by 
> the time the compiler instance is invoked, we have language options that are 
> correct (on the assumption that later stages will not be modifying the 
> language options particularly often).
I ended up reworking this so that the language option is only for the command 
line extension, and there's a helper method to ask whether strict prototypes is 
enabled (based on the values of the language options). It's still a little 
fragile (someone could use the language option instead of the helper method), 
but I think that's mitigated by the name of the language option.



Comment at: clang/lib/Parse/ParseDecl.cpp:6664-
+// OpenCL disallows variadic functions, so it also disallows a function
+// without a prototype. However, it doesn't enforce strict prototypes
+// because it allows function definitions with an identifier list.

aaron.ballman wrote:
> rsmith wrote:
> > I don't follow this comment: functions without a prototype are not variadic 
> > (they're compatible with any *non-variadic* prototype), so OpenCL 
> > disallowing variadic functions seems

[PATCH] D124006: [DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.

[Ying Yi via Phabricator via cfe-commits]

MaggieYi created this revision.
MaggieYi added reviewers: echristo, jmorse, wolfgangp, probinson, dblaikie, 
aprantl.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The detailed description of the issue could be found in 
https://bugs.llvm.org/show_bug.cgi?id=51277.

In the original code, the 'getDeclAlignIfRequired' function is used.
The 'getDeclAlignIfRequired' function will return the max alignment
of all aligned attributes if the type has aligned attributes. The
function doesn’t consider the type at all.

The 'getTypeAlignIfRequired' function uses the type’s alignment value,
which also used by the 'alignof' function. I think we should use the
function of 'getTypeAlignIfRequired'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124006

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-struct-align.c


Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct 
type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm 
%s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", 
{{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", 
{{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", 
{{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+  __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 
'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 


Index: clang/test/CodeGen/debug-info-struct-align.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-struct-align.c
@@ -0,0 +1,21 @@
+//  Test for debug info related to DW_AT_alignment attribute in the struct type.
+// RUN: %clang_cc1 -dwarf-version=5 -debug-info-kind=standalone -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType", {{.*}}, align: 32
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType1", {{.*}}, align: 8
+// CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "MyType2", {{.*}}, align: 8
+
+struct MyType {
+  int m;
+} __attribute__((aligned(1)));
+struct MyType mt;
+
+struct MyType1 {
+  int m;
+} __attribute__((packed, aligned(1)));
+struct MyType1 mt1;
+
+struct MyType2 {
+  __attribute__((packed))int m;
+} __attribute__((aligned(1)));
+struct MyType2 mt2;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3553,7 +3553,11 @@
 return getOrCreateRecordFwdDecl(Ty, RDContext);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  // __attribute__((aligned)) can increase or decrease alignment *except* on a
+  // struct or struct member, where it only increases  alignment unless 'packed'
+  // is also specified. To handle this case, the `getTypeAlignIfRequired` needs
+  // to be used.
+  auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

should we have a test to show the effect of -foffload-new-driver on 
-ccc-print-phases for CUDA/HIP?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123325

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

In D123498#3458967 , @pengfei wrote:

>> Maybe I can ask cmake to check for architecture/targets during configuration 
>> and select the headers automatically, but that is beyond the scope of this 
>> patch.
>
> I'm not familar with cmake, but I guess it might be doable. I once verified 
> the X86 headers by command `echo '#include ' | clang -x c -E - | 
> grep '#.*clang.*h' | grep -o '[^\/]*\.h' |sort|uniq`.
> Notice, targets like X86 doesn't have a single entry. But it still available 
> with a bit more work.

Ah I see! Let me look into adopting something along this line. Maybe eventually 
we can remove all the lists (or reduce the lists to minimal) here.

Thanks for the input!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D123325#3459108 , @yaxunl wrote:

> should we have a test to show the effect of -foffload-new-driver on 
> -ccc-print-phases for CUDA/HIP?

We probably should, it's tested w/ OpenMP but we should check the bound 
architectures and such. I can add one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123325

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

[PATCH] D123783: [clang] Eliminate TypeProcessingState::trivial.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:170
 /// Whether we saved the attributes in the decl spec.
 bool hasSavedAttrs;
 

mboehme wrote:
> aaron.ballman wrote:
> > Isn't the same true for this variable? It seems like:
> > 
> > `trivial` == `savedAttrs.empty()`
> > `hasSavedAttrs` == `!savedAttrs.empty()`
> That's what I also thought at first -- but the situation for `hasSavedAttrs` 
> is a bit more complicated. It gets set whenever `saveDeclAttrs()` is called, 
> even if it doesn't actually end up saving any attributes (i.e. if 
> `spec.getAttributes()` is empty).
> 
> `hasSavedAttrs` is then used to prevent any further calls to 
> `saveDeclSpecAttrs()` from doing anything:
> 
> ```
> // Don't try to save them multiple times.
> if (hasSavedAttrs) return;
> ```
> 
> Conceivably, `spec` _might_ have had attributes added to it in the meantime 
> -- not sure? It might be OK to just replace this logic with `if 
> (!savedAttrs.empty()) return;` -- but I'm not familiar enough with how this 
> code gets used and therefore decided it would be better not to change it. Can 
> you give additional input on this?
I have the impression that is an oversight in the code rather than an 
intentional behavior. I think it may be okay to replace the logic with 
`!savedAttrs.empty()` as well; if you do that, do you get any test failures? 
(If you do, then that's a sign something else might be going on.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123783

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

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

need a CodeGenCUDA test for the registering. Also need a Driver test for the 
subcommands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123812

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> I wouldn't think so either? In this case the problem is that 'u' is not in 
> the re-manufactured scope, I think there is a bit of work to make sure that 
> lambdas ALSO get the scope of their containing function, if they are in a 
> functiondecl.

I wouldn't expect lambdas to require special handling; I think they should be 
handled via their transformation to a member function of a dependent local 
class or dependent member class.


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

https://reviews.llvm.org/D119544

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

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Also, I am wondering whether we should document the new embedding scheme: 
section names, symbol names, entries, etc, if it has not bee done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123812

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D119544#3459169 , @tahonermann 
wrote:

>> I wouldn't think so either? In this case the problem is that 'u' is not in 
>> the re-manufactured scope, I think there is a bit of work to make sure that 
>> lambdas ALSO get the scope of their containing function, if they are in a 
>> functiondecl.
>
> I wouldn't expect lambdas to require special handling; I think they should be 
> handled via their transformation to a member function of a dependent local 
> class or dependent member class.

Yeah, its not lambda-specific, thats just the example here.  The example you 
gave offline shows that this is the case with a dependent local class as well.


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

https://reviews.llvm.org/D119544

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

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D123812#3459164 , @yaxunl wrote:

> need a CodeGenCUDA test for the registering. Also need a Driver test for the 
> subcommands.

Testing things inside the linker wrapper is a little hairy. I may need to add a 
special option for doing dry runs and printing the wrapping code to we can test 
this more satisfactorily. Doing that will create some more noise for review 
unfortunately. For OpenMP is simply ran all of our unit tests using the new 
driver and considered that sufficient evidence that it was working. Also which 
driver sub-commands should be tested?

In D123812#3459172 , @yaxunl wrote:

> Also, I am wondering whether we should document the new embedding scheme: 
> section names, symbol names, entries, etc, if it has not bee done.

There's some existing documentation 
 I wrote for OpenMP 
offloading once we started using this scheme. I was planning on updating it 
with this scheme for CUDA once it's landed, no sense writing documentation 
before it's final.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123812

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

[PATCH] D123958: [randstruct] Randomize all elements of a record

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think you'll need a more targeted approach than assuming the only kinds of 
declarations in a struct are field-like in C.

It seems that the issue you've got is with anonymous objects in a structure 
where the inner fields are available for lookup in the outer structure. One 
question I have is: what's the expectation for the user? There's two ways to 
look at this. 1) The anonymous object is a single field; that its members can 
be found in the outer object is not relevant. 2) The fields of the anonymous 
object should also be randomized. The same is true for any inner structure, not 
just anonymous ones.

I had assumed that any structure not marked for randomization would not be 
randomized. Based on that, I don't think inner structure objects (anonymous or 
otherwise) should automatically randomize their fields. WDYT?




Comment at: clang/lib/Sema/SemaDecl.cpp:18049
 !Record->isRandomized()) {
-  SmallVector OrigFieldOrdering(Record->fields());
+  SmallVector OrigFieldOrdering(Record->decls());
   SmallVector NewFieldOrdering;

This (and everything else about fields) is now misnamed.



Comment at: clang/lib/Sema/SemaInit.cpp:2126
   RecordDecl::field_iterator FieldEnd = RD->field_end();
-  size_t NumRecordFields = std::distance(RD->field_begin(), RD->field_end());
+  size_t NumRecordFields = std::distance(RD->decls_begin(), RD->decls_end());
   bool CheckForMissingFields =

This is now misnamed.



Comment at: clang/lib/Sema/SemaInit.cpp:2189
   if (IList->getNumInits() == 1) {
 if (NumRecordFields == 1)
   return true;

This is no longer testing the correct thing. e.g.,
```
struct S {
  int f;
  _Static_assert(sizeof(int) == 4, "oh no!");
};
```
This has two decls but only one field:
```
TranslationUnitDecl
`-RecordDecl  line:1:8 struct S definition
  |-FieldDecl  col:7 f 'int'
  `-StaticAssertDecl  col:3
|-ImplicitCastExpr  '_Bool' 
| `-BinaryOperator  'int' '=='
|   |-UnaryExprOrTypeTraitExpr  'unsigned long' sizeof 'int'
|   `-ImplicitCastExpr  'unsigned long' 
| `-IntegerLiteral  'int' 4
`-StringLiteral  'char[7]' lvalue "oh no!"
```
Note, when you randomize that structure, we drop the `_Static_assert` 
declaration, so there is definitely a bug to be fixed here: 
https://godbolt.org/z/n9YE63rno


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123958

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