[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1186
 
-defm unstable : BoolFOption<"unstable",
-  LangOpts<"Unstable">, DefaultFalse,
-  PosFlag,
+defm experimental_library : BoolFOption<"experimental-library",
+  LangOpts<"ExperimentalLibrary">, DefaultFalse,

ldionne wrote:
> MaskRay wrote:
> > This can be simplified with `OptInCC1FFlag` (both driver/CC1 for the pos 
> > form, but driver-only for the neg form).
> > You'll need to set CoreOption to make the option available to clang-cl.
> I was looking for documentation on `OptInCC1FFlag`, and I found:
> 
> ```
> // A boolean option which is opt-in in CC1. The positive option exists in CC1 
> and
> // Args.hasArg(OPT_ffoo) can be used to check that the flag is enabled.
> // This is useful if the option is usually disabled.
> // Use this only when the option cannot be declared via BoolFOption.
> multiclass OptInCC1FFlag ```
> 
> It says to use `BoolFOption` is we can. So should I stick with `BoolFOption`?
OK. Using `BoolFOption` is fine as we can express the `ExperimentalLibrary` 
logic in the tablegen file. I just feel that the boilerplate is a bit higher 
than `OptInCC1FFlag`.



Comment at: clang/lib/Driver/ToolChain.cpp:1016
 CmdArgs.push_back("-lc++");
+if (Args.hasArg(options::OPT_fexperimental_library))
+  CmdArgs.push_back("-lc++experimental");

ldionne wrote:
> MaskRay wrote:
> > There may be an archive ordering problem. 
> I'm not sure I follow -- what problem are you concerned about?
https://lld.llvm.org/ELF/warn_backrefs.html 

When these -l options are used to link archives (.a), they should be added in a 
dependency order.

-lc++experimental presumably uses symbols from -lc++abi and must precede 
-lc++abi.
-lc++abi uses symbols from -lunwind and must precede -lunwind.

For macOS and Windows, the order usually doesn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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

[PATCH] D129855: [clang][PowerPC] Set lld as clang's default linker for PowerPC Linux

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This is not right as using `ld.lld` as the default linker isn't the majority 
case. If you want to change the default for your distribution, set 
`-DCLANG_DEFAULT_LINKER=lld`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129855

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

[PATCH] D129921: [clang-format] Never remove braces in macro definitions

[Marek Kurdej via Phabricator via cfe-commits]

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

Nice finding! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129921

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

[PATCH] D129926: [clang-format] Handle constructor invocations after new operator in C# correct

[Danil Sidoruk via Phabricator via cfe-commits]

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

- Handle constructor invocations after new operator in C# correct
- Add test for the case with lambda in constructor arguments after new operator

Closes https://github.com/llvm/llvm-project/issues/56549


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129926

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


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -616,6 +616,24 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTestCSharp, CSharpNewOperator) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat("public void F() {\n"
+   "  var v = new C(() => { var t = 5; });\n"
+   "}",
+   Style);
+  verifyFormat("public void F() {\n"
+   "  var v = new C(() => {\n"
+   "try {\n"
+   "} catch {\n"
+   "  var t = 5;\n"
+   "}\n"
+   "  });\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpLambdas) {
   FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
   FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2861,6 +2861,11 @@
 
   if (Style.isCSharp()) {
 do {
+  // Handle constructor invocation
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+
+  // Handle array initialization syntax
   if (FormatTok->is(tok::l_brace))
 parseBracedList();
 


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -616,6 +616,24 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTestCSharp, CSharpNewOperator) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat("public void F() {\n"
+   "  var v = new C(() => { var t = 5; });\n"
+   "}",
+   Style);
+  verifyFormat("public void F() {\n"
+   "  var v = new C(() => {\n"
+   "try {\n"
+   "} catch {\n"
+   "  var t = 5;\n"
+   "}\n"
+   "  });\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpLambdas) {
   FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
   FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2861,6 +2861,11 @@
 
   if (Style.isCSharp()) {
 do {
+  // Handle constructor invocation
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+
+  // Handle array initialization syntax
   if (FormatTok->is(tok::l_brace))
 parseBracedList();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128907: [Clang] Disable noundef attribute for languages which allow uninitialized function arguments

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

The consensus in the meeting was that we should try introducing a new argument 
attribute for values that are allowed to use undef values. It would need to be 
applied to the builtins and any wrapper functions for them. This would leave 
the normal language undefined behavior for arbitrary arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128907

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

It's a little confusing, because it now looks like _every_ `Type` in the AST is 
wrapped in an `ElaboratedTypeLoc` + `ElaboratedType`. IWYU's debug AST dump 
shows this (excerpt):

  tests/cxx/sizeof_reference.cc:51:8: (1) [ VarDecl ] size_t s2 

 
  tests/cxx/sizeof_reference.cc:51:1: (2) [ ElaboratedTypeLoc ] size_t  

 
  tests/cxx/sizeof_reference.cc:51:1: (2) [ ElaboratedType ] size_t 

 
  tests/cxx/sizeof_reference.cc:51:1: (3) [ TypedefTypeLoc ] size_t 

 
  tests/cxx/sizeof_reference.cc:51:1: (3) [ TypedefType ] size_t

 
  Marked full-info use of decl size_t (from 
/home/kimgr/code/iwyu/out/main/lib/clang/15.0.0/include/stddef.h:46:23) at 
tests/cxx/sizeof_reference.cc:51:1
  tests/cxx/sizeof_reference.cc:51:13: (2) [ UnaryExprOrTypeTraitExpr ] 
UnaryExprOrTypeTraitExpr 0x5589fd2a4230 'unsigned long' sizeof 
'IndirectTemplateStruct &' 


 
  (For type IndirectTemplateStruct): 

 
  Marked full-info use of decl IndirectTemplateStruct (from 
tests/cxx/sizeof_reference.cc:18:30) at tests/cxx/sizeof_reference.cc:51:20 
 
  tests/cxx/sizeof_reference.cc:51:20: (3) [ LValueReferenceTypeLoc ] 
IndirectTemplateStruct & 
   
  tests/cxx/sizeof_reference.cc:51:20: (3) [ LValueReferenceType ] 
IndirectTemplateStruct & 
  
  tests/cxx/sizeof_reference.cc:51:20: (4) [ ElaboratedTypeLoc ] 
IndirectTemplateStruct   

  tests/cxx/sizeof_reference.cc:51:20: (4) [ ElaboratedType ] 
IndirectTemplateStruct   
   
  tests/cxx/sizeof_reference.cc:51:20: (5) [ TemplateSpecializationTypeLoc ] 
IndirectTemplateStruct   

  tests/cxx/sizeof_reference.cc:51:20: (5) [ TemplateSpecializationType ] 
IndirectTemplateStruct   
   
  Marked fwd-decl use of decl IndirectTemplateStruct (from 
tests/cxx/sizeof_reference.cc:18:30) at tests/cxx/sizeof_reference.cc:51:20 
  
  tests/cxx/sizeof_reference.cc:51:20: (6, fwd decl) [ TemplateName ] 
IndirectTemplateStruct  
   
  tests/cxx/sizeof_reference.cc:51:43: (6, fwd decl) [ TemplateArgumentLoc ] 
 

  tests/cxx/sizeof_reference.cc:51:43: (7, fwd decl) [ ElaboratedTypeLoc ] 
IndirectClass   
  
  tests/cxx/sizeof_reference.cc:51:43: (7, fwd decl) [ ElaboratedType ] 
IndirectClass   
 
  tests/cxx/sizeof_reference.cc:51:43: (8, fwd decl) [ RecordTypeLoc ] class 
IndirectClass   

  tests/cxx/sizeof_reference.cc:51:43: (8, fwd decl) [ RecordType ] class 
IndirectClass   
   
  Marked fwd-decl use

[clang] b246574 - [clang-format][docs] Fix incorrect 'clang-format 7' option markers

[Krystian Kuzniarek via cfe-commits]

Author: Krystian Kuzniarek
Date: 2022-07-16T18:19:11+02:00
New Revision: b2465748f236810944cf31f9438ac715e5362334

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

LOG: [clang-format][docs] Fix incorrect 'clang-format 7' option markers

Introduced by 23a5090c6, some style option markers indicated
'clang-format 7', though their respective options were available in
different releases.

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/include/clang/Tooling/Inclusions/IncludeStyle.h

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index a65382729da4a..ffd61ee2a1ae4 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1256,7 +1256,7 @@ the configuration (without a prefix: ``Auto``).
  """";
  "";
 
-**AlwaysBreakTemplateDeclarations** (``BreakTemplateDeclarationsStyle``) 
:versionbadge:`clang-format 7`
+**AlwaysBreakTemplateDeclarations** (``BreakTemplateDeclarationsStyle``) 
:versionbadge:`clang-format 3.4`
   The template declaration breaking style to use.
 
   Possible values:
@@ -2623,7 +2623,7 @@ the configuration (without a prefix: ``Auto``).
   For example: `KJ_IF_MAYBE
   `_
 
-**IncludeBlocks** (``IncludeBlocksStyle``) :versionbadge:`clang-format 7`
+**IncludeBlocks** (``IncludeBlocksStyle``) :versionbadge:`clang-format 6`
   Dependent on the value, multiple ``#include`` blocks can be sorted
   as one and divided based on category.
 
@@ -2663,7 +2663,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**IncludeCategories** (``List of IncludeCategories``) 
:versionbadge:`clang-format 7`
+**IncludeCategories** (``List of IncludeCategories``) 
:versionbadge:`clang-format 3.8`
   Regular expressions denoting the 
diff erent ``#include`` categories
   used for ordering ``#includes``.
 
@@ -2711,7 +2711,7 @@ the configuration (without a prefix: ``Auto``).
 Priority:1
 SortPriority:0
 
-**IncludeIsMainRegex** (``String``) :versionbadge:`clang-format 7`
+**IncludeIsMainRegex** (``String``) :versionbadge:`clang-format 3.9`
   Specify a regular expression of suffixes that are allowed in the
   file-to-main-include mapping.
 
@@ -2724,7 +2724,7 @@ the configuration (without a prefix: ``Auto``).
   For example, if configured to "(_test)?$", then a header a.h would be seen
   as the "main" include in both a.cc and a_test.cc.
 
-**IncludeIsMainSourceRegex** (``String``) :versionbadge:`clang-format 7`
+**IncludeIsMainSourceRegex** (``String``) :versionbadge:`clang-format 10`
   Specify a regular expression for files being formatted
   that are allowed to be considered "main" in the
   file-to-main-include mapping.

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index f8a4b069b2e75..77ff9a8634295 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -786,7 +786,7 @@ struct FormatStyle {
   };
 
   /// The template declaration breaking style to use.
-  /// \version 7
+  /// \version 3.4
   BreakTemplateDeclarationsStyle AlwaysBreakTemplateDeclarations;
 
   /// A vector of strings that should be interpreted as attributes/qualifiers

diff  --git a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h 
b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
index d5638642d0172..d6b2b0192477d 100644
--- a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -50,7 +50,7 @@ struct IncludeStyle {
 
   /// Dependent on the value, multiple ``#include`` blocks can be sorted
   /// as one and divided based on category.
-  /// \version 7
+  /// \version 6
   IncludeBlocksStyle IncludeBlocks;
 
   /// See documentation of ``IncludeCategories``.
@@ -114,7 +114,7 @@ struct IncludeStyle {
   ///   Priority:1
   ///   SortPriority:0
   /// \endcode
-  /// \version 7
+  /// \version 3.8
   std::vector IncludeCategories;
 
   /// Specify a regular expression of suffixes that are allowed in the
@@ -128,7 +128,7 @@ struct IncludeStyle {
   ///
   /// For example, if configured to "(_test)?$", then a header a.h would be 
seen
   /// as the "main" include in both a.cc and a_test.cc.
-  /// \version 7
+  /// \version 3.9
   std::string IncludeIsMainRegex;
 
   /// Specify a regular expression for files being formatted
@@ -149,7 +149,7 @@ struct IncludeStyle {
   /// also being respected in later phase). Without this option set,
   /// ``ClassImpl.hpp`` would not have the main include file put on top
   /// before 

[PATCH] D124625: [clang-format][docs] Fix incorrect 'clang-format 7' option markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

I forgot to add "Differential revision" footer to the commit message and so 
I've got to manually close this review. Sorry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124625

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

[PATCH] D129934: [clang-format][docs] Fix incorrect 'clang-format 4' option markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry created this revision.
kuzkry added a reviewer: MyDeveloperDay.
kuzkry added a project: clang-format.
Herald added a project: All.
kuzkry requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Introduced by 23a5090c6 
, some 
style option markers indicated
'clang-format 4', though their respective options were available in
earlier releases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129934

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


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2980,7 +2980,7 @@
   ////* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment 
with plenty of
   /// * information */
   /// \endcode
-  /// \version 4
+  /// \version 3.8
   bool ReflowComments;
   // clang-format on
 
@@ -3236,7 +3236,7 @@
   /// insensitive fashion.
   /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
   /// sensitive fashion.
-  /// \version 4
+  /// \version 3.8
   SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3670,7 +3670,7 @@
 
 
 
-**ReflowComments** (``Boolean``) :versionbadge:`clang-format 4`
+**ReflowComments** (``Boolean``) :versionbadge:`clang-format 3.8`
   If ``true``, clang-format will attempt to re-flow comments.
 
   .. code-block:: c++
@@ -3910,7 +3910,7 @@
int bar;   int bar;
  } // namespace b   } // namespace b
 
-**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 4`
+**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8`
   Controls if and how clang-format will sort ``#includes``.
   If ``Never``, includes are never sorted.
   If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2980,7 +2980,7 @@
   ////* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
   /// * information */
   /// \endcode
-  /// \version 4
+  /// \version 3.8
   bool ReflowComments;
   // clang-format on
 
@@ -3236,7 +3236,7 @@
   /// insensitive fashion.
   /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
   /// sensitive fashion.
-  /// \version 4
+  /// \version 3.8
   SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3670,7 +3670,7 @@
 
 
 
-**ReflowComments** (``Boolean``) :versionbadge:`clang-format 4`
+**ReflowComments** (``Boolean``) :versionbadge:`clang-format 3.8`
   If ``true``, clang-format will attempt to re-flow comments.
 
   .. code-block:: c++
@@ -3910,7 +3910,7 @@
int bar;   int bar;
  } // namespace b   } // namespace b
 
-**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 4`
+**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8`
   Controls if and how clang-format will sort ``#includes``.
   If ``Never``, includes are never sorted.
   If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129934: [clang-format][docs] Fix incorrect 'clang-format 4' option markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

Hi! Here's a list of earliest commits where each option changed in this PR was 
introduced:

ReflowComments - a0a5039d2e520fd42646cb305ed8b696acb41e63
SortIncludes - da446770823be1b1d1562734e55da7c1a5f2579c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129934

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov reopened this revision.
mizvekov added a comment.

In D112374#3653967 , @JDevlieghere 
wrote:

> I'm sorry to hear you're having trouble building LLDB. The LLDB website has 
> quite an elaborate guide with instructions in how to build LLDB: 
> https://lldb.llvm.org/resources/build.html, including specific instructions 
> on Windows.

The instructions exist, doesn't mean they work or that they are a fair burden 
on the developers of the other projects.

The LLDB build / test system is made of the same 'stuff' as the rest of LLVM 
sure, but it does a lot more 'questionable' things which makes it look more 
hazardous.
For example, opening hundreds of frozen terminal windows or creating paths in 
the build directory which contain unsubstituted variables.
So it seems to me that for me to be comfortable working with this, I would have 
to adjust my workflow to build LLVM in a sandbox instead.

If we tried polling other clang devs, we might find that standard practice is 
that we are not really even trying to build and run LLDB tests locally anymore.

libcxx devs have a CI pre-commit system which only runs when a path in their 
sub-project is touched. I think this would be reasonable start for LLDB.

Lastly, my main concern is that by keeping this patch off, even if we don't 
suspect a problem in it, this will create a very long tail. The users affected 
don't know about it yet, and they will keep coming
with a delay one by one as we re-land and revert.

> I'm happy to help out. I personally don't know if we should go with (1) or 
> (2), both sound reasonable in their own way. I'd like @teemperor, who's the 
> author of the feature and the affected tests, to weigh in.

I think, but can't confirm, that this is just a case of (1) and for what is 
being tested we don't really care how the return type of those size() methods 
is written.
I would like some way to test that the functionality is not really broken, we 
just changed that test expectation, but alas I can't.

> I don't. I think reverting your change was well within the guidelines 
> outlined by LLVM's patch reversion policy: 
> https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy
>
> Additionally, I think you could've given me a little bit more time to catch 
> up on the discussion here. The code review policy and practices 
> (https://llvm.org/docs/CodeReview.html#code-reviews-speed-and-reciprocity) 
> recommend pinging every few days to once per week depending on how urgent the 
> patch is.

I think, given the size of this patch and the other points I made, we could 
have simply fixed those issues post-commit, if I had received any prior 
notification.

In D112374#3657332 , @kimgr wrote:

> It's a little confusing, because it now looks like _every_ `Type` in the AST 
> is wrapped in an `ElaboratedTypeLoc` + `ElaboratedType`. IWYU's debug AST 
> dump shows this (excerpt):
> I'm not sure I understand why the elaborated type nodes are necessary even 
> when they don't seem to add any additional information?

It's the difference in knowing the type was written without any tag or 
nested-name specifier, and having a type that you are not sure how it was 
written.

When we are dealing with a type which we are not sure, we would like to print 
it fully qualified, with a synthetic nested name specifier computed from it's 
DC, because otherwise it could be confusing as the type could come from 
somewhere very distant from the context we are printing the type from. We would 
not want to assume that a type which has been desugared was written how it's 
desugared state would seem to imply.

FWIW, in the state of affairs we leave clang after this patch, I don't think 
it's worth keeping a separate ElaboratedType anymore, we might as well fuse 
it's functionality into the type nodes which could be wrapped in it. Taking 
care to optimize storage when not used otherwise, I think we can recoup the 
performance lost in this patch, perhaps even end in a better state overall.

But I think doing these two steps in one go would not be sensibly incremental. 
We have in this patch here a very simple core change, which is very unlikely to 
have bugs in itself, but creates enormous test churn.

The second step of eliminating ElaboratedType could be a less simple core 
change with almost zero test churn, which makes it less risky that it would 
introduce a bug that escapes review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

> It's the difference in knowing the type was written without any tag or 
> nested-name specifier, and having a type that you are not sure how it was 
> written.
>
> When we are dealing with a type which we are not sure, we would like to print 
> it fully qualified, with a synthetic nested name specifier computed from it's 
> DC,
> because otherwise it could be confusing as the type could come from somewhere 
> very distant from the context we are printing the type from. We would not
> want to assume that a type which has been desugared was written how it's 
> desugared state would seem to imply.

I'm coming at this from pretty far away, so there's very likely lots of details 
that I'm overlooking. But it seems to me the mainline had only had an 
`ElaboratedType` node if there was elaboration, and not otherwise. And that 
makes a lot more sense to me than having 2 `ElaboratedType*` nodes _for every 
type in the AST_, just to express that "hey, by the way, this type had no 
elaboration".

> FWIW, in the state of affairs we leave clang after this patch, I don't think 
> it's worth keeping a separate ElaboratedType anymore, we might as 
> well fuse it's functionality into the type nodes which could be wrapped in 
> it. Taking care to optimize storage when not used otherwise, I think
> we can recoup the performance lost in this patch, perhaps even end in a 
> better state overall.
>
> But I think doing these two steps in one go would not be sensibly 
> incremental. We have in this patch here a very simple core change, which
> is very unlikely to have bugs in itself, but creates enormous test churn.
>
> The second step of eliminating ElaboratedType could be a less simple core 
> change with almost zero test churn, which makes it less risky that
> it would introduce a bug that escapes review.

That sounds good at face value, but if you're planning to remove these nodes 
again, that would create enormous churn for out-of-tree tools to re-adjust to 
the new shape of the tree.

I can't say what the best solution is, but this patch generates quite a lot of 
work for me, and I would really hope that catching up with the new AST does not 
generate even more work down the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[clang] bbc4a71 - [test] Fix leak in test

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2022-07-16T12:41:12-07:00
New Revision: bbc4a71e413226798972a2180c306efade48ef4f

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

LOG: [test] Fix leak in test

Added: 


Modified: 
clang/unittests/Interpreter/InterpreterTest.cpp

Removed: 




diff  --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index 720e30fafca7e..494926f7c116d 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -234,8 +234,10 @@ static void *AllocateObject(TypeDecl *TD, Interpreter 
&Interp) {
  << std::hex << std::showbase << (size_t)Addr << ")" << Name << "();";
 
   auto R = Interp.ParseAndExecute(SS.str());
-  if (!R)
+  if (!R) {
+free(Addr);
 return nullptr;
+  }
 
   return Addr;
 }
@@ -291,6 +293,7 @@ TEST(IncrementalProcessing, InstantiateTemplate) {
   typedef int (*TemplateSpecFn)(void *);
   auto fn = (TemplateSpecFn)cantFail(Interp->getSymbolAddress(MangledName));
   EXPECT_EQ(42, fn(NewA));
+  free(NewA);
 }
 
 } // end anonymous namespace



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

[PATCH] D129311: [clang-format] Update return code

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/tools/clang-format/git-clang-format:539
   # filter.
-  subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
- '--'])
+  return subprocess.call(['git', 'diff', '--diff-filter=M',
+  old_tree, new_tree, '--exit-code', '--'])

Can we use `run()` instead of `call()` here and below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129311

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

[PATCH] D129940: [clang-format] Fix misannotation of colon in presence of requires clause

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

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

For clauses without parentheses it was annotated as TT_InheritanceColon.
Relates to https://github.com/llvm/llvm-project/issues/56215

  


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129940

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -799,6 +799,70 @@
   << I;
 }
   }
+
+  BaseTokens = annotate("constexpr Foo(Foo const &other)\n"
+": value{other.value} {\n"
+"  do_magic();\n"
+"  do_more_magic();\n"
+"}");
+
+  ConstrainedTokens = annotate("constexpr Foo(Foo const &other)\n"
+   "  requires std::is_copy_constructible\n"
+   ": value{other.value} {\n"
+   "  do_magic();\n"
+   "  do_more_magic();\n"
+   "}");
+
+  NumberOfBaseTokens = 26u;
+  NumberOfAdditionalRequiresClauseTokens = 7u;
+  NumberOfTokensBeforeRequires = 8u;
+
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I) {
+if (I < NumberOfTokensBeforeRequires) {
+  EXPECT_EQ(*BaseTokens[I], *ConstrainedTokens[I]) << I;
+} else {
+  EXPECT_EQ(*BaseTokens[I],
+*ConstrainedTokens[I + NumberOfAdditionalRequiresClauseTokens])
+  << I;
+}
+  }
+
+  BaseTokens = annotate("constexpr Foo(Foo const &other)\n"
+": value{other.value} {\n"
+"  do_magic();\n"
+"  do_more_magic();\n"
+"}");
+
+  ConstrainedTokens = annotate("constexpr Foo(Foo const &other)\n"
+   "  requires (std::is_copy_constructible)\n"
+   ": value{other.value} {\n"
+   "  do_magic();\n"
+   "  do_more_magic();\n"
+   "}");
+
+  NumberOfBaseTokens = 26u;
+  NumberOfAdditionalRequiresClauseTokens = 9u;
+  NumberOfTokensBeforeRequires = 8u;
+
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I) {
+if (I < NumberOfTokensBeforeRequires) {
+  EXPECT_EQ(*BaseTokens[I], *ConstrainedTokens[I]) << I;
+} else {
+  EXPECT_EQ(*BaseTokens[I],
+*ConstrainedTokens[I + NumberOfAdditionalRequiresClauseTokens])
+  << I;
+}
+  }
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsAsm) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -999,7 +999,8 @@
 FormatToken *Prev = Tok->getPreviousNonComment();
 if (!Prev)
   break;
-if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept)) {
+if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept) ||
+Prev->ClosesRequiresClause) {
   Tok->setType(TT_CtorInitializerColon);
 } else if (Prev->is(tok::kw_try)) {
   // Member initializer list within function try block.


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -799,6 +799,70 @@
   << I;
 }
   }
+
+  BaseTokens = annotate("constexpr Foo(Foo const &other)\n"
+": value{other.value} {\n"
+"  do_magic();\n"
+"  do_more_magic();\n"
+"}");
+
+  ConstrainedTokens = annotate("constexpr Foo(Foo const &other)\n"
+   "  requires std::is_copy_constructible\n"
+   ": value{other.value} {\n"
+   "  do_magic();\n"
+   "  do_more_magic();\n"
+   "}");
+
+  NumberOfBaseTokens = 26u;
+  NumberOfAdditio

[clang] a0458d9 - [clang-format] Never remove braces in macro definitions

[via cfe-commits]

Author: owenca
Date: 2022-07-16T13:11:10-07:00
New Revision: a0458d92e9e7b279c9ff491429aad86ccedee7c4

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

LOG: [clang-format] Never remove braces in macro definitions

Fixes #56559.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 97c3d86282a02..bfacfa3e3595f 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -539,7 +539,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken 
*OpeningBrace,
   break;
 case tok::r_brace:
   if (OpeningBrace) {
-if (!Style.RemoveBracesLLVM ||
+if (!Style.RemoveBracesLLVM || Line->InPPDirective ||
 !OpeningBrace->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)) {
   return false;
 }

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 08cfdbe2cc7a1..6b7454f9afd70 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25803,6 +25803,13 @@ TEST_F(FormatTest, RemoveBraces) {
 
   Style.ColumnLimit = 20;
 
+  verifyFormat("int i;\n"
+   "#define FOO(a, b)  \\\n"
+   "  while (a) {  \\\n"
+   "b; \\\n"
+   "  }",
+   Style);
+
   verifyFormat("int ab = [](int i) {\n"
"  if (i > 0) {\n"
"i = 12345678 -\n"



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

[PATCH] D129921: [clang-format] Never remove braces in macro definitions

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0458d92e9e7: [clang-format] Never remove braces in macro 
definitions (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129921

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25803,6 +25803,13 @@
 
   Style.ColumnLimit = 20;
 
+  verifyFormat("int i;\n"
+   "#define FOO(a, b)  \\\n"
+   "  while (a) {  \\\n"
+   "b; \\\n"
+   "  }",
+   Style);
+
   verifyFormat("int ab = [](int i) {\n"
"  if (i > 0) {\n"
"i = 12345678 -\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -539,7 +539,7 @@
   break;
 case tok::r_brace:
   if (OpeningBrace) {
-if (!Style.RemoveBracesLLVM ||
+if (!Style.RemoveBracesLLVM || Line->InPPDirective ||
 !OpeningBrace->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)) {
   return false;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25803,6 +25803,13 @@
 
   Style.ColumnLimit = 20;
 
+  verifyFormat("int i;\n"
+   "#define FOO(a, b)  \\\n"
+   "  while (a) {  \\\n"
+   "b; \\\n"
+   "  }",
+   Style);
+
   verifyFormat("int ab = [](int i) {\n"
"  if (i > 0) {\n"
"i = 12345678 -\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -539,7 +539,7 @@
   break;
 case tok::r_brace:
   if (OpeningBrace) {
-if (!Style.RemoveBracesLLVM ||
+if (!Style.RemoveBracesLLVM || Line->InPPDirective ||
 !OpeningBrace->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)) {
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129942: [clang-format] Indent TT_CtorInitializerColon after requires clauses

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

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129942

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24711,6 +24711,18 @@
   "struct S {};",
   Style);
 
+  Style = getLLVMStyle();
+  Style.ConstructorInitializerIndentWidth = 4;
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
+  Style.PackConstructorInitializers = FormatStyle::PCIS_Never;
+  verifyFormat("constexpr Foo(Foo const &other)\n"
+   "  requires std::is_copy_constructible\n"
+   ": value{other.value} {\n"
+   "  do_magic();\n"
+   "  do_more_magic();\n"
+   "}",
+   Style);
+
   // Not a clause, but we once hit an assert.
   verifyFormat("#if 0\n"
"#else\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1193,6 +1193,10 @@
   break;
 }
   }
+  if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
+  TT_InheritanceComma)) {
+return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
+  }
   if ((PreviousNonComment &&
(PreviousNonComment->ClosesTemplateDeclaration ||
 PreviousNonComment->ClosesRequiresClause ||
@@ -1267,10 +1271,6 @@
   Style.BreakInheritanceList == FormatStyle::BILS_AfterColon) {
 return CurrentState.Indent;
   }
-  if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
-  TT_InheritanceComma)) {
-return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
-  }
   if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() &&
   !Current.isOneOf(tok::colon, tok::comment)) {
 return ContinuationIndent;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24711,6 +24711,18 @@
   "struct S {};",
   Style);
 
+  Style = getLLVMStyle();
+  Style.ConstructorInitializerIndentWidth = 4;
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
+  Style.PackConstructorInitializers = FormatStyle::PCIS_Never;
+  verifyFormat("constexpr Foo(Foo const &other)\n"
+   "  requires std::is_copy_constructible\n"
+   ": value{other.value} {\n"
+   "  do_magic();\n"
+   "  do_more_magic();\n"
+   "}",
+   Style);
+
   // Not a clause, but we once hit an assert.
   verifyFormat("#if 0\n"
"#else\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1193,6 +1193,10 @@
   break;
 }
   }
+  if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
+  TT_InheritanceComma)) {
+return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
+  }
   if ((PreviousNonComment &&
(PreviousNonComment->ClosesTemplateDeclaration ||
 PreviousNonComment->ClosesRequiresClause ||
@@ -1267,10 +1271,6 @@
   Style.BreakInheritanceList == FormatStyle::BILS_AfterColon) {
 return CurrentState.Indent;
   }
-  if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
-  TT_InheritanceComma)) {
-return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
-  }
   if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() &&
   !Current.isOneOf(tok::colon, tok::comment)) {
 return ContinuationIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 45067f8 - [test] Don't leak DerivedArgList in test

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2022-07-16T14:03:38-07:00
New Revision: 45067f8fbf61284839c739807c2da2e2505661eb

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

LOG: [test] Don't leak DerivedArgList in test

Added: 


Modified: 
clang/unittests/Driver/ToolChainTest.cpp

Removed: 




diff  --git a/clang/unittests/Driver/ToolChainTest.cpp 
b/clang/unittests/Driver/ToolChainTest.cpp
index 3637b10cdd667..02ab9e743ebe6 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -486,8 +486,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   for (auto *A : Args)
 DAL->append(A);
 
-  auto *TranslatedArgs =
-  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
+  std::unique_ptr TranslatedArgs{
+  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None)};
   EXPECT_NE(TranslatedArgs, nullptr);
   if (TranslatedArgs) {
 auto *A = TranslatedArgs->getLastArg(
@@ -506,7 +506,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   for (auto *A : Args)
 DAL->append(A);
 
-  TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
+  TranslatedArgs.reset(
+  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None));
   EXPECT_EQ(Diags.getNumErrors(), 1u);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(),
"invalid validator version : 0.1\nIf validator major version is 
"
@@ -521,7 +522,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   for (auto *A : Args)
 DAL->append(A);
 
-  TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
+  TranslatedArgs.reset(
+  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None));
   EXPECT_EQ(Diags.getNumErrors(), 2u);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(),
"invalid validator version : 1\nFormat of validator version is "
@@ -536,7 +538,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   for (auto *A : Args)
 DAL->append(A);
 
-  TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
+  TranslatedArgs.reset(
+  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None));
   EXPECT_EQ(Diags.getNumErrors(), 3u);
   EXPECT_STREQ(
   DiagConsumer->Errors.back().c_str(),
@@ -552,7 +555,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   for (auto *A : Args)
 DAL->append(A);
 
-  TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
+  TranslatedArgs.reset(
+  TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None));
   EXPECT_EQ(Diags.getNumErrors(), 4u);
   EXPECT_STREQ(
   DiagConsumer->Errors.back().c_str(),



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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D112374#3657472 , @kimgr wrote:

> I'm coming at this from pretty far away, so there's very likely lots of 
> details that I'm overlooking. But it seems to me the mainline had only had an 
> `ElaboratedType` node if there was elaboration, and not otherwise. And that 
> makes a lot more sense to me than having 2 `ElaboratedType*` nodes _for every 
> type in the AST_, just to express that "hey, by the way, this type had no 
> elaboration".

There are no 2 `ElaboratedType` nodes, there is only one. If you are seeing 
something like an ElaboratedType wrapping directly over another ElaboratedType, 
that would seem to be a bug.

To the second point, it's a problem of representation. Having no elaboration is 
not the same thing as having no information about elaboration, so we better not 
represent both things with the same state.

> That sounds good at face value, but if you're planning to remove these nodes 
> again, that would create enormous churn for out-of-tree tools to re-adjust to 
> the new shape of the tree.
>
> I can't say what the best solution is, but this patch generates quite a lot 
> of work for me, and I would really hope that catching up with the new AST 
> does not generate even more work down the line.

That part I don't understand why. Before this patch, clang can produce a bunch 
of type nodes wrapped in an ElTy, or not. After this patch, we add ElTys in 
more cases, but the basic situation remains the same.

Why IWYU would even care about ElaboratedTypes at all? I would have expected a 
`git grep ElaboratedType` on IWYU sources to have no matches.

Can you elaborate on that?

In general, I would not expect external tools to care about the shape of the 
AST. I would expect the type API would be used in a way where we ignore a type 
sugar node we have no reason to acknowledge.
Ie you query if some type is a (possible sugar to) X, and you would either get 
X or nothing. The type sugar over it would just be skipped over and you would 
have no reason to know what was in there or what shape it had.

Of course that was not what happened in practice. A lot of code used `dyn_cast` 
where it should have used `getAs`. Just look at all the fixes in this patch for 
examples.
And fixing that, besides making that code compatible with this patch, also 
fixed other bugs where it would not properly ignore other pre-existing type 
sugar.

If IWYU has unit tests that test too much clang implementation details, that 
would generate unneeded burden on both sides.  Is it for example doing AST dump 
tests and expecting exact outputs?
Not even the clang test suite does that too much.
Is it to compensate for any perceived lack of testing on mainline side?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D129946: [clang-format] Mark constexpr lambdas as lambda

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

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

Otherwise the brace was detected as a function brace, not wrong per se, but 
when directly calling the lambda the calling parens were put on the next line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129946

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -894,6 +894,13 @@
   EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsLambdas) {
+  auto Tokens = annotate("[]() constexpr {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_LambdaLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2127,6 +2127,7 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+case tok::kw_constexpr:
   nextToken();
   break;
 // Specialization of a template with an integer parameter can contain


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -894,6 +894,13 @@
   EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsLambdas) {
+  auto Tokens = annotate("[]() constexpr {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_LambdaLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2127,6 +2127,7 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+case tok::kw_constexpr:
   nextToken();
   break;
 // Specialization of a template with an integer parameter can contain
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D112374#3657472 , @kimgr wrote:

> I can't say what the best solution is, but this patch generates quite a lot 
> of work for me, and I would really hope that catching up with the new AST 
> does not generate even more work down the line.

Okay, I checked out IWYU and I see why you need to look at ElaboratedType in 
some cases. And that also answers a lot of my previous questions.

Some type nodes were before rarely ever elaborated, but will have an 
ElaboratedType over them consistently now.
Searching IWYU source code, some cases where dyn_cast is used in some of them:

iwyu.cc:

  // If we're a constructor, we also need to construct the entire class,
  // even typedefs that aren't used at construct time. Try compiling
  //template struct C { typedef typename T::a t; };
  //class S; int main() { C c; }
  if (isa(fn_decl)) {
CHECK_(parent_type && "How can a constructor have no parent?");
parent_type = RemoveElaboration(parent_type);
if (!TraverseDataAndTypeMembersOfClassHelper(
dyn_cast(parent_type)))
  return false;
  }
  return true;
  `

  if (const auto* enum_type = dyn_cast(type))
return !CanBeOpaqueDeclared(enum_type);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

@kimgr One other general comment.

The way this function is implemented is quite error prone:

  static const NamedDecl* TypeToDeclImpl(const Type* type, bool as_written) {
// Get past all the 'class' and 'struct' prefixes, and namespaces.
type = RemoveElaboration(type);
  
// Read past SubstTemplateTypeParmType (this can happen if a
// template function returns the tpl-arg type: e.g. for
// 'T MyFn() {...}; MyFn.a', the type of MyFn will be a Subst.
type = RemoveSubstTemplateTypeParm(type);
  
CHECK_(!isa(type) && "IWYU doesn't support Objective-C");

Ie the beginning is being too explicit, testing for very specific sugar type 
nodes kinds, in a very specific order, just to skip over them.

That makes it very fragile against clang changes.

You can instead just use `getAs` to step over them in a generic fashion.

I don't think this one gets broken by this MR, but I am very confident it will 
get broken by another patch I have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D129940: [clang-format] Fix misannotation of colon in presence of requires clause

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:820-833
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I) {
+if (I < NumberOfTokensBeforeRequires) {

Can you make it a function or lambda?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129940

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

[clang] 0fbafb5 - [test] Fix memory leak in validateTargetProfile

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2022-07-16T16:47:50-07:00
New Revision: 0fbafb5a1c4381ded4bc7f59a5a6091c229faed7

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

LOG: [test] Fix memory leak in validateTargetProfile

Unfortunatly fixing leak expose use-after-free if delete more then one
Compilation for the same Driver, so I am changing validateTargetProfile
to create own Driver each time.

The test was added by D122865.

Added: 


Modified: 
clang/unittests/Driver/ToolChainTest.cpp

Removed: 




diff  --git a/clang/unittests/Driver/ToolChainTest.cpp 
b/clang/unittests/Driver/ToolChainTest.cpp
index 02ab9e743ebe..64bc616523f0 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
+#include 
 using namespace clang;
 using namespace clang::driver;
 
@@ -388,22 +389,27 @@ struct SimpleDiagnosticConsumer : public 
DiagnosticConsumer {
   std::vector> Errors;
 };
 
-static void validateTargetProfile(StringRef TargetProfile,
-  StringRef ExpectTriple, Driver &TheDriver,
-  DiagnosticsEngine &Diags) {
-  EXPECT_TRUE(TheDriver.BuildCompilation(
-  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"}));
+static void validateTargetProfile(
+StringRef TargetProfile, StringRef ExpectTriple,
+IntrusiveRefCntPtr &InMemoryFileSystem,
+DiagnosticsEngine &Diags) {
+  Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem);
+  std::unique_ptr C{TheDriver.BuildCompilation(
+  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})};
+  EXPECT_TRUE(C);
   EXPECT_STREQ(TheDriver.getTargetTriple().c_str(), ExpectTriple.data());
   EXPECT_EQ(Diags.getNumErrors(), 0u);
 }
 
-static void validateTargetProfile(StringRef TargetProfile,
-  StringRef ExpectError, Driver &TheDriver,
-  DiagnosticsEngine &Diags,
-  SimpleDiagnosticConsumer *DiagConsumer,
-  unsigned NumOfErrors) {
-  EXPECT_TRUE(TheDriver.BuildCompilation(
-  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"}));
+static void validateTargetProfile(
+StringRef TargetProfile, StringRef ExpectError,
+IntrusiveRefCntPtr &InMemoryFileSystem,
+DiagnosticsEngine &Diags, SimpleDiagnosticConsumer *DiagConsumer,
+unsigned NumOfErrors) {
+  Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem);
+  std::unique_ptr C{TheDriver.BuildCompilation(
+  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})};
+  EXPECT_TRUE(C);
   EXPECT_EQ(Diags.getNumErrors(), NumOfErrors);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data());
   Diags.Clear();
@@ -422,41 +428,40 @@ TEST(DxcModeTest, TargetProfileValidation) {
   auto *DiagConsumer = new SimpleDiagnosticConsumer;
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer);
-  Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem);
 
-  validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex", TheDriver,
-Diags);
-  validateTargetProfile("-Ths_6_1", "dxil--shadermodel6.1-hull", TheDriver,
-Diags);
-  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain", TheDriver,
-Diags);
-  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain", TheDriver,
-Diags);
-  validateTargetProfile("-Tgs_6_3", "dxil--shadermodel6.3-geometry", TheDriver,
-Diags);
-  validateTargetProfile("-Tps_6_4", "dxil--shadermodel6.4-pixel", TheDriver,
-Diags);
-  validateTargetProfile("-Tcs_6_5", "dxil--shadermodel6.5-compute", TheDriver,
-Diags);
-  validateTargetProfile("-Tms_6_6", "dxil--shadermodel6.6-mesh", TheDriver,
-Diags);
+  validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex",
+InMemoryFileSystem, Diags);
+  validateTargetProfile("-Ths_6_1", "dxil--shadermodel6.1-hull",
+InMemoryFileSystem, Diags);
+  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
+InMemoryFileSystem, Diags);
+  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
+InMemoryFileSystem, Diags);
+  validateTargetProfile("-Tgs_6_3", "dxil--shadermodel6.3-geometry",
+InMemoryFileSystem, Diags);
+  validateTargetProfile("-Tps_6_4", "dxil--shadermodel

[PATCH] D129946: [clang-format] Mark constexpr lambdas as lambda

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2130
 case tok::kw_noexcept:
+case tok::kw_constexpr:
   nextToken();

Maybe move it up so that it's grouped with `kw_const`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129946

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

[PATCH] D125585: [HLSL][clang][Driver] Parse target profile early to update Driver::TargetTriple.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

@python3kgae
0fbafb5a1c4381ded4bc7f59a5a6091c229faed7 
 is fixing 
leaks in this tests.

Note, the leak was missed because of unrelated issues in gtest and lsan setup 
which we fixed recently.

I incorrectly blamed the leak on D122865 , 
but in fact it was D125585 . I see that 
before this patch we have one Driver, one Compilation and both deleted 
correctly. Leaks are obvious, after this patch we don't delete Compilation.

I fixed the leak, but I had to remove shared driver. My concern is that maybe 
shared driver is important use-case. If it's so, would you like to take a look, 
and find how to avoid a leak with shared driver?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125585

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

[clang] abc8f2b - [Driver] Don't passs --dynamic-linker in -r mode

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2022-07-16T20:13:24-04:00
New Revision: abc8f2b7245f5da09612784ca6e5e0f3dfe4b42d

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

LOG: [Driver] Don't passs --dynamic-linker in -r mode

No behavior change as GNU ld/gold/ld.lld ignore --dynamic-linker in -r mode.
This change makes the intention clearer as we already suppress --dynamic-linker
for -shared, -static, and -static-pie.

Reviewed by: MaskRay, phosek

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Ananas.cpp
clang/lib/Driver/ToolChains/DragonFly.cpp
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/lib/Driver/ToolChains/NetBSD.cpp
clang/lib/Driver/ToolChains/OpenBSD.cpp
clang/test/Driver/ananas.c
clang/test/Driver/dragonfly.c
clang/test/Driver/freebsd.c
clang/test/Driver/fuchsia.c
clang/test/Driver/netbsd.c
clang/test/Driver/openbsd.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Ananas.cpp 
b/clang/lib/Driver/ToolChains/Ananas.cpp
index 40f9e56b38e94..a9c13464a0d68 100644
--- a/clang/lib/Driver/ToolChains/Ananas.cpp
+++ b/clang/lib/Driver/ToolChains/Ananas.cpp
@@ -71,7 +71,7 @@ void ananas::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   CmdArgs.push_back("-export-dynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-Bshareable");
-} else {
+} else if (!Args.hasArg(options::OPT_r)) {
   Args.AddAllArgs(CmdArgs, options::OPT_pie);
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/lib/ld-ananas.so");

diff  --git a/clang/lib/Driver/ToolChains/DragonFly.cpp 
b/clang/lib/Driver/ToolChains/DragonFly.cpp
index 8cfec6a6c4e05..ba901407715f3 100644
--- a/clang/lib/Driver/ToolChains/DragonFly.cpp
+++ b/clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -69,7 +69,7 @@ void dragonfly::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   CmdArgs.push_back("-export-dynamic");
 if (Args.hasArg(options::OPT_shared))
   CmdArgs.push_back("-Bshareable");
-else {
+else if (!Args.hasArg(options::OPT_r)) {
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/usr/libexec/ld-elf.so.2");
 }

diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index 79e3c5cbca5f2..1476d11cd16d5 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -170,7 +170,7 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   CmdArgs.push_back("-export-dynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-Bshareable");
-} else {
+} else if (!Args.hasArg(options::OPT_r)) {
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }

diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 03ff9fe894c88..f96cfde8e9570 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -101,7 +101,7 @@ void fuchsia::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 
   const SanitizerArgs &SanArgs = ToolChain.getSanitizerArgs(Args);
 
-  if (!Args.hasArg(options::OPT_shared)) {
+  if (!Args.hasArg(options::OPT_shared) && !Args.hasArg(options::OPT_r)) {
 std::string Dyld = D.DyldPrefix;
 if (SanArgs.needsAsanRt() && SanArgs.needsSharedRt())
   Dyld += "asan/";

diff  --git a/clang/lib/Driver/ToolChains/NetBSD.cpp 
b/clang/lib/Driver/ToolChains/NetBSD.cpp
index d1eda14a51f01..ac90ed49b8a54 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -139,7 +139,7 @@ void netbsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   CmdArgs.push_back("-export-dynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-Bshareable");
-} else {
+} else if (!Args.hasArg(options::OPT_r)) {
   Args.AddAllArgs(CmdArgs, options::OPT_pie);
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld.elf_so");

diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp 
b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index 54cf3cc89caf7..cd50582c30e84 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -147,7 +147,7 @@ void openbsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back("-Bdynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-shared");
-} else {
+} else if (!Args.hasArg(options::OPT_r)) {
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/usr/libexec

[PATCH] D129714: [Driver] Don't passs --dynamic-linker in -r mode

[Brad Smith 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 rGabc8f2b7245f: [Driver] Don't passs --dynamic-linker in 
-r mode (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129714

Files:
  clang/lib/Driver/ToolChains/Ananas.cpp
  clang/lib/Driver/ToolChains/DragonFly.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/test/Driver/ananas.c
  clang/test/Driver/dragonfly.c
  clang/test/Driver/freebsd.c
  clang/test/Driver/fuchsia.c
  clang/test/Driver/netbsd.c
  clang/test/Driver/openbsd.c

Index: clang/test/Driver/openbsd.c
===
--- clang/test/Driver/openbsd.c
+++ clang/test/Driver/openbsd.c
@@ -37,6 +37,7 @@
 // RUN: %clang --target=mips64el-unknown-openbsd -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64EL-LD %s
 // CHECK-LD-R: "-r"
+// CHECK-LD-R-NOT: "-dynamic-linker"
 // CHECK-LD-R-NOT: "-l
 // CHECK-LD-R-NOT: crt{{[^./]+}}.o
 // CHECK-LD-S: "-cc1" "-triple" "i686-pc-openbsd"
Index: clang/test/Driver/netbsd.c
===
--- clang/test/Driver/netbsd.c
+++ clang/test/Driver/netbsd.c
@@ -468,10 +468,12 @@
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
 
-// -r suppresses default -l and crt*.o like -nostdlib.
+// -r suppresses -dynamic-linker, default -l and crt*.o like -nostdlib.
 // RUN: %clang --target=x86_64-unknown-netbsd -r \
 // RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=RELOCATABLE %s
 // RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-pie"
+// RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -86,6 +86,7 @@
 // RUN: | FileCheck %s -check-prefix=CHECK-RELOCATABLE
 // CHECK-RELOCATABLE-NOT: "-pie"
 // CHECK-RELOCATABLE-NOT: "--build-id"
+// CHECK-RELOCATABLE-NOT "-dynamic-linker"
 // CHECK-RELOCATABLE: "-r"
 // CHECK-RELOCATABLE-NOT: "-l
 // CHECK-RELOCATABLE-NOT: crt{{[^./]+}}.o
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -206,9 +206,10 @@
 // RUN: FileCheck -check-prefix=PPC64-MUNWIND %s
 // PPC64-MUNWIND: "-funwind-tables=2"
 
-/// -r suppresses default -l and crt*.o like -nostdlib.
+/// -r suppresses -dynamic-linker, default -l and crt*.o like -nostdlib.
 // RUN: %clang -### %s --target=aarch64-pc-freebsd11 -r \
 // RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree 2>&1 | FileCheck %s --check-prefix=RELOCATABLE
 // RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
Index: clang/test/Driver/dragonfly.c
===
--- clang/test/Driver/dragonfly.c
+++ clang/test/Driver/dragonfly.c
@@ -4,9 +4,10 @@
 // CHECK: "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" "-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" "{{.*}}crtn.o"
 
-// -r suppresses default -l and crt*.o like -nostdlib.
+// -r suppresses -dynamic-linker, default -l and crt*.o like -nostdlib.
 // RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
 // RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
 // RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: {{.*}}crt{{[^./]+}}.o
Index: clang/test/Driver/ananas.c
===
--- clang/test/Driver/ananas.c
+++ clang/test/Driver/ananas.c
@@ -16,9 +16,11 @@
 // CHECK-SHARED: crtendS.o
 // CHECK-SHARED: crtn.o
 
-// -r suppresses default -l and crt*.o like -nostdlib.
+// -r suppresses -dynamic-linker, default -l and crt*.o like -nostdlib.
 // RUN: %clang %s -### -o %t.o --target=x86_64-unknown-ananas -r 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-RELOCATABLE
 // CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-pie"
+// CHECK-RELOCATABLE-NOT: "-dynamic-linker"
 // CHECK-RELOCATABLE-NOT: "-l
 // CHECK-RELOCATABLE-NOT: /crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/Open

[PATCH] D125585: [HLSL][clang][Driver] Parse target profile early to update Driver::TargetTriple.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D125585#3657734 , @vitalybuka 
wrote:

> @python3kgae
> 0fbafb5a1c4381ded4bc7f59a5a6091c229faed7 
>  is 
> fixing leaks in this tests.
>
> Note, the leak was missed because of unrelated issues in gtest and lsan setup 
> which we fixed recently.
>
> I incorrectly blamed the leak on D122865 , 
> but in fact it was D125585 . I see that 
> before this patch we have one Driver, one Compilation and both deleted 
> correctly. Leaks are obvious, after this patch we don't delete Compilation.
>
> I fixed the leak, but I had to remove shared driver. My concern is that maybe 
> shared driver is important use-case. If it's so, would you like to take a 
> look, and find how to avoid a leak with shared driver?

Hi Vitaly,
Thanks for fixing the leak. Shared driver is not important use-case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125585

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

[PATCH] D129951: [clang] teaches Clang the special ADL rules for functions in std::ranges

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added a reviewer: aaron.ballman.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Per per [range.iter.ops]/2 and [algorithms.requirements]/2, functions
declared in the namespace 'std::ranges' aren't found by ADL, and
suppress ADL when they're called in an unqualified context (e.g. a
using-directive).

Libraries have been implementing these functions as function objects
with varying rules (e.g. libc++ and Microsoft/STL both try their best to
make the function objects appear as standard library function templates,
while libstdc++ makes them plain function objects).

Having a large number of types typically has a negative impact on both
compile-times and progam size, and there are approximately 130 of these
at present. Furthermore, the diagnostics can be marginally improved by
switching to proper functions, which will make it clearer that the
problem is at the user level, rather than the implementation level (see
https://godbolt.org/z/1sYMsxdfM for an example).

By making it possible to implement ranges functions as functions, it's
hoped that the library will be incentivised to migrate over.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129951

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/disable-adl.cpp

Index: clang/test/Sema/disable-adl.cpp
===
--- /dev/null
+++ clang/test/Sema/disable-adl.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+// expected-n...@disable-adl.cpp:* 2{{}}
+
+namespace std {
+  struct S1 {};
+  S1 inhibited(S1);
+
+  namespace ranges {
+struct S2 {};
+void hidden(S2);
+int inhibited(S1);
+  }
+}
+
+void test_functions() {
+  hidden(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden'; did you mean 'std::ranges::hidden'?}}
+
+  using namespace std::ranges;
+  int x = inhibited(std::S1{}); // no error
+}
+
+namespace std {
+  template
+  S1 inhibited_template(T);
+
+  namespace ranges {
+template
+void hidden_template(T);
+
+template
+int inhibited_template(T);
+  }
+}
+
+void test_function_templates() {
+  hidden_template(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden_template'; did you mean 'std::ranges::hidden_template'?}}
+
+  using namespace std::ranges;
+  int x = inhibited_template(std::S1{});
+}
+
+namespace std {
+  S1 inhibited_mixed(S1);
+
+  namespace ranges {
+template
+int inhibited_mixed(T);
+  }
+}
+
+void test_mixed() {
+  using namespace std::ranges;
+  int x = inhibited_mixed(std::S1{});
+}
+
+// Should be covered by the hidden functions checks, but just to be sure.
+void test_ranges_hidden() {
+  {
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+  {
+using namespace std;
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+}
+
+namespace std {
+  namespace ranges {
+void operator-(S2);
+
+struct hidden_friend_operator {
+  friend void operator-(hidden_friend_operator i, int) {}
+};
+
+struct hidden_friend_swap {
+  friend void swap(hidden_friend_swap, hidden_friend_swap) {}
+};
+  }
+}
+
+void test_friends_and_operators() {
+  -std::ranges::S2{};// no error
+  std::ranges::hidden_friend_operator{} - 1; // no error
+
+  swap(std::ranges::hidden_friend_swap{}, std::ranges::hidden_friend_swap{});
+  // expected-error@-1{{use of undeclared identifier 'swap'}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9452,6 +9452,12 @@
   for (ADLResult::iterator I = Fns.begin(), E = Fns.end(); I != E; ++I) {
 DeclAccessPair FoundDecl = DeclAccessPair::make(*I, AS_none);
 
+// Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 and
+// [algorithms.requirements]/2.
+if ((*I)->isInStdRangesNamespace() &&
+Name.getNameKind() == DeclarationName::NameKind::Identifier)
+  continue;
+
 if (FunctionDecl *FD = dyn_cast(*I)) {
   if (ExplicitTemplateArgs)
 continue;
@@ -9650,7 +9656,7 @@
   const OverloadCandidate &Cand1,
   const OverloadCandidate &Cand2) {
   // FIXME: Per P2113R0 we also need to compare the template parameter lists
-  // when comparing template functions. 
+  // when comparing template functions.
   if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
   Cand2.Function->hasPrototype()) {
 auto *PT1 = cast(Cand1.Function->getFunctionType())

[PATCH] D129951: [clang] teaches Clang the special ADL rules for functions in std::ranges

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 445285.
cjdb added a comment.

moves test from Sema to SemaCXX

I Noticed that SemaCXX is only a test directory, and that there's no 
corresponding SemaCXX in either include or lib.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/disable-adl.cpp

Index: clang/test/SemaCXX/disable-adl.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/disable-adl.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+// expected-n...@disable-adl.cpp:* 2{{}}
+
+namespace std {
+  struct S1 {};
+  S1 inhibited(S1);
+
+  namespace ranges {
+struct S2 {};
+void hidden(S2);
+int inhibited(S1);
+  }
+}
+
+void test_functions() {
+  hidden(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden'; did you mean 'std::ranges::hidden'?}}
+
+  using namespace std::ranges;
+  int x = inhibited(std::S1{}); // no error
+}
+
+namespace std {
+  template
+  S1 inhibited_template(T);
+
+  namespace ranges {
+template
+void hidden_template(T);
+
+template
+int inhibited_template(T);
+  }
+}
+
+void test_function_templates() {
+  hidden_template(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden_template'; did you mean 'std::ranges::hidden_template'?}}
+
+  using namespace std::ranges;
+  int x = inhibited_template(std::S1{});
+}
+
+namespace std {
+  S1 inhibited_mixed(S1);
+
+  namespace ranges {
+template
+int inhibited_mixed(T);
+  }
+}
+
+void test_mixed() {
+  using namespace std::ranges;
+  int x = inhibited_mixed(std::S1{});
+}
+
+// Should be covered by the hidden functions checks, but just to be sure.
+void test_ranges_hidden() {
+  {
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+  {
+using namespace std;
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+}
+
+namespace std {
+  namespace ranges {
+void operator-(S2);
+
+struct hidden_friend_operator {
+  friend void operator-(hidden_friend_operator i, int) {}
+};
+
+struct hidden_friend_swap {
+  friend void swap(hidden_friend_swap, hidden_friend_swap) {}
+};
+  }
+}
+
+void test_friends_and_operators() {
+  -std::ranges::S2{};// no error
+  std::ranges::hidden_friend_operator{} - 1; // no error
+
+  swap(std::ranges::hidden_friend_swap{}, std::ranges::hidden_friend_swap{});
+  // expected-error@-1{{use of undeclared identifier 'swap'}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9452,6 +9452,12 @@
   for (ADLResult::iterator I = Fns.begin(), E = Fns.end(); I != E; ++I) {
 DeclAccessPair FoundDecl = DeclAccessPair::make(*I, AS_none);
 
+// Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 and
+// [algorithms.requirements]/2.
+if ((*I)->isInStdRangesNamespace() &&
+Name.getNameKind() == DeclarationName::NameKind::Identifier)
+  continue;
+
 if (FunctionDecl *FD = dyn_cast(*I)) {
   if (ExplicitTemplateArgs)
 continue;
@@ -9650,7 +9656,7 @@
   const OverloadCandidate &Cand1,
   const OverloadCandidate &Cand2) {
   // FIXME: Per P2113R0 we also need to compare the template parameter lists
-  // when comparing template functions. 
+  // when comparing template functions.
   if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
   Cand2.Function->hasPrototype()) {
 auto *PT1 = cast(Cand1.Function->getFunctionType());
@@ -12828,6 +12834,12 @@
CandidateSet, PartialOverloading,
/*KnownValid*/ true);
 
+  // Functions in 'std::ranges' inhibit ADL per [range.iter.ops]/2 and
+  // [algorithms.requirements]/2.
+  if (!ULE->decls().empty() && ULE->decls_begin()->isInStdRangesNamespace() &&
+  ULE->getName().getNameKind() == DeclarationName::NameKind::Identifier)
+return;
+
   if (ULE->requiresADL())
 AddArgumentDependentLookupCandidates(ULE->getName(), ULE->getExprLoc(),
  Args, ExplicitTemplateArgs,
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -396,6 +396,11 @@
   return DC && DC->isStdNamespace();
 }
 
+bool Decl::isInStdRangesNamespace() const {
+  const DeclContext *DC = getDeclContext();
+  return DC && D

[PATCH] D129951: [clang] teaches Clang the special ADL rules for functions in std::ranges

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 445286.
cjdb edited the summary of this revision.
cjdb added a comment.

updates an inaccuracy in the commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/disable-adl.cpp

Index: clang/test/SemaCXX/disable-adl.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/disable-adl.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+// expected-n...@disable-adl.cpp:* 2{{}}
+
+namespace std {
+  struct S1 {};
+  S1 inhibited(S1);
+
+  namespace ranges {
+struct S2 {};
+void hidden(S2);
+int inhibited(S1);
+  }
+}
+
+void test_functions() {
+  hidden(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden'; did you mean 'std::ranges::hidden'?}}
+
+  using namespace std::ranges;
+  int x = inhibited(std::S1{}); // no error
+}
+
+namespace std {
+  template
+  S1 inhibited_template(T);
+
+  namespace ranges {
+template
+void hidden_template(T);
+
+template
+int inhibited_template(T);
+  }
+}
+
+void test_function_templates() {
+  hidden_template(std::ranges::S2{});
+  // expected-error@-1{{use of undeclared identifier 'hidden_template'; did you mean 'std::ranges::hidden_template'?}}
+
+  using namespace std::ranges;
+  int x = inhibited_template(std::S1{});
+}
+
+namespace std {
+  S1 inhibited_mixed(S1);
+
+  namespace ranges {
+template
+int inhibited_mixed(T);
+  }
+}
+
+void test_mixed() {
+  using namespace std::ranges;
+  int x = inhibited_mixed(std::S1{});
+}
+
+// Should be covered by the hidden functions checks, but just to be sure.
+void test_ranges_hidden() {
+  {
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+  {
+using namespace std;
+std::S1 a = inhibited(std::S1{});
+std::S1 b = inhibited_template(std::S1{});
+std::S1 c = inhibited_mixed(std::S1{});
+  }
+}
+
+namespace std {
+  namespace ranges {
+void operator-(S2);
+
+struct hidden_friend_operator {
+  friend void operator-(hidden_friend_operator i, int) {}
+};
+
+struct hidden_friend_swap {
+  friend void swap(hidden_friend_swap, hidden_friend_swap) {}
+};
+  }
+}
+
+void test_friends_and_operators() {
+  -std::ranges::S2{};// no error
+  std::ranges::hidden_friend_operator{} - 1; // no error
+
+  swap(std::ranges::hidden_friend_swap{}, std::ranges::hidden_friend_swap{});
+  // expected-error@-1{{use of undeclared identifier 'swap'}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9452,6 +9452,12 @@
   for (ADLResult::iterator I = Fns.begin(), E = Fns.end(); I != E; ++I) {
 DeclAccessPair FoundDecl = DeclAccessPair::make(*I, AS_none);
 
+// Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 and
+// [algorithms.requirements]/2.
+if ((*I)->isInStdRangesNamespace() &&
+Name.getNameKind() == DeclarationName::NameKind::Identifier)
+  continue;
+
 if (FunctionDecl *FD = dyn_cast(*I)) {
   if (ExplicitTemplateArgs)
 continue;
@@ -9650,7 +9656,7 @@
   const OverloadCandidate &Cand1,
   const OverloadCandidate &Cand2) {
   // FIXME: Per P2113R0 we also need to compare the template parameter lists
-  // when comparing template functions. 
+  // when comparing template functions.
   if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
   Cand2.Function->hasPrototype()) {
 auto *PT1 = cast(Cand1.Function->getFunctionType());
@@ -12828,6 +12834,12 @@
CandidateSet, PartialOverloading,
/*KnownValid*/ true);
 
+  // Functions in 'std::ranges' inhibit ADL per [range.iter.ops]/2 and
+  // [algorithms.requirements]/2.
+  if (!ULE->decls().empty() && ULE->decls_begin()->isInStdRangesNamespace() &&
+  ULE->getName().getNameKind() == DeclarationName::NameKind::Identifier)
+return;
+
   if (ULE->requiresADL())
 AddArgumentDependentLookupCandidates(ULE->getName(), ULE->getExprLoc(),
  Args, ExplicitTemplateArgs,
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -396,6 +396,11 @@
   return DC && DC->isStdNamespace();
 }
 
+bool Decl::isInStdRangesNamespace() const {
+  const DeclContext *DC = getDeclContext();
+  return DC && DC->isStdRangesNamespace();
+}
+
 TranslationUnitDecl *Decl::getTr

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: njames93, nridge, alexfh_, stephenkelly.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Hi,
I think we should add add preposition "of" to code annotation of  
ElseAfterReturnCheck ;-)
Many Thanks Zhouyi

Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129953

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp


Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -263,7 +263,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
@@ -299,7 +299,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor


Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -263,7 +263,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
@@ -299,7 +299,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129954: [CodeGen][inlineasm] assume the flag output of inline asm is boolean value

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: nikic, efriedma, lebedev.ri.
Herald added subscribers: jsji, pengfei, kristof.beyls.
Herald added a project: All.
ychen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC inline asm document says that
"... the general rule is that the output variable must be a scalar
integer, and the value is boolean."

Commit e5c37958f901cc9bec50624dbee85d40143e4bca lowers flag output of
inline asm on X86 with setcc, hence it is guaranteed that the flag
is of boolean value. Clang does not support ARM inline asm flag output
yet so nothing need to be worried about ARM.

See "Flag Output" section at
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#OutputOperands

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129954

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/inline-asm-x86-flag-output.c


Index: clang/test/CodeGen/inline-asm-x86-flag-output.c
===
--- clang/test/CodeGen/inline-asm-x86-flag-output.c
+++ clang/test/CodeGen/inline-asm-x86-flag-output.c
@@ -374,3 +374,18 @@
   : "cx");
   return b;
 }
+
+int test_assume_boolean_flag(long nr, volatile long *addr) {
+  //CHECK-LABEL: @test_assume_boolean_flag
+  //CHECK: = tail call i32 asm "cmp $2,$1", 
"={@cca},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i64* elementtype(i64) %addr, 
i64 %nr)
+  //CHECK: %1 = icmp ult i32 %0, 2
+  //CHECK: call void @llvm.assume(i1 %1)
+  int x;
+  asm("cmp %2,%1"
+  : "=@cca"(x), "=m"(*(volatile long *)(addr))
+  : "r"(nr)
+  : "cc");
+  if (x)
+return 0;
+  return 1;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2343,6 +2343,7 @@
   std::vector ArgElemTypes;
   std::vector Args;
   llvm::BitVector ResultTypeRequiresCast;
+  llvm::BitVector ResultRegIsFlagReg;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2400,6 +2401,9 @@
   ResultRegQualTys.push_back(QTy);
   ResultRegDests.push_back(Dest);
 
+  bool IsFlagReg = llvm::StringRef(OutputConstraint).startswith("{@cc");
+  ResultRegIsFlagReg.push_back(IsFlagReg);
+
   llvm::Type *Ty = ConvertTypeForMem(QTy);
   const bool RequiresCast = Info.allowsRegister() &&
   (getTargetHooks().isScalarizableAsmOperand(*this, Ty) ||
@@ -2717,10 +2721,21 @@
   // ResultRegDests can be also populated by addReturnRegisterOutputs() above,
   // in which case its size may grow.
   assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
+  assert(ResultRegIsFlagReg.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {
 llvm::Value *Tmp = RegResults[i];
 llvm::Type *TruncTy = ResultTruncRegTypes[i];
 
+if ((i < ResultRegIsFlagReg.size()) && ResultRegIsFlagReg[i]) {
+  // Target must guarantee the Value `Tmp` here is lowered to a boolean
+  // value.
+  llvm::Constant *OneVal = llvm::ConstantInt::get(Tmp->getType(), 1);
+  llvm::Value *IsBooleanValue =
+  Builder.CreateCmp(llvm::CmpInst::ICMP_ULE, Tmp, OneVal);
+  llvm::Function *FnAssume = CGM.getIntrinsic(llvm::Intrinsic::assume);
+  Builder.CreateCall(FnAssume, IsBooleanValue);
+}
+
 // If the result type of the LLVM IR asm doesn't match the result type of
 // the expression, do the conversion.
 if (ResultRegTypes[i] != ResultTruncRegTypes[i]) {


Index: clang/test/CodeGen/inline-asm-x86-flag-output.c
===
--- clang/test/CodeGen/inline-asm-x86-flag-output.c
+++ clang/test/CodeGen/inline-asm-x86-flag-output.c
@@ -374,3 +374,18 @@
   : "cx");
   return b;
 }
+
+int test_assume_boolean_flag(long nr, volatile long *addr) {
+  //CHECK-LABEL: @test_assume_boolean_flag
+  //CHECK: = tail call i32 asm "cmp $2,$1", "={@cca},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i64* elementtype(i64) %addr, i64 %nr)
+  //CHECK: %1 = icmp ult i32 %0, 2
+  //CHECK: call void @llvm.assume(i1 %1)
+  int x;
+  asm("cmp %2,%1"
+  : "=@cca"(x), "=m"(*(volatile long *)(addr))
+  : "r"(nr)
+  : "cc");
+  if (x)
+return 0;
+  return 1;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2343,6 +2343,7 @@
   std::vector ArgElemTypes;
   std::vector Args;
   llvm::BitVector ResultTypeRequiresCast;
+  llvm::BitVector ResultRegIsFlagReg;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2400,6 +2401,9 @@
   ResultRegQualTys.push_back(QTy);
   ResultRegDests.push_back(Dest);
 
+  bool IsFlagReg = llvm::StringRef(OutputConstraint).startswith("{@cc");
+  ResultRegIsFlagReg.pu

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[Nathan James via Phabricator via cfe-commits]

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

While you're here could you also put full stops( or periods 🙃) at the end of 
the sentences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129953

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

[PATCH] D129951: [clang] teaches Clang the special ADL rules for functions in std::ranges

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Looking at the output from Clang 14 , I'm 
observing that a binary with 178 function templates is 13% the size of the one 
with 89 function objects. When only one function object is used vs all 178 
function templates, the functions still win out, with the binary being 80% the 
size.

The AST also becomes substantially larger.

I ran `time clang++-13 -c file.cpp -std=c++20 -Oz -DNDEBUG` locally, a hundred 
times, and got the following results:

  # Function objects
  real mean:0.07447
  real median:  0.074
  real stddev:  0.002267268191

  sys mean: 0.01664
  sys median:   0.016
  sys stddev:   0.005569614534

  user mean:0.05785
  user median:  0.057
  user stddev:  0.005848896987

  # Function templates
  real mean:0.06336
  real median:  0.063
  real stddev:  0.002076905235

  sys mean: 0.01701
  sys median:   0.017
  sys stddev:   0.005545942188

  user mean:0.04645
  user median:  0.047
  user stddev:  0.005678908346


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D129953#3657920 , @njames93 wrote:

> While you're here could you also put full stops( or periods 🙃) at the end of 
> the sentences.

Thank Nathan for reviewing my patch
I am here, and am very happy to make further enhancement ;-) But because my 
native language is not English, I don't know where should I put the full stops 
at . 
Isn't  "If the if statement is the last statement of its enclosing statements 
scope, we can pull the decl out of the if statement." constitute a complete 
sentence?

Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129953

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