[PATCH] D119599: Add option to align compound assignments like `+=`

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

HazardyKnusperkeks added a comment.

I think we are going a very good way. Just some steps ahead.




Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > sstwcw wrote:
> > > HazardyKnusperkeks wrote:
> > > > sstwcw wrote:
> > > > > HazardyKnusperkeks wrote:
> > > > > > curdeius wrote:
> > > > > > > I guess it would be complicated to avoid adding an additional 
> > > > > > > space here. I mean, it could be:
> > > > > > > ```
> > > > > > > a  &= 2;
> > > > > > > bbb = 2;
> > > > > > > ```
> > > > > > > And with 3-char operators, there's one more space.
> > > > > > That would be awesome, but it should be an option to turn off or on.
> > > > > > But I think this would really be complicated.
> > > > > I can do it either way. But I thought without the extra space the 
> > > > > formatted code looked ugly, especially when mixing `>>=` and `=`.  
> > > > > Which way do you prefer?
> > > > > 
> > > > > 
> > > > > ```
> > > > > a >>= 2;
> > > > > bbb = 2;
> > > > > ```
> > > > I would prefer an option, to be able to do both.
> > > > I think I would use the variant which is more compact, but can't say 
> > > > for sure until I really have the option and tried it.
> > > You mean like a new entry under `AlignConsecutiveStyle` with the options 
> > > to align the left ends of the operators and to align the equal sign with 
> > > and without padding them to the same length? I don't want the new option 
> > > to be merged with `AlignCompound`, because we are working on adding 
> > > support for a language that uses both `=` and `<=` for regular 
> > > assignments, and maybe someone will want to support a language that has 
> > > both `=` and `:=` in the future.
> > Yeah. Basing on @MyDeveloperDay 's suggestion:
> > ```
> > struct AlignConsecutiveStyle {
> >   bool Enabled;
> >   bool AcrossComments;
> >   bool AcrossEmptyLines;
> >   bool AlignCompound;
> >   bool CompactWhitespace; // This is just a suggestion, I'm open for better 
> > names.
> > };
> > ```
> I added the new option. Moving the old options into the struct turned out to 
> be too much work for me on a weekend. If you can accept the configuration 
> style in the current revision, I will probably move the old options into the 
> struct at some later date. By the way, what is the purpose of 
> `FormatStyle::operator==`? It looks like it does not compare`BraceWrapping`.
I can believe that it is too much for a weekend. But I'd like the complete 
migration in one patch. There are also other align styles, which currently use 
the same enum. And they should keep to use the same type (struct in this case). 
Even though there are then some options which are not (yet?) applicable to them.

We can mark that in the code:
```Whether compound statements ae aligned along with the normal ones. Note this 
currently applies only to assignments: Align i.e. ``+=`` with ``=`



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > sstwcw wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > sstwcw wrote:
> > > > > > HazardyKnusperkeks wrote:
> > > > > > > curdeius wrote:
> > > > > > > > I guess it would be complicated to avoid adding an additional 
> > > > > > > > space here. I mean, it could be:
> > > > > > > > ```
> > > > > > > > a  &= 2;
> > > > > > > > bbb = 2;
> > > > > > > > ```
> > > > > > > > And with 3-char operators, there's one more space.
> > > > > > > That would be awesome, but it should be an option to turn off or 
> > > > > > > on.
> > > > > > > But I think this would really be complicated.
> > > > > > I can do it either way. But I thought without the extra space the 
> > > > > > formatted code looked ugly, especially when mixing `>>=` and `=`.  
> > > > > > Which way do you prefer?
> > > > > > 
> > > > > > 
> > > > > > ```
> > > > > > a >>= 2;
> > > > > > bbb = 2;
> > > > > > ```
> > > > > I would prefer an option, to be able to do both.
> > > > > I think I would use the variant which is more compact, but can't say 
> > > > > for sure until I really have the option and tried it.
> > > > You mean like a new entry under `AlignConsecutiveStyle` with the 
> > > > options to align the left ends of the operators and to align the equal 
> > > > sign with and without padding them to the same length? I don't want the 
> > > > new option to be merged with `AlignCompound`, because we are working on 
> > > > adding support for a language that uses both `=` and `<=` for regular 
> > > > assignments, and maybe someone will want to support a language that has 
> > > > both `=` and `:=` in the future.
> > > Yeah. Basing on @MyDeveloperDay 's suggestion:
> > > ```
> > > struct AlignConsecutiveStyle {
> > >   bool Enabled;
> > >   bool AcrossComments;
> > >   bool AcrossEmptyLines;
> > >   bool AlignCompou

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D112730#3316646 , @carlosgalvezp 
wrote:

> Thanks a lot @tonic for your update and for the work done in the last months 
> ! It's indeed very sad to hear the news. It's a pity that the work will 
> probably be duplicated in many local forks with sub-optimal solutions instead 
> of a centralized, high-quality, peer-reviewed open-source solution.
>
> I'm not very familiar with the terms so I'm not sure I fully understand the 
> reasons why we are advised not to proceed. Could you clarify/elaborate on 
> what it means "the exact patent burden is not disclosed"? I was hoping that 
> the written consent we got from AUTOSAR would be a strong basis to move 
> forward.

In general it would also be good to know if there was any other impediment to 
move forward. This information would be extremely valuable to try and do better 
in the future - I'm thinking on the upcoming MISRA release.

Chris Tapp (chair of MISRA) has been involved in similar discussions 

 in the cfe-dev mailing list, so I think it would be beneficial for him to get 
this feedback and hopefully make the upcoming MISRA release easy to work with 
from this standpoint (if MISRA is interested in an open-source checker 
implementation, of course).

Looking forward to your thoughts :)


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

https://reviews.llvm.org/D112730

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

[clang] e01f624 - [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-14T09:41:24+01:00
New Revision: e01f624adb0ed5d0f1369b0679a64484bac10d02

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

LOG: [clang-format] Fix PointerAlignment within lambdas in a multi-variable 
declaration statement.

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

Also, handle while loops with initializers (C++20) the same way as for loops.

Reviewed By: HazardyKnusperkeks, owenpan

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index a6c6ddcad117b..72f49478d5b4b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3134,7 +3134,15 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
   return false;
 if (getTokenPointerOrReferenceAlignment(Left) == FormatStyle::PAS_Right)
   return false;
-if (Line.IsMultiVariableDeclStmt)
+// FIXME: Setting IsMultiVariableDeclStmt for the whole line is 
error-prone,
+// because it does not take into account nested scopes like lambdas.
+// In multi-variable declaration statements, attach */& to the variable
+// independently of the style. However, avoid doing it if we are in a 
nested
+// scope, e.g. lambda. We still need to special-case statements with
+// initializers.
+if (Line.IsMultiVariableDeclStmt &&
+(Left.NestingLevel == Line.First->NestingLevel ||
+ startsWithInitStatement(Line)))
   return false;
 return Left.Previous && !Left.Previous->isOneOf(
 tok::l_paren, tok::coloncolon, tok::l_square);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index fc2b121faee02..689600c591b26 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2031,6 +2031,10 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   verifyFormat("for (int x = 0; int &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int &c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int &a) { return 0; },\n"
+  " res2 = [](int &a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2068,6 +2072,10 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2121,6 +2129,10 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"



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

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe01f624adb0e: [clang-format] Fix PointerAlignment within 
lambdas in a multi-variable… (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2031,6 +2031,10 @@
   verifyFormat("for (int x = 0; int &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int &c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int &a) { return 0; },\n"
+  " res2 = [](int &a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2068,6 +2072,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2121,6 +2129,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3134,7 +3134,15 @@
   return false;
 if (getTokenPointerOrReferenceAlignment(Left) == FormatStyle::PAS_Right)
   return false;
-if (Line.IsMultiVariableDeclStmt)
+// FIXME: Setting IsMultiVariableDeclStmt for the whole line is 
error-prone,
+// because it does not take into account nested scopes like lambdas.
+// In multi-variable declaration statements, attach */& to the variable
+// independently of the style. However, avoid doing it if we are in a 
nested
+// scope, e.g. lambda. We still need to special-case statements with
+// initializers.
+if (Line.IsMultiVariableDeclStmt &&
+(Left.NestingLevel == Line.First->NestingLevel ||
+ startsWithInitStatement(Line)))
   return false;
 return Left.Previous && !Left.Previous->isOneOf(
 tok::l_paren, tok::coloncolon, tok::l_square);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2031,6 +2031,10 @@
   verifyFormat("for (int x = 0; int &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto &c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int &c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int &a) { return 0; },\n"
+  " res2 = [](int &a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2068,6 +2072,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2121,6 +2129,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotat

[clang] e967d97 - [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-14T09:53:16+01:00
New Revision: e967d97a35a93c883528a9672159edff05f5addb

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

LOG: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

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

Reviewed By: HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index ef8dc2a864a2a..5138c7cd42cc6 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@ BreakableLineCommentSection::BreakableLineCommentSection(
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@ BreakableLineCommentSection::BreakableLineCommentSection(
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This lambda also considers multibyte character that is not handled in
   // functions like isPunctuation provided by CharInfo.
@@ -792,12 +793,11 @@ BreakableLineCommentSection::BreakableLineCommentSection(
   // e.g. from "///" to "//".
   if (i == 0 || OriginalPrefix[i].rtrim(Blanks) !=
 OriginalPrefix[i - 1].rtrim(Blanks)) {
-if (SpacesInPrefix < Style.SpacesInLineCommentPrefix.Minimum &&
-Lines[i].size() > IndentPrefix.size() &&
+if (SpacesInPrefix < Minimum && Lines[i].size() > IndentPrefix.size() 
&&
 !NoSpaceBeforeFirstCommentChar()) {
-  FirstLineSpaceChange =
-  Style.SpacesInLineCommentPrefix.Minimum - SpacesInPrefix;
-} else if (SpacesInPrefix > Style.SpacesInLineCommentPrefix.Maximum) {
+  FirstLineSpaceChange = Minimum - SpacesInPrefix;
+} else if (static_cast(SpacesInPrefix) >
+   Style.SpacesInLineCommentPrefix.Maximum) {
   FirstLineSpaceChange =
   Style.SpacesInLineCommentPrefix.Maximum - SpacesInPrefix;
 } else {
@@ -808,10 +808,9 @@ BreakableLineCommentSection::BreakableLineCommentSection(
   if (Lines[i].size() != IndentPrefix.size()) {
 PrefixSpaceChange[i] = FirstLineSpaceChange;
 
-if (SpacesInPrefix + PrefixSpaceChange[i] <
-Style.SpacesInLineCommentPrefix.Minimum) {
-  PrefixSpaceChange[i] += Style.SpacesInLineCommentPrefix.Minimum -
-  (SpacesInPrefix + PrefixSpaceChange[i]);
+if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
+  PrefixSpaceChange[i] +=
+  Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
 }
 
 assert(Lines[i].size() > IndentPrefix.size());

diff  --git a/clang/unittests/Format/FormatTestComments.cpp 
b/clang/unittests/Format/FormatTestComments.cpp
index dcfd219484fa5..8cdca0eb5900a 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@ TEST_F(FormatTestComments, SpaceAtLineCommentBegin) {
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"



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

[PATCH] D119680: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe967d97a35a9: [clang-format] Fix SpacesInLineCommentPrefix 
deleting tokens. (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119680

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This lambda also considers multibyte character that is not handled in
   // functions like isPunctuation provided by CharInfo.
@@ -792,12 +793,11 @@
   // e.g. from "///" to "//".
   if (i == 0 || OriginalPrefix[i].rtrim(Blanks) !=
 OriginalPrefix[i - 1].rtrim(Blanks)) {
-if (SpacesInPrefix < Style.SpacesInLineCommentPrefix.Minimum &&
-Lines[i].size() > IndentPrefix.size() &&
+if (SpacesInPrefix < Minimum && Lines[i].size() > IndentPrefix.size() 
&&
 !NoSpaceBeforeFirstCommentChar()) {
-  FirstLineSpaceChange =
-  Style.SpacesInLineCommentPrefix.Minimum - SpacesInPrefix;
-} else if (SpacesInPrefix > Style.SpacesInLineCommentPrefix.Maximum) {
+  FirstLineSpaceChange = Minimum - SpacesInPrefix;
+} else if (static_cast(SpacesInPrefix) >
+   Style.SpacesInLineCommentPrefix.Maximum) {
   FirstLineSpaceChange =
   Style.SpacesInLineCommentPrefix.Maximum - SpacesInPrefix;
 } else {
@@ -808,10 +808,9 @@
   if (Lines[i].size() != IndentPrefix.size()) {
 PrefixSpaceChange[i] = FirstLineSpaceChange;
 
-if (SpacesInPrefix + PrefixSpaceChange[i] <
-Style.SpacesInLineCommentPrefix.Minimum) {
-  PrefixSpaceChange[i] += Style.SpacesInLineCommentPrefix.Minimum -
-  (SpacesInPrefix + PrefixSpaceChange[i]);
+if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
+  PrefixSpaceChange[i] +=
+  Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
 }
 
 assert(Lines[i].size() > IndentPrefix.size());


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This la

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D119138

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

[PATCH] D119656: [Driver][DragonFly] -r: imply -nostdlib like GCC

[Fangrui Song via Phabricator via cfe-commits]

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

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119656

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

[PATCH] D118700: Add support to --gcc-toolchain flag for GCC compiled with --enable-version-specific-runtime-libs.

[Fangrui Song via Phabricator via cfe-commits]

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

The library path (`lib/gcc/$triple/$libdir`) still looks weird and I think I 
don't understand the GCC rationale well.
If you still think it's the right thing the do, let's do it.

For driver tests involving search paths, it's usually worth checking whether 
the test passes with `-DCLANG_DEFAULT_RTLIB=compiler-rt 
-DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++`.
Some groups use this configuration and expect the tests to pass in this mode.


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

https://reviews.llvm.org/D118700

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

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

[Nathan Ridge via Phabricator via cfe-commits]

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

Very neat! A few minor suggestions but generally looks good.




Comment at: clang-tools-extra/clangd/AST.cpp:519
+  if (Spec->getTemplateSpecializationKind() != TSK_ImplicitInstantiation)
+continue;
+  // Find the type for this specialization.

Should we `break` in this case? Otherwise there could be an explicit 
specialization with a second type



Comment at: clang-tools-extra/clangd/AST.cpp:545
+  // TemplateTypeParmTypes for implicit TTPs, instead of AutoTypes.
+  // Also we don't look very hard, just stripping const, references, pointers.
+  static TemplateTypeParmTypeLoc findContainedAutoTTPLoc(TypeLoc TL) {

Maybe call out an example like `vector` as a future nice-to-handle?



Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+  StartsWith("fail: Could not deduce"));
+  EXPECT_EQ(apply("auto X = [](^auto){return 0;}; int Y = X(42);"),
+"auto X = [](int){return 0;}; int Y = X(42);");

Maybe add a conflicting-deduction case? (I know the GetDeducedType test already 
has one, but here it would be especially wrong.)

Also, should we perhaps disable this in header files (maybe excepting 
function-local symbols), since an including source file could have additional 
instantiations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119537

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

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM. My naming suggestion is not binding. I have no strong opinion on this, 
but a shorter name would get my 👍 :).
So please sync with other reviewers.




Comment at: clang/docs/ClangFormatStyleOptions.rst:4007
 
+  * ``bool AfterRequiresKeywordInRequiresClause`` If ``true``, put space 
between requires keyword in a requires clause and
+opening parentheses, if there is one.

Wouldn't `AfterRequiresInClause`/`AfterRequiresInExpression` be meaningful 
enough?


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

https://reviews.llvm.org/D113369

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

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/CodeGenCXX/static-variable-in-module.cpp:2-8
+// RUN: mkdir %t
+// RUN: echo "struct S { S(); };" >> %t/foo.h
+// RUN: echo "static S s = S();" >> %t/foo.h
+// RUN: %clang -std=c++20 -I%t %s -S -emit-llvm -o - | FileCheck %s
+module;
+#include "foo.h"
+export module m;

ChuanqiXu wrote:
> urnathan wrote:
> > rather than generate a foo.h file, why not (ab)use the preprocessor with 
> > internal line directives?
> > 
> > ```
> > module;
> > # 3 __FILE__ 1 // use the next physical line number here (and below)
> > struct S { S(); };
> > static S s = S();
> > # 6 "" 2
> > export module m;
> > ...
> > ```
> Yeah, the form is useful when we need to add expected-* diagnostic message to 
> GMF. But I feel it is a little bit hacker. I guess the form mimics looks like 
> user more wouldn't be worse personally.
Ok, I admit this is really helpful and time saving : ) (Now all my new test 
cases would be wrote in this form)


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

https://reviews.llvm.org/D119409

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

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

I gave this patch a test against our downstream code, and found that it doesn't 
stop me needing the change I suggested in D119199 
: in `clang/tools/driver/CMakeLists.txt`, I 
had to change

  if(CLANG_PLUGIN_SUPPORT)
export_executable_symbols_for_plugins(clang)
  endif()
  ``` so that the if statement reads
  ```if(CLANG_PLUGIN_SUPPORT OR LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)

because otherwise, the clang/examples builds will still try to link against the 
clang executable target, which needs its symbols to have been exported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119591

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

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Well, it doesn't work with GCC either, that's why I don't care much about this 
change. It just attempts to legalize a user bug (using a linker option directly 
as a compiler flag). But I don't care enough to object either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119655

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I'd really want to see simpler tests and everything put into a single 
`AlignConsecutiveAssignment` option.




Comment at: clang/include/clang/Format/Format.h:163
+  /// \endcode
+  bool AlignCompoundAssignments;
+

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > This option is not independent of `AlignConsecutiveAssignments` is it? this 
> > will cause confusion when people turn it on without turning on 
> > `AlignConsecutiveAssignments` 
> > 
> > Options have a lifecycle we have noticed
> > 
> > 1) They start as bool
> > 2) They become enums
> > 3) They then end up as a struct of bool
> > 4) Sometimes the struct becomes of enums
> > 
> > Whilst I like what you are doing here I fear we will get bugs where people 
> > say I set AlignCompoundAssignments: true but its doesn't work.
> > 
> > `AlignConsecutiveAssignments` is already gone too far on the enum, it 
> > should be a struct
> > 
> > so rather than
> > 
> > ```
> > enum AlignConsecutiveStyle {
> > ACS_None,
> > ACS_Consecutive,
> > ACS_AcrossEmptyLines,
> > ACS_AcrossComments,
> > ACS_AcrossEmptyLinesAndComments
> >   };
> > AlignConsecutiveStyle  AlignConsecutiveAssignments ;
> > 
> > ```
> > it should be perhaps
> > 
> > ```
> > struct {
> > bool AcrossEmptyLines,
> > bool AcrossComments,
> > bool AlignCompound
> > } AlignConsecutiveStyle;
> > 
> > AlignConsecutiveStyle  AlignConsecutiveAssignments;
> > ```
> > 
> > in the .clang-format file it would become
> > 
> > ```
> > AlignConsecutiveAssignments: Custom
> > AlignConsecutiveAssignmentsOptions:
> >AcrossEmptyLines: true
> >AcrossComments: true
> >AlignCompound: false
> > ```
> > 
> > I realise this would be a much bigger piece of work (in the config) but the 
> > existing options would map to the new options, and then we have a structure 
> > for which have space for future expansion.
> > 
> > The introduction of a dependent option in my view triggers the need for 
> > that config change? @HazardyKnusperkeks 
> >  you thoughts, I know we've done this before, what  do you think in this 
> > case? 
> > 
> > 
> > 
> I would even go further (and that I already told the last time). Drop the 
> ``Custom`` and map the old enums to the struct when parsing, so no new option.
> I would even go further (and that I already told the last time). Drop the 
> ``Custom`` and map the old enums to the struct when parsing, so no new option.

:+1:

That's my preference too. Having both `AlignConsecutiveAssignments` and 
`AlignConsecutiveAssignmentsOptions` is error-prone.



Comment at: clang/lib/Format/WhitespaceManager.cpp:475
+const FormatStyle::AlignConsecutiveStyle &ACS = FormatStyle::ACS_None,
+bool RightJustify = false, bool PadAnchors = false) {
+  // We arrange each line in 3 parts. The operator to be aligned (the

HazardyKnusperkeks wrote:
> This shouldn't be necessary if the options are put into 
> `AlignConsecutiveStyle`.
Yeah, you can get `PadAnchors` from the `Style`. Maybe `RightJustify` too, nope?



Comment at: clang/unittests/Format/FormatTest.cpp:16400-16410
+  verifyFormat("aa <= 5;\n"
+   "a   = 5;\n"
+   "bcd = 5;\n"
+   "ghtyf   = 5;\n"
+   "dvfvdb  = 5;\n"
+   "a   = 5;\n"
+   "vdsvsv  = 5;\n"

No need for long tests. Please simplify other too.



Comment at: clang/unittests/Format/FormatTest.cpp:16422-16432
+  verifyFormat("a  &= 5;\n"
+   "bcd*= 5;\n"
+   "ghtyf >>= 5;\n"
+   "dvfvdb -= 5;\n"
+   "a  /= 5;\n"
+   "aa <= 5;\n"
+   "vdsvsv %= 5;\n"

Do you really need to do such big test cases?
I'd rather see small ones like:

test that `<=` is not treated as a compound assignment:
```
aa &= 5;
b <= 10;
c = 15;
```
etc.



Comment at: clang/unittests/Format/FormatTest.cpp:16433
+   Alignment);
+  Alignment.AlignConsecutiveAssignmentsOptions.PadOperators = true;
+  verifyFormat("aa <= 5;\n"

I'd like to see a test that verifies what you put in the documentation:
```
a   >>= 2;
bbb   = 2;
```

but also the other way round:
```
aaa >>= 2;
b = 2;
```
and a mix of 1-, 2- and 3-char operators as well.



Comment at: clang/unittests/Format/FormatTest.cpp:16564-16565
Alignment);
-  verifyFormat("a &= 5;\n"
+  verifyFormat("aa <= 5;\n"
+   "a &= 5;\n"
"bcd *= 5;\n"

It should be a separate case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

___
cfe-commits mailing list
cfe-commits@lis

[PATCH] D119599: Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@sstwcw, would it help if I created a parent revision that only splits the 
current enum-based option into a struct, so that you only add compound ops and 
padding?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119701: [clangd] Re-enable clang-tidy's nolint blocks

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

The previous inefficient implementation is polished.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119701

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -477,9 +477,8 @@
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
   // NOLINTBEGIN
-  // FIXME: re-enable when NOLINTBEGIN suppresss block is enabled in 
clangd.
-  // double x = BAD2;
-  // double y = BAD2;
+  double x = BAD2;
+  double y = BAD2;
   // NOLINTEND
 
   // verify no crashes on unmatched nolints.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -473,7 +473,7 @@
   if (IsInsideMainFile && CTContext->shouldSuppressDiagnostic(
   DiagLevel, Info, TidySuppressedErrors,
   /*AllowIO=*/false,
-  /*EnableNolintBlocks=*/false)) {
+  /*EnableNolintBlocks=*/true)) {
 // FIXME: should we expose the suppression error (invalid use of
 // NOLINT comments)?
 return DiagnosticsEngine::Ignored;


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -477,9 +477,8 @@
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
   // NOLINTBEGIN
-  // FIXME: re-enable when NOLINTBEGIN suppresss block is enabled in clangd.
-  // double x = BAD2;
-  // double y = BAD2;
+  double x = BAD2;
+  double y = BAD2;
   // NOLINTEND
 
   // verify no crashes on unmatched nolints.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -473,7 +473,7 @@
   if (IsInsideMainFile && CTContext->shouldSuppressDiagnostic(
   DiagLevel, Info, TidySuppressedErrors,
   /*AllowIO=*/false,
-  /*EnableNolintBlocks=*/false)) {
+  /*EnableNolintBlocks=*/true)) {
 // FIXME: should we expose the suppression error (invalid use of
 // NOLINT comments)?
 return DiagnosticsEngine::Ignored;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] fc84ebf - [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers`

[Haojian Wu via cfe-commits]

Author: Evgeny Shulgin
Date: 2022-02-14T11:38:27+01:00
New Revision: fc84ebfff3a3dd373129f465f2ad5763ed4423c6

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

LOG: [clang-tidy] Ignore variable template partial specializations in 
`misc-definitions-in-headers`

Variable template partial specializations are inline and can't lead
to ODR-violations. The checker now ignores them.

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

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
index 411d6db582436..9a7a7e108b120 100644
--- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -149,6 +149,9 @@ void DefinitionsInHeadersCheck::check(const 
MatchFinder::MatchResult &Result) {
 // Ignore inline variables.
 if (VD->isInline())
   return;
+// Ignore partial specializations.
+if (isa(VD))
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
index c2a4a81ae3e16..f57cc2c256fef 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
@@ -193,6 +193,16 @@ const int f12() { return 0; }
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template 
specialization 'f12' defined in a header file;
 // CHECK-FIXES: inline const int f12() { return 0; }
 
+template 
+constexpr bool f13 = false;
+
+template 
+constexpr bool f13 = true; // OK: template partial specialization
+
+template <>
+constexpr bool f13 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: variable 'f13' defined 
in a header file;
+
 int main() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'main' defined in a 
header file;
 // CHECK-FIXES: {{^}}int main() {



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

[PATCH] D119098: [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers`

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc84ebfff3a3: [clang-tidy] Ignore variable template partial 
specializations in `misc… (authored by Izaron, committed by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119098

Files:
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
@@ -193,6 +193,16 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template 
specialization 'f12' defined in a header file;
 // CHECK-FIXES: inline const int f12() { return 0; }
 
+template 
+constexpr bool f13 = false;
+
+template 
+constexpr bool f13 = true; // OK: template partial specialization
+
+template <>
+constexpr bool f13 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: variable 'f13' defined 
in a header file;
+
 int main() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'main' defined in a 
header file;
 // CHECK-FIXES: {{^}}int main() {
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -149,6 +149,9 @@
 // Ignore inline variables.
 if (VD->isInline())
   return;
+// Ignore partial specializations.
+if (isa(VD))
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "


Index: clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
@@ -193,6 +193,16 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template specialization 'f12' defined in a header file;
 // CHECK-FIXES: inline const int f12() { return 0; }
 
+template 
+constexpr bool f13 = false;
+
+template 
+constexpr bool f13 = true; // OK: template partial specialization
+
+template <>
+constexpr bool f13 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: variable 'f13' defined in a header file;
+
 int main() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'main' defined in a header file;
 // CHECK-FIXES: {{^}}int main() {
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -149,6 +149,9 @@
 // Ignore inline variables.
 if (VD->isInline())
   return;
+// Ignore partial specializations.
+if (isa(VD))
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

Except for three nits. LGTM.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1505
+  /// valid in the condition block (i.e., defined in the preheader) and is
+  /// interpreted as an unsigned integer.
+  void setTripCount(Value *TripCount);

Nit: integer -> 64-bit integer?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1676
+  Value *SrcLoc = getOrCreateIdent(getOrCreateSrcLocStr(DL));
+  Value *ThreadNum = getOrCreateThreadID(SrcLoc);
+  Constant *SchedulingType = ConstantInt::get(

peixin wrote:
> Can you move "Value *ThreadNum = getOrCreateThreadID(SrcLoc);" after 
> "Builder.CreateStore(One, PStride);" in order that the 
> "kmpc_global_thread_num" call is right before the "kmpc_static_init" call to 
> keep consistence with others?
This comment is not addressed.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1765
+  switch (SchedKind) {
+  case llvm::omp::ScheduleKind ::OMP_SCHEDULE_Default:
+assert(!ChunkSize && "No chunk size with default schedule (which for clang 
"

peixin wrote:
> Please remove the space between "ScheduleKind" and "OMP_SCHEDULE_Default"? 
> Also for the following switch cases.
The extra space is not removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw added a comment.

@curdeius That would be great.




Comment at: clang/include/clang/Format/Format.h:163
+  /// \endcode
+  bool AlignCompoundAssignments;
+

curdeius wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > This option is not independent of `AlignConsecutiveAssignments` is it? 
> > > this will cause confusion when people turn it on without turning on 
> > > `AlignConsecutiveAssignments` 
> > > 
> > > Options have a lifecycle we have noticed
> > > 
> > > 1) They start as bool
> > > 2) They become enums
> > > 3) They then end up as a struct of bool
> > > 4) Sometimes the struct becomes of enums
> > > 
> > > Whilst I like what you are doing here I fear we will get bugs where 
> > > people say I set AlignCompoundAssignments: true but its doesn't work.
> > > 
> > > `AlignConsecutiveAssignments` is already gone too far on the enum, it 
> > > should be a struct
> > > 
> > > so rather than
> > > 
> > > ```
> > > enum AlignConsecutiveStyle {
> > > ACS_None,
> > > ACS_Consecutive,
> > > ACS_AcrossEmptyLines,
> > > ACS_AcrossComments,
> > > ACS_AcrossEmptyLinesAndComments
> > >   };
> > > AlignConsecutiveStyle  AlignConsecutiveAssignments ;
> > > 
> > > ```
> > > it should be perhaps
> > > 
> > > ```
> > > struct {
> > > bool AcrossEmptyLines,
> > > bool AcrossComments,
> > > bool AlignCompound
> > > } AlignConsecutiveStyle;
> > > 
> > > AlignConsecutiveStyle  AlignConsecutiveAssignments;
> > > ```
> > > 
> > > in the .clang-format file it would become
> > > 
> > > ```
> > > AlignConsecutiveAssignments: Custom
> > > AlignConsecutiveAssignmentsOptions:
> > >AcrossEmptyLines: true
> > >AcrossComments: true
> > >AlignCompound: false
> > > ```
> > > 
> > > I realise this would be a much bigger piece of work (in the config) but 
> > > the existing options would map to the new options, and then we have a 
> > > structure for which have space for future expansion.
> > > 
> > > The introduction of a dependent option in my view triggers the need for 
> > > that config change? @HazardyKnusperkeks 
> > >  you thoughts, I know we've done this before, what  do you think in this 
> > > case? 
> > > 
> > > 
> > > 
> > I would even go further (and that I already told the last time). Drop the 
> > ``Custom`` and map the old enums to the struct when parsing, so no new 
> > option.
> > I would even go further (and that I already told the last time). Drop the 
> > ``Custom`` and map the old enums to the struct when parsing, so no new 
> > option.
> 
> :+1:
> 
> That's my preference too. Having both `AlignConsecutiveAssignments` and 
> `AlignConsecutiveAssignmentsOptions` is error-prone.
About `AlignConsecutiveAssignments` and `AlignConsecutiveAssignmentsOptions`. 
Based on the current YAML infrastructure, it does not seem possible to support 
both enum and struct under one name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local 
)?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local 
)?}}constant i32 3,

ChuanqiXu wrote:
> urnathan wrote:
> > I don;t think this is correct.  That should still be a linkonce odr, 
> > otherwise you'll get conflicts with other module implementation units.
> It is still linkonce_odr in the module it get defined. See the new added test 
> case: inline-variable-in-module.cpp for example. The attribute 
> `available_externally` is equivalent to external from the perspective of 
> linker. See https://llvm.org/docs/LangRef.html#linkage-types. According to 
> [dcl.inline]p7, inline variable attached to named module should be defined in 
> that domain. Note that the variable attached to global module fragment and 
> private module fragment shouldn't be accessed outside the module, so it 
> implies that all the variable defined in the module could only be defined in 
> the module unit itself.
There's a couple of issues with this.  module.cppm is emitting a (linkonce) 
definition of inlne_var_exported, but only because it itself is ODR-using that 
variable.  If you take out the ODR-use in noninline_exported, there is no 
longer a symbol emitted.

But, even if you forced inline vars to be emitted in their defining-module's 
interface unit, that would be an ABI change.  inline vars are emitted whereever 
ODR-used.  They have no fixed home TU.  Now, we could alter the ABI and allow 
interface units to define a home location for inline vars and similar entities 
(eg, vtables for keyless classes).  But we'd need buy-in from other compilers 
to do that.

FWIW such a discussion did come up early in implementing modules-ts, but we 
decided there was enough going on just getting the TS implemented.  I'm fine 
with revisiting that, but it is a more significant change.

And it wouldn't apply to (eg) templated variables, which of course could be 
instantiated anywhere.



Comment at: clang/test/CodeGenCXX/static-variable-in-module.cpp:2-8
+// RUN: mkdir %t
+// RUN: echo "struct S { S(); };" >> %t/foo.h
+// RUN: echo "static S s = S();" >> %t/foo.h
+// RUN: %clang -std=c++20 -I%t %s -S -emit-llvm -o - | FileCheck %s
+module;
+#include "foo.h"
+export module m;

ChuanqiXu wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > rather than generate a foo.h file, why not (ab)use the preprocessor with 
> > > internal line directives?
> > > 
> > > ```
> > > module;
> > > # 3 __FILE__ 1 // use the next physical line number here (and below)
> > > struct S { S(); };
> > > static S s = S();
> > > # 6 "" 2
> > > export module m;
> > > ...
> > > ```
> > Yeah, the form is useful when we need to add expected-* diagnostic message 
> > to GMF. But I feel it is a little bit hacker. I guess the form mimics looks 
> > like user more wouldn't be worse personally.
> Ok, I admit this is really helpful and time saving : ) (Now all my new test 
> cases would be wrote in this form)
The other thing you can do, which I've seen elsewhere, is an Input subdirectory 
and put the #include file there.  I prefer the direct encoding, 'cos that's 
less indirection when trying to understand the testcase :)


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

https://reviews.llvm.org/D119409

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

[clang] c72fdad - [clang-format] Reformat. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-14T14:05:05+01:00
New Revision: c72fdad71b6a9e5ca39fdcc7cb8bd476ecc5bbd7

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

LOG: [clang-format] Reformat. NFC.

Added: 


Modified: 
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 689600c591b26..d230964cd2860 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8412,8 +8412,11 @@ TEST_F(FormatTest, DeclarationsOfMultipleVariables) {
   verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
   verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
Style);
-  verifyFormat("switch (int *p, *q; p != q) {\n  default:\nbreak;\n}", 
Style);
-  verifyFormat("/*comment*/ switch (int *p, *q; p != q) {\n  default:\n
break;\n}", Style);
+  verifyFormat("switch (int *p, *q; p != q) {\n  default:\nbreak;\n}",
+   Style);
+  verifyFormat(
+  "/*comment*/ switch (int *p, *q; p != q) {\n  default:\nbreak;\n}",
+  Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {



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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I tried

  cmake -GNinja -DCLANG_ENABLE_STATIC_ANALYZER=OFF 
-DLLVM_ENABLE_PROJECTS="llvm;clang;clang-tools-extra" -DCLANG_ENABLE_ARCMT=OFF 
../llvm

and verified that the breakage occurs at 
6d7b3d6b3a8dbd62650b6c3dae1fe904a8ae9048 
, doesn't 
occur on origin/main, but breaks again if I apply this patch on top of 
origin/main. So it seems nothing has changed. I also double checked that the 
error at 6d7b3d6b3a8dbd62650b6c3dae1fe904a8ae9048 
 happens 
both on my Linux and Windows box.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, jyknight, rsmith.
aaron.ballman requested review of this revision.
Herald added a project: clang.

This adds support for 
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf, which was adopted at 
the Feb 2022 WG14 meeting. That paper adds `[[noreturn]]` and `[[_Noreturn]]` 
to the list of supported attributes in C2x. These attributes have the same 
semantics as the `[[noreturn]]` attribute in C++.

The `[[_Noreturn]]` attribute was added as a deprecated feature so that 
translation units which include `` do not get an error on use of 
`[[noreturn]]` because the macro expands to `_Noreturn`. Users can use 
`-Wno-deprecated-attributes` to silence the diagnostic.

Use of `` or the `noreturn` macro were both deprecated. Users 
can define the `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` macro to suppress the 
deprecation diagnostics coming from the header file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119707

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-lambda.cpp
  clang/test/Sema/c2x-noreturn.c

Index: clang/test/Sema/c2x-noreturn.c
===
--- /dev/null
+++ clang/test/Sema/c2x-noreturn.c
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -verify=all,c2x -std=c2x -fsyntax-only %s
+// RUN: %clang_cc1 -verify=all -std=c17 -fdouble-square-bracket-attributes -fsyntax-only %s
+// RUN: %clang_cc1 -verify=none -Wno-deprecated-attributes -D_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS -std=c2x -fsyntax-only %s
+// RUN: %clang_cc1 -verify=none -Wno-deprecated-attributes -D_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS -std=c17 -fdouble-square-bracket-attributes -fsyntax-only %s
+// none-no-diagnostics
+
+// Test preprocessor functionality.
+#if !__has_c_attribute(noreturn)
+#error "No noreturn attribute support?"
+#endif
+
+#if !__has_c_attribute(_Noreturn)
+#error "No _Noreturn attribute support?"
+#endif
+
+#if __has_c_attribute(noreturn) != __has_c_attribute(_Noreturn) || \
+__has_c_attribute(noreturn) != 202202L
+#error "Wrong value for __has_c_attribute(noreturn)"
+#endif
+
+// If we're testings with deprecations disabled, we don't care about testing
+// the scenarios that trigger errors because we're only interested in the
+// deprecation warning behaviors.
+#ifndef _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS
+// Test that the attribute accepts no args, applies to the correct subject, etc.
+[[noreturn(12)]] void func4(void); // all-error {{attribute 'noreturn' cannot have an argument list}}
+[[noreturn]] int not_a_func; // all-error {{'noreturn' attribute only applies to functions}}
+void func5(void) [[noreturn]]; // all-error {{'noreturn' attribute cannot be applied to types}}
+#endif
+
+_Noreturn void func1(void); // ok, using the function specifier
+[[noreturn]] void func2(void);
+
+// This is deprecated because it's only for compatibility with inclusion of the
+//  header where the noreturn macro expands to _Noreturn.
+[[_Noreturn]] void func3(void); // all-warning {{the '[[_Noreturn]]' attribute spelling is deprecated in C2x; use '[[noreturn]]' instead}}
+
+// Test the behavior of including 
+#include  // c2x-warning@stdnoreturn.h:* {{the '' header is deprecated in C2x}}
+
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+   // c2x-warning {{macro 'noreturn' has been marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 'deprecated' here}}
+
+void func7 [[noreturn]] (void); // all-warning {{the '[[_Noreturn]]' attribute spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+// c2x-warning {{macro 'noreturn' has been marked as deprecated}} \
+// c2x-note@stdnoreturn.h:* {{macro marked 'deprecated' here}}
+
+noreturn void func8(void); // c2x-warning {{macro 'noreturn' has been marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 'deprecated' here}}
+
+// Ensure the function specifier form still works
+void noreturn func9(void); // c2x-warning {{macro 'noreturn' has been marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 'deprecated' here}}
+
+// Test preprocessor functionality after including .
+#if !__has_c_attribute(noreturn) // c2x-warning {{macro 'noreturn' has been marked as deprecated}} \
+ // c2x-note@stdnoreturn.h:* {{macro marked 'deprecated' here}}
+#error "No noreturn attribute support?"
+#endif
+
+#if !__has_c_attribute(_Noreturn)
+#error "No _Noreturn attribute support?"
+#endif
Index: clang/test/AST/ast-dump-lambda.cpp
===

[PATCH] D119574: [clang] Expose -fprofile-use in clang-cl

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D119574#3316216 , @paulkirth wrote:

> Hi,
>
> I think we hit a test failure due to this patch in Fuchsia's Clang canary for 
> Windows. We've had a lot of breakages on Windows today, so this was hidden 
> until 0574b5fc 
>  landed.
>
> The failing test is: Clang :: Driver/cl-options.c
>
> The failing bot can be found here: 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8822483567668224209/overview

I can't seem to find the actual error in that output. What's the exact failure 
you're seeing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119574

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

[clang] 76032b0 - Check for the overloadable attribute in all the appropriate syntactic locations

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-02-14T08:54:21-05:00
New Revision: 76032b0e3f58d4abe8d00ac61ff1b2044e076ba7

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

LOG: Check for the overloadable attribute in all the appropriate syntactic 
locations

When forming the function type from a declarator, we look for an
overloadable attribute before issuing a diagnostic in C about a
function signature containing only  When the attribute is present,
we allow such a declaration for compatibility with the overloading
rules in C++. However, we were not looking for the attribute in all of
the places it is legal to write it on a declarator and so we only
accepted the signature in some forms and incorrectly rejected the
signature in others.

We now check for the attribute preceding the declarator instead of only
being applied to the declarator directly.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaType.cpp
clang/test/Sema/overloadable.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4a63959412678..26860d3118020 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -77,6 +77,9 @@ Attribute Changes in Clang
 
 - Added support for parameter pack expansion in `clang::annotate`.
 
+- The ``overloadable`` attribute can now be written in all of the syntactic
+  locations a declaration attribute may appear. Fixes PR53805.
+
 Windows Support
 ---
 

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index ab47e9f03eaf2..74969749e54ae 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5237,7 +5237,9 @@ static TypeSourceInfo 
*GetFullTypeForDeclarator(TypeProcessingState &state,
 // function is marked with the "overloadable" attribute. Scan
 // for this attribute now.
 if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus)
-  if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable))
+  if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable) &&
+  !D.getDeclSpec().getAttributes().hasAttribute(
+  ParsedAttr::AT_Overloadable))
 S.Diag(FTI.getEllipsisLoc(), diag::err_ellipsis_first_param);
 
 if (FTI.NumParams && FTI.Params[0].Param == nullptr) {

diff  --git a/clang/test/Sema/overloadable.c b/clang/test/Sema/overloadable.c
index b520d76f9e7e8..e9bbdeab7f66c 100644
--- a/clang/test/Sema/overloadable.c
+++ b/clang/test/Sema/overloadable.c
@@ -248,3 +248,14 @@ void typeof_function_is_not_a_pointer() {
   // if take_fn is passed a void (**)(void *), we'll get a warning.
   take_fn(fn);
 }
+
+// PR53805
+// We previously failed to consider the attribute being written before the
+// declaration when considering whether to allow a variadic signature with no
+// other parameters, and so we handled these cases 
diff erently.
+__attribute__((overloadable)) void can_overload_1(...); // ok, was previously 
rejected
+void can_overload_2(...) __attribute__((overloadable)); // ok
+[[clang::overloadable]] void can_overload_3(...);   // ok, was previously 
rejected
+void can_overload_4 [[clang::overloadable]] (...);  // ok
+void cannot_overload(...) [[clang::overloadable]];  // expected-error 
{{ISO C requires a named parameter before '...'}} \
+// expected-error 
{{'overloadable' attribute cannot be applied to types}}



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

[PATCH] D119664: Check for the overloadable attribute in all the appropriate syntactic locations

[Aaron Ballman via Phabricator via cfe-commits]

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

Thanks! I've commit in 76032b0e3f58d4abe8d00ac61ff1b2044e076ba7 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119664

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

[clang] f208644 - [CGBuilder] Remove CreateBitCast() method

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-02-14T15:06:04+01:00
New Revision: f208644ed3618fb1db195adbd35ae0acf2819f23

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

LOG: [CGBuilder] Remove CreateBitCast() method

Use CreateElementBitCast() instead, or don't work on Address
where not necessary.

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 1f1de3df857c9..9696c9d3e4d2a 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1265,8 +1265,7 @@ Address CodeGenFunction::GetAddrOfBlockDecl(const VarDecl 
*variable) {
 auto &byrefInfo = getBlockByrefInfo(variable);
 addr = Address(Builder.CreateLoad(addr), byrefInfo.ByrefAlignment);
 
-auto byrefPointerType = llvm::PointerType::get(byrefInfo.Type, 0);
-addr = Builder.CreateBitCast(addr, byrefPointerType, "byref.addr");
+addr = Builder.CreateElementBitCast(addr, byrefInfo.Type, "byref.addr");
 
 addr = emitBlockByrefAddress(addr, byrefInfo, /*follow*/ true,
  variable->getName());
@@ -1940,15 +1939,15 @@ CodeGenFunction::GenerateCopyHelperFunction(const 
CGBlockInfo &blockInfo) {
   StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args);
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
-  llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
-
   Address src = GetAddrOfLocalVar(&SrcDecl);
   src = Address(Builder.CreateLoad(src), blockInfo.BlockAlign);
-  src = Builder.CreateBitCast(src, structPtrTy, "block.source");
+  src = Builder.CreateElementBitCast(src, blockInfo.StructureType,
+ "block.source");
 
   Address dst = GetAddrOfLocalVar(&DstDecl);
   dst = Address(Builder.CreateLoad(dst), blockInfo.BlockAlign);
-  dst = Builder.CreateBitCast(dst, structPtrTy, "block.dest");
+  dst =
+  Builder.CreateElementBitCast(dst, blockInfo.StructureType, "block.dest");
 
   for (auto &capture : blockInfo.SortedCaptures) {
 if (capture.isConstantOrTrivial())
@@ -2130,11 +2129,9 @@ CodeGenFunction::GenerateDestroyHelperFunction(const 
CGBlockInfo &blockInfo) {
 
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
-  llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
-
   Address src = GetAddrOfLocalVar(&SrcDecl);
   src = Address(Builder.CreateLoad(src), blockInfo.BlockAlign);
-  src = Builder.CreateBitCast(src, structPtrTy, "block");
+  src = Builder.CreateElementBitCast(src, blockInfo.StructureType, "block");
 
   CodeGenFunction::RunCleanupsScope cleanups(*this);
 
@@ -2171,9 +2168,9 @@ class ObjectByrefHelpers final : public BlockByrefHelpers 
{
 
   void emitCopy(CodeGenFunction &CGF, Address destField,
 Address srcField) override {
-destField = CGF.Builder.CreateBitCast(destField, CGF.VoidPtrTy);
+destField = CGF.Builder.CreateElementBitCast(destField, CGF.Int8Ty);
 
-srcField = CGF.Builder.CreateBitCast(srcField, CGF.VoidPtrPtrTy);
+srcField = CGF.Builder.CreateElementBitCast(srcField, CGF.Int8PtrTy);
 llvm::Value *srcValue = CGF.Builder.CreateLoad(srcField);
 
 unsigned flags = (Flags | BLOCK_BYREF_CALLER).getBitMask();
@@ -2186,7 +2183,7 @@ class ObjectByrefHelpers final : public BlockByrefHelpers 
{
   }
 
   void emitDispose(CodeGenFunction &CGF, Address field) override {
-field = CGF.Builder.CreateBitCast(field, CGF.Int8PtrTy->getPointerTo(0));
+field = CGF.Builder.CreateElementBitCast(field, CGF.Int8PtrTy);
 llvm::Value *value = CGF.Builder.CreateLoad(field);
 
 CGF.BuildBlockRelease(value, Flags | BLOCK_BYREF_CALLER, false);
@@ -2376,13 +2373,11 @@ generateByrefCopyHelper(CodeGenFunction &CGF, const 
BlockByrefInfo &byrefInfo,
   auto AL = ApplyDebugLocation::CreateArtificial(CGF);
 
   if (generator.needsCopy()) {
-llvm::Type *byrefPtrType = byrefInfo.Type->getPointerTo(0);
-
 // dst->x
 Address destField = CGF.GetAddrOfLocalVar(&Dst);
 destField = Address(CGF.Builder.CreateLoad(destField),
 byrefInfo.ByrefAlignment);
-destField = CGF.Builder.CreateBitCast(destField, byrefPtrType);
+destField = CGF.Builder.CreateElementBitCast(destField, byrefInfo.Type);
 destField = CGF.emitBlockByrefAddress(destField, byrefInfo, 

[PATCH] D119708: [clang][lex] Remove `PPCallbacks::FileNotFound()`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
Herald added subscribers: usaxena95, shchenz, kadircet, arphaman, kbarton, 
nemanjai.
jansvoboda11 requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

The purpose of the `FileNotFound` preprocessor callback was to add the ability 
to recover from failed header lookups. This was to support downstream project.

However, injecting additional search path while performing header search can 
invalidate currently used iterators/references to `DirectoryLookup` in 
`Preprocessor` and `HeaderSearch`.

The downstream project ended up maintaining a separate patch to further tweak 
the functionality. Since we don't have any upstream users nor open source 
downstream users, I'd like to remove this callback for good to prevent future 
misuse. I doubt there are any actual downstream users, since the functionality 
is definitely broken at the moment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119708

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.h
  clang/include/clang/Lex/PPCallbacks.h
  clang/lib/Lex/PPDirectives.cpp

Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1845,28 +1845,6 @@
   if (File)
 return File;
 
-  if (Callbacks) {
-// Give the clients a chance to recover.
-SmallString<128> RecoveryPath;
-if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
-  if (auto DE = FileMgr.getOptionalDirectoryRef(RecoveryPath)) {
-// Add the recovery path to the list of search paths.
-DirectoryLookup DL(*DE, SrcMgr::C_User, false);
-HeaderInfo.AddSearchPath(DL, isAngled);
-
-// Try the lookup again, skipping the cache.
-Optional File = LookupFile(
-FilenameLoc,
-LookupFilename, isAngled,
-LookupFrom, LookupFromFile, CurDir, nullptr, nullptr,
-&SuggestedModule, &IsMapped, /*IsFrameworkFound=*/nullptr,
-/*SkipCache*/ true);
-if (File)
-  return File;
-  }
-}
-  }
-
   if (SuppressIncludeNotFoundError)
 return None;
 
Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -61,23 +61,6 @@
const Token &FilenameTok,
SrcMgr::CharacteristicKind FileType) {}
 
-  /// Callback invoked whenever an inclusion directive results in a
-  /// file-not-found error.
-  ///
-  /// \param FileName The name of the file being included, as written in the
-  /// source code.
-  ///
-  /// \param RecoveryPath If this client indicates that it can recover from
-  /// this missing file, the client should set this as an additional header
-  /// search patch.
-  ///
-  /// \returns true to indicate that the preprocessor should attempt to recover
-  /// by adding \p RecoveryPath as a header search path.
-  virtual bool FileNotFound(StringRef FileName,
-SmallVectorImpl &RecoveryPath) {
-return false;
-  }
-
   /// Callback invoked whenever an inclusion directive of
   /// any kind (\c \#include, \c \#import, etc.) has been processed, regardless
   /// of whether the inclusion will actually result in an inclusion.
@@ -443,12 +426,6 @@
 Second->FileSkipped(SkippedFile, FilenameTok, FileType);
   }
 
-  bool FileNotFound(StringRef FileName,
-SmallVectorImpl &RecoveryPath) override {
-return First->FileNotFound(FileName, RecoveryPath) ||
-   Second->FileNotFound(FileName, RecoveryPath);
-  }
-
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.h
===
--- clang-tools-extra/pp-trace/PPCallbacksTracker.h
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.h
@@ -91,8 +91,6 @@
FileID PrevFID = FileID()) override;
   void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok,
SrcMgr::CharacteristicKind FileType) override;
-  bool FileNotFound(llvm::StringRef FileName,
-llvm::SmallVectorImpl &RecoveryPath) override;
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
   llvm::StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D119651#3317619 , @Izaron wrote:

>> Should we maybe always treat `PotentiallyEvaluated` as `ConstantEvaluated` 
>> in constant evaluated contexts? could that work ?
>
> Indeed, within this patch we can prevent similars bugs to appear. I 
> researched other places where a new context is pushed, and haven't find other 
> bugs, but nevertheless.
> In my subjective opinion, the architecture of `ExprEvalContext` is pretty 
> fragile... We could add an assert before this line 
> ,
>  ensuring that we don't push a (runtime) evaluated context after an immediate 
> context. Or should we just don't push the new context in this case?... I 
> wonder what is better, can't say right now =(
>
> In D119651#3317458 , @cor3ntin 
> wrote:
>
>> There seems to be quite a number of consteval related issues still - 
>> https://reviews.llvm.org/D113859 is very similar - yet completely unrelated -
>>
>> This patch does look okay to me, in so far as it fixes this issue, in a way 
>> that seems sensible to me. I'm just wondering if there are similar issues in 
>> other places...
>
> BTW after looking at consteval-related issues on github 
> ,
>  I've made four bite-sized patches. The issues are indeed completely 
> unrelated to each other and do not have common source of errors.
>
> https://reviews.llvm.org/D119095 Extra constructor call - a fix in 
> `RemoveNestedImmediateInvocation`.
> https://reviews.llvm.org/D119375 Trying to evaluate value-dependent 
> ConstantExpr - a fix in `CheckForImmediateInvocation` (approved)
> https://reviews.llvm.org/D119646 Trying to mark declarations as "referenced" 
> inside a ConstantExpr in default arguments - a fix in the custom def. arg. 
> AST visitor.
> https://reviews.llvm.org/D119651 (This patch) a `PotentiallyEvaluated` 
> context is created within a `ConstantEvaluated` context - remove where we do 
> this.
>
> As far as I see, the consteval bugs rarely have common source... They mostly 
> require ad-hoc solutions.

I like the idea of the assert, can we do it as a part of this?  That way when 
we run into it it becomes more obvious/linked to this discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119651

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

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

I will try to get back to this soon, but it will probably not be this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

[Alexander Richardson via Phabricator via cfe-commits]

arichardson planned changes to this revision.
arichardson added a comment.

Have to fix `cmake -GNinja -DCLANG_ENABLE_STATIC_ANALYZER=OFF 
-DLLVM_ENABLE_PROJECTS="llvm;clang;clang-tools-extra" -DCLANG_ENABLE_ARCMT=OFF 
../llvm`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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

[PATCH] D119710: [Docs][OpenCL] Release 14 notes

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, azabaznov.
Herald added subscribers: Naghasan, ebevhan, yaxunl.
Anastasia requested review of this revision.

Summary of important changes for OpenCL in release 14.


https://reviews.llvm.org/D119710

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -253,10 +253,38 @@
 Objective-C Language Changes in Clang
 -
 
-OpenCL C Language Changes in Clang
---
-
-...
+OpenCL Kernel Language Changes in Clang
+---
+
+OpenCL 3.0 (see :ref:`OpenCL 3.0 status page ` for more details):
+
+- Added parsing support for optionality of device side enqueue and blocks (not
+  fully incomplete yet!).
+- Added missing support for optionality of various builtin functions:
+
+  - ``read_write`` images, pipes, collective workgroup in the default header.
+  - ``read_write`` images, named address space atomics in internal 
``opencl-c.h``
+(enabled via ``-finclude-default-header`` frontend flag).
+
+C++ for OpenCL:
+
+- Added experimental support of C++ for OpenCL 2021 as per `the provisional
+  language documentation
+  
`_.
+  Support of all optional features is aligned with the OpenCL 3.0 status.
+- Added ``__remove_address_space`` utility (documentation available in
+  :doc:`LanguageExtensions`).
+- Fix address space for temporaries (to be ``__private``).
+- Disallows static kernel functions.
+- Fix implicit definition of ``__cpp_threadsafe_static_init`` macro.
+
+Misc changes:
+
+- Added generation of SPIR-V binaries via external ``llvm-spirv`` tool.
+  For more details refer to :ref:`the SPIR-V support section `.
+- Added new extensions for ``atomic_half`` and ``cl_ext_float_atomics``.
+- Fixed/improved support of ``vload``/``vstore``.
+- Fixed incorrect ``as_type`` support for 3-element vector types. 
 
 ABI Changes in Clang
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -253,10 +253,38 @@
 Objective-C Language Changes in Clang
 -
 
-OpenCL C Language Changes in Clang
---
-
-...
+OpenCL Kernel Language Changes in Clang
+---
+
+OpenCL 3.0 (see :ref:`OpenCL 3.0 status page ` for more details):
+
+- Added parsing support for optionality of device side enqueue and blocks (not
+  fully incomplete yet!).
+- Added missing support for optionality of various builtin functions:
+
+  - ``read_write`` images, pipes, collective workgroup in the default header.
+  - ``read_write`` images, named address space atomics in internal ``opencl-c.h``
+(enabled via ``-finclude-default-header`` frontend flag).
+
+C++ for OpenCL:
+
+- Added experimental support of C++ for OpenCL 2021 as per `the provisional
+  language documentation
+  `_.
+  Support of all optional features is aligned with the OpenCL 3.0 status.
+- Added ``__remove_address_space`` utility (documentation available in
+  :doc:`LanguageExtensions`).
+- Fix address space for temporaries (to be ``__private``).
+- Disallows static kernel functions.
+- Fix implicit definition of ``__cpp_threadsafe_static_init`` macro.
+
+Misc changes:
+
+- Added generation of SPIR-V binaries via external ``llvm-spirv`` tool.
+  For more details refer to :ref:`the SPIR-V support section `.
+- Added new extensions for ``atomic_half`` and ``cl_ext_float_atomics``.
+- Fixed/improved support of ``vload``/``vstore``.
+- Fixed incorrect ``as_type`` support for 3-element vector types. 
 
 ABI Changes in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1aeb4c6 - [ItaniumCXXABI] Avoid pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-02-14T15:17:14+01:00
New Revision: 1aeb4c6b5081944fd1e53a8f6ee0488ca33644c1

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

LOG: [ItaniumCXXABI] Avoid pointer element type accesses

Added: 


Modified: 
clang/lib/CodeGen/ItaniumCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 2979d92c8417..5ec9d3289ee8 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4472,7 +4472,7 @@ static void InitCatchParam(CodeGenFunction &CGF,
   // pad.  The best solution is to fix the personality function.
   } else {
 // Pull the pointer for the reference type off.
-llvm::Type *PtrTy = LLVMCatchTy->getPointerElementType();
+llvm::Type *PtrTy = CGF.ConvertTypeForMem(CaughtType);
 
 // Create the temporary and write the adjusted pointer into it.
 Address ExnPtrTmp =
@@ -4555,7 +4555,7 @@ static void InitCatchParam(CodeGenFunction &CGF,
   if (!copyExpr) {
 llvm::Value *rawAdjustedExn = CallBeginCatch(CGF, Exn, true);
 Address adjustedExn(CGF.Builder.CreateBitCast(rawAdjustedExn, PtrTy),
-caughtExnAlignment);
+LLVMCatchTy, caughtExnAlignment);
 LValue Dest = CGF.MakeAddrLValue(ParamAddr, CatchType);
 LValue Src = CGF.MakeAddrLValue(adjustedExn, CatchType);
 CGF.EmitAggregateCopy(Dest, Src, CatchType, AggValueSlot::DoesNotOverlap);
@@ -4569,7 +4569,7 @@ static void InitCatchParam(CodeGenFunction &CGF,
 
   // Cast that to the appropriate type.
   Address adjustedExn(CGF.Builder.CreateBitCast(rawAdjustedExn, PtrTy),
-  caughtExnAlignment);
+  LLVMCatchTy, caughtExnAlignment);
 
   // The copy expression is defined in terms of an OpaqueValueExpr.
   // Find it and map it to the adjusted expression.



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

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D119528#3316311 , @junaire wrote:

>> I think we should probably have test coverage for `long double` as well, but 
>> I also wonder whether it makes sense to add coverage for the small 
>> floating-point types (like `_Float16`) as well, or whether we already have 
>> coverage for those elsewhere.
>
> Test long double is fine for me. But I'm not really sure if we need to test 
> for these half-precision floats, as they are part of clang language extension 
> and not supported in all targets. IMHO, maybe we can just simply test these 
> normal or standard floats first?

I'm fine with that approach. Thanks for adding the `long double` support, but 
it looks like the precommit CI spotted an issue:

  FAIL: Clang :: Sema/warn-literal-range.c (12332 of 29830)
   TEST 'Clang :: Sema/warn-literal-range.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include 
-nostdsysteminc -std=c99 -verify 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include" 
"-nostdsysteminc" "-std=c99" "-verify" 
"C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c"
  # command stderr:
  error: 'warning' diagnostics expected but not seen: 
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 29: magnitude of floating-point constant too small for type 'long double'; 
minimum is 3.64519953188247460253E-4951
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 31: magnitude of floating-point constant too large for type 'long double'; 
maximum is 1.18973149535723176502E+4932
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 35: magnitude of floating-point constant too small for type 'long double'; 
minimum is 3.64519953188247460253E-4951
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 37: magnitude of floating-point constant too large for type 'long double'; 
maximum is 1.18973149535723176502E+4932
  
  error: 'warning' diagnostics seen but not expected: 
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 29: magnitude of floating-point constant too small for type 'long double'; 
minimum is 4.9406564584124654E-324
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 31: magnitude of floating-point constant too large for type 'long double'; 
maximum is 1.7976931348623157E+308
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 35: magnitude of floating-point constant too small for type 'long double'; 
minimum is 4.9406564584124654E-324
  
File 
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
Line 37: magnitude of floating-point constant too large for type 'long double'; 
maximum is 1.7976931348623157E+308
  
  8 errors generated.
  
  
  error: command failed with exit status: 1
  
  --
  
  

You might need to add a target triple to the test to specify exactly which 
target you are testing for. Bonus points if you pick a second target with 
different range of values. You can do that with something like:

  // RUN: %clang_cc1 -triple=whatever -std=c99 -verify=whatever %s
  // RUN: %clang_cc1 -triple=whatever-else -std=c99 -verify=whatever-else %s
  
  long double ld5 = 0x0.42p+42000L; // whatever-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}} \

whatever-else-warning {{magnitude of floating-point constant too large for type 
'long double'; maximum is 12}} \

(the identifier after `-verify=` is used in place of the `expected` in 
diagnostic verification so you can vary which warnings are checked against 
which RUN lines.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119528

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

[PATCH] D119711: Add asan support for MSVC debug runtimes

[Petr Mikhalitsyn via Phabricator via cfe-commits]

lo1ol created this revision.
lo1ol added a reviewer: rnk.
lo1ol requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use special debug asan versions (prefixed with asan_dbg) for MSVC debug 
runtimes (/MTd, /MDd, /LDd).

Resolves: 
https://github.com/llvm/llvm-project/compare/main...lo1ol:asan_dbg?expand=1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119711

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp


Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -553,13 +553,20 @@
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
   }
 
+  bool DebugRuntime = Args.hasArg(options::OPT__SLASH_MTd) ||
+  Args.hasArg(options::OPT__SLASH_MDd) ||
+  Args.hasArg(options::OPT__SLASH_LDd);
+
+  std::string AsanPrefix = DebugRuntime ? "asan_dbg" : "asan";
+  std::string AsanCXX = DebugRuntime ? "asan_cxx_dbg" : "asan_cxx";
+
   if (TC.getSanitizerArgs(Args).needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
 if (TC.getSanitizerArgs(Args).needsSharedRt() ||
 Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
-  for (const auto &Lib : {"asan_dynamic", "asan_dynamic_runtime_thunk"})
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
+  for (const auto &Lib : {"_dynamic", "_dynamic_runtime_thunk"})
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, AsanPrefix + Lib));
   // Make sure the dynamic runtime thunk is not optimized out at link time
   // to ensure proper SEH handling.
   CmdArgs.push_back(Args.MakeArgString(
@@ -568,12 +575,14 @@
   : "-include:__asan_seh_interceptor"));
   // Make sure the linker consider all object files from the dynamic 
runtime
   // thunk.
-  CmdArgs.push_back(Args.MakeArgString(std::string("-wholearchive:") +
-  TC.getCompilerRT(Args, "asan_dynamic_runtime_thunk")));
+  CmdArgs.push_back(Args.MakeArgString(
+  std::string("-wholearchive:") +
+  TC.getCompilerRT(Args, AsanPrefix + "_dynamic_runtime_thunk")));
 } else if (DLL) {
-  CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dll_thunk"));
+  CmdArgs.push_back(
+  TC.getCompilerRTArgString(Args, AsanPrefix + "_dll_thunk"));
 } else {
-  for (const auto &Lib : {"asan", "asan_cxx"}) {
+  for (const auto &Lib : {AsanPrefix, AsanCXX}) {
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
 // Make sure the linker consider all object files from the static lib.
 // This is necessary because instrumented dlls need access to all the
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -814,23 +814,6 @@
 }
 }
 
-if (Arg *WindowsDebugRTArg =
-Args.getLastArg(options::OPT__SLASH_MTd, options::OPT__SLASH_MT,
-options::OPT__SLASH_MDd, options::OPT__SLASH_MD,
-options::OPT__SLASH_LDd, options::OPT__SLASH_LD)) {
-  switch (WindowsDebugRTArg->getOption().getID()) {
-  case options::OPT__SLASH_MTd:
-  case options::OPT__SLASH_MDd:
-  case options::OPT__SLASH_LDd:
-if (DiagnoseErrors) {
-  D.Diag(clang::diag::err_drv_argument_not_allowed_with)
-  << WindowsDebugRTArg->getAsString(Args)
-  << lastArgumentForMask(D, Args, SanitizerKind::Address);
-  D.Diag(clang::diag::note_drv_address_sanitizer_debug_runtime);
-}
-  }
-}
-
 AsanUseAfterScope = Args.hasFlag(
 options::OPT_fsanitize_address_use_after_scope,
 options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);


Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -553,13 +553,20 @@
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
   }
 
+  bool DebugRuntime = Args.hasArg(options::OPT__SLASH_MTd) ||
+  Args.hasArg(options::OPT__SLASH_MDd) ||
+  Args.hasArg(options::OPT__SLASH_LDd);
+
+  std::string AsanPrefix = DebugRuntime ? "asan_dbg" : "asan";
+  std::string AsanCXX = DebugRuntime ? "asan_cxx_dbg" : "asan_cxx";
+
   if (TC.getSanitizerArgs(Args).needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
 if (TC.getSanitizerArgs(Args).needsSharedRt() ||
 Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
-  fo

[PATCH] D119710: [Docs][OpenCL] Release 14 notes

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:259-268
+OpenCL 3.0 (see :ref:`OpenCL 3.0 status page ` for more details):
+
+- Added parsing support for optionality of device side enqueue and blocks (not
+  fully incomplete yet!).
+- Added missing support for optionality of various builtin functions:
+
+  - ``read_write`` images, pipes, collective workgroup in the default header.

This might need to be updated depending on the situation with backports: 
https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+label%3Aopencl+label%3Arelease%3Abackport

But we can do this separately...


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

https://reviews.llvm.org/D119710

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

[PATCH] D119713: [Docs] Release 14 notes for SPIR-V in clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, linjamaki.
Herald added a subscriber: ebevhan.
Anastasia requested review of this revision.

Added important information about SPIR-V support in the upcoming release.


https://reviews.llvm.org/D119713

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -47,6 +47,8 @@
 Major New Features
 --
 
+- Added SPIR-V triple and binary generation using external ``llvm-spirv`` tool.
+  For more details refer to :ref:`the SPIR-V support section `.
 -  ...
 
 Improvements to Clang's diagnostics
@@ -345,6 +347,17 @@
 - The ``attribute((target("branch-protection=...)))`` attributes will now also
   work for the ARM backend.
 
+SPIR-V Support in Clang
+---
+
+- Added triple/target ``spirv32`` and ``spirv64`` for 32-bit and 64-bit SPIR-V
+  respectively.
+- Added generation of binaries via external ``llvm-spirv`` tool. This can now
+  be used for HIP or OpenCL.
+- Added linking of separate object files in SPIR-V format using external
+  ``spirv-link`` tool.
+
+
 Floating Point Support in Clang
 ---
 - The default setting of FP contraction (FMA) is now -ffp-contract=on (for


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -47,6 +47,8 @@
 Major New Features
 --
 
+- Added SPIR-V triple and binary generation using external ``llvm-spirv`` tool.
+  For more details refer to :ref:`the SPIR-V support section `.
 -  ...
 
 Improvements to Clang's diagnostics
@@ -345,6 +347,17 @@
 - The ``attribute((target("branch-protection=...)))`` attributes will now also
   work for the ARM backend.
 
+SPIR-V Support in Clang
+---
+
+- Added triple/target ``spirv32`` and ``spirv64`` for 32-bit and 64-bit SPIR-V
+  respectively.
+- Added generation of binaries via external ``llvm-spirv`` tool. This can now
+  be used for HIP or OpenCL.
+- Added linking of separate object files in SPIR-V format using external
+  ``spirv-link`` tool.
+
+
 Floating Point Support in Clang
 ---
 - The default setting of FP contraction (FMA) is now -ffp-contract=on (for
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119713: [Docs] Release 14 notes for SPIR-V in clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@linjamaki feel free to suggest other topics to document for the release.


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

https://reviews.llvm.org/D119713

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

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 408389.
ro added a subscriber: nikic.
ro added a comment.

Since the build-time check for the `ld -z relax=transtls` option was met with 
massive resistance and was only necessary for Illumos anyway, this revision 
just uses it unconditionally.

Tested on `amd64-pc-solaris2.11` and `sparcv9-sun-solaris2.11`.

The Illumos community will have to provide a way to somehow distinguish the two 
OSes.  The configure triple is still the same, and the `uname -o` output 
(`Solaris` vs. `illumos` seems to be hardcoded in the `uname` command and not 
be available programmatically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

Files:
  clang/lib/Driver/ToolChains/Solaris.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_solaris.h

Index: compiler-rt/lib/sanitizer_common/sanitizer_solaris.h
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_solaris.h
@@ -0,0 +1,63 @@
+//===-- sanitizer_solaris.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file is a part of Sanitizer runtime. It contains Solaris-specific
+// definitions.
+//
+//===--===//
+
+#ifndef SANITIZER_SOLARIS_H
+#define SANITIZER_SOLARIS_H
+
+#include "sanitizer_internal_defs.h"
+
+#if SANITIZER_SOLARIS
+
+#include 
+
+namespace __sanitizer {
+
+// Beginning of declaration from OpenSolaris/Illumos
+// $SRC/cmd/sgs/include/rtld.h.
+struct Rt_map {
+  Link_map rt_public;
+  const char *rt_pathname;
+  ulong_t rt_padstart;
+  ulong_t rt_padimlen;
+  ulong_t rt_msize;
+  uint_t rt_flags;
+  uint_t rt_flags1;
+  ulong_t rt_tlsmodid;
+};
+
+// Structure matching the Solaris 11.4 struct dl_phdr_info used to determine
+// presence of dlpi_tls_modid field at runtime.  Cf. Solaris 11.4
+// dl_iterate_phdr(3C), Example 2.
+struct dl_phdr_info_test {
+  ElfW(Addr) dlpi_addr;
+  const char *dlpi_name;
+  const ElfW(Phdr) * dlpi_phdr;
+  ElfW(Half) dlpi_phnum;
+  u_longlong_t dlpi_adds;
+  u_longlong_t dlpi_subs;
+  size_t dlpi_tls_modid;
+  void *dlpi_tls_data;
+};
+
+struct TLS_index {
+  unsigned long ti_moduleid;
+  unsigned long ti_tlsoffset;
+};
+
+extern "C" void *__tls_get_addr(TLS_index *);
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SOLARIS
+
+#endif  // SANITIZER_SOLARIS_H
Index: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
===
--- compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -27,6 +27,7 @@
 #include "sanitizer_linux.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
+#include "sanitizer_solaris.h"
 
 #if SANITIZER_NETBSD
 #define _RTLD_SOURCE  // for __lwp_gettcb_fast() / __lwp_getprivate_fast()
@@ -62,6 +63,7 @@
 #endif
 
 #if SANITIZER_SOLARIS
+#include 
 #include 
 #include 
 #endif
@@ -444,6 +446,39 @@
   void **);
 #endif
 
+#if SANITIZER_SOLARIS
+// dlpi_tls_modid is only available since Solaris 11.4 SRU 10.  Use
+// dlinfo(RTLD_DI_LINKMAP) instead which works on both Solaris 11.3 and Illumos.
+
+static size_t main_tls_modid;
+
+int GetSizeFromHdr(struct dl_phdr_info *info, size_t size, void *data) {
+  const ElfW(Phdr) *hdr = info->dlpi_phdr;
+  const ElfW(Phdr) *last_hdr = hdr + info->dlpi_phnum;
+
+  // With the introduction of dlpi_tls_modid, the tlsmodid of the executable
+  // was changed to 1 to match other implementations.
+  if (size >= offsetof(dl_phdr_info_test, dlpi_tls_modid))
+main_tls_modid = 1;
+  else
+main_tls_modid = 0;
+
+  for (; hdr != last_hdr; ++hdr) {
+if (hdr->p_type == PT_TLS) {
+  Rt_map *map;
+
+  dlinfo(RTLD_SELF, RTLD_DI_LINKMAP, &map);
+
+  if (map->rt_tlsmodid == main_tls_modid) {
+*(uptr *)data = hdr->p_memsz;
+return -1;
+  }
+}
+  }
+  return 0;
+}
+#endif  // SANITIZER_SOLARIS
+
 #if !SANITIZER_GO
 static void GetTls(uptr *addr, uptr *size) {
 #if SANITIZER_ANDROID
@@ -538,9 +573,15 @@
 }
   }
 #elif SANITIZER_SOLARIS
-  // FIXME
   *addr = 0;
   *size = 0;
+  // Find size (p_memsz) of TLS block of the main program.
+  dl_iterate_phdr(GetSizeFromHdr, size);
+
+  if (*size != 0) {
+TLS_index ti = {(unsigned long)main_tls_modid, 0};
+*addr = (uptr)__tls_get_addr(&ti);
+  }
 #else
 #error "Unknown OS"
 #endif
Index: clang/lib/Driver/ToolChains/Solaris.cpp

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

@vitalybuka , @MaskRay could I persuade you to review the revision version 
which should be much less controversial and completely Solaris-specific?  
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D119714: [clang][lex] Remove `Preprocessor::GetCurDirLookup()`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`Preprocessor` exposes the search directory iterator via `GetCurDirLookup()` 
getter, which is only used in two static functions.

To simplify reasoning about search directory iterators/references and to 
simplify the `Preprocessor` API, this patch makes the two static functions 
private member functions and removes the getter entirely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119714

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

Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1244,45 +1244,39 @@
   return File.hasValue();
 }
 
-/// EvaluateHasInclude - Process a '__has_include("path")' expression.
-/// Returns true if successful.
-static bool EvaluateHasInclude(Token &Tok, IdentifierInfo *II,
-   Preprocessor &PP) {
-  return EvaluateHasIncludeCommon(Tok, II, PP, nullptr, nullptr);
+bool Preprocessor::EvaluateHasInclude(Token &Tok, IdentifierInfo *II) {
+  return EvaluateHasIncludeCommon(Tok, II, *this, nullptr, nullptr);
 }
 
-/// EvaluateHasIncludeNext - Process '__has_include_next("path")' expression.
-/// Returns true if successful.
-static bool EvaluateHasIncludeNext(Token &Tok,
-   IdentifierInfo *II, Preprocessor &PP) {
+bool Preprocessor::EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II) {
   // __has_include_next is like __has_include, except that we start
   // searching after the current found directory.  If we can't do this,
   // issue a diagnostic.
   // FIXME: Factor out duplication with
   // Preprocessor::HandleIncludeNextDirective.
-  const DirectoryLookup *Lookup = PP.GetCurDirLookup();
+  const DirectoryLookup *Lookup = CurDirLookup;
   const FileEntry *LookupFromFile = nullptr;
-  if (PP.isInPrimaryFile() && PP.getLangOpts().IsHeaderFile) {
+  if (isInPrimaryFile() && getLangOpts().IsHeaderFile) {
 // If the main file is a header, then it's either for PCH/AST generation,
 // or libclang opened it. Either way, handle it as a normal include below
 // and do not complain about __has_include_next.
-  } else if (PP.isInPrimaryFile()) {
+  } else if (isInPrimaryFile()) {
 Lookup = nullptr;
-PP.Diag(Tok, diag::pp_include_next_in_primary);
-  } else if (PP.getCurrentLexerSubmodule()) {
+Diag(Tok, diag::pp_include_next_in_primary);
+  } else if (getCurrentLexerSubmodule()) {
 // Start looking up in the directory *after* the one in which the current
 // file would be found, if any.
-assert(PP.getCurrentLexer() && "#include_next directive in macro?");
-LookupFromFile = PP.getCurrentLexer()->getFileEntry();
+assert(getCurrentLexer() && "#include_next directive in macro?");
+LookupFromFile = getCurrentLexer()->getFileEntry();
 Lookup = nullptr;
   } else if (!Lookup) {
-PP.Diag(Tok, diag::pp_include_next_absolute_path);
+Diag(Tok, diag::pp_include_next_absolute_path);
   } else {
 // Start looking up in the next directory.
 ++Lookup;
   }
 
-  return EvaluateHasIncludeCommon(Tok, II, PP, Lookup, LookupFromFile);
+  return EvaluateHasIncludeCommon(Tok, II, *this, Lookup, LookupFromFile);
 }
 
 /// Process single-argument builtin feature-like macros that return
@@ -1736,9 +1730,9 @@
 // double-quotes ("").
 bool Value;
 if (II == Ident__has_include)
-  Value = EvaluateHasInclude(Tok, II, *this);
+  Value = EvaluateHasInclude(Tok, II);
 else
-  Value = EvaluateHasIncludeNext(Tok, II, *this);
+  Value = EvaluateHasIncludeNext(Tok, II);
 
 if (Tok.isNot(tok::r_paren))
   return;
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -2077,13 +2077,6 @@
  ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
  bool *IsFrameworkFound, bool SkipCache = false);
 
-  /// Get the DirectoryLookup structure used to find the current
-  /// FileEntry, if CurLexer is non-null and if applicable.
-  ///
-  /// This allows us to implement \#include_next and find directory-specific
-  /// properties.
-  const DirectoryLookup *GetCurDirLookup() { return CurDirLookup; }
-
   /// Return true if we're in the top-level file, not in a \#include.
   bool isInPrimaryFile() const;
 
@@ -2197,6 +2190,16 @@
   /// If the expression is equivalent to "!defined(X)" return X in IfNDefMacro.
   DirectiveEvalResult EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro);
 
+  /// Process a '__has_include("path")' expression.
+  ///
+  /// Returns true if successful.
+  b

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

Hey @steakhal , any updates on this ? should I commit it ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119462

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

[clang] 5029dce - Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-02-14T09:38:26-05:00
New Revision: 5029dce492b3cf3ac191eda0b5bf268c3acac2e0

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

LOG: Implement WG14 N2764 the [[noreturn]] attribute

This adds support for http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf,
which was adopted at the Feb 2022 WG14 meeting. That paper adds
[[noreturn]] and [[_Noreturn]] to the list of supported attributes in
C2x. These attributes have the same semantics as the [[noreturn]]
attribute in C++.

The [[_Noreturn]] attribute was added as a deprecated feature so that
translation units which include  do not get an error on
use of [[noreturn]] because the macro expands to _Noreturn. Users can
use -Wno-deprecated-attributes to silence the diagnostic.

Use of  or the noreturn macro were both deprecated.
Users can define the _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS macro to
suppress the deprecation diagnostics coming from the header file.

Added: 
clang/test/Sema/c2x-noreturn.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Headers/stdnoreturn.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/AST/ast-dump-lambda.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 26860d3118020..d6115e0e4a51c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -92,6 +92,11 @@ Windows Support
 C Language Changes in Clang
 ---
 
+C2x Feature Support
+---
+
+- Implemented `WG14 N2674 The noreturn attribute 
`_.
+
 C++ Language Changes in Clang
 -
 

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 8e8b7bc16e3b2..f47bb413ea997 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1209,10 +1209,10 @@ def C11NoReturn : InheritableAttr {
 }
 
 def CXX11NoReturn : InheritableAttr {
-  let Spellings = [CXX11<"", "noreturn", 200809>];
+  let Spellings = [CXX11<"", "noreturn", 200809>,
+   C2x<"", "noreturn", 202202>, C2x<"", "_Noreturn", 202202>];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [CXX11NoReturnDocs];
-  let SimpleHandler = 1;
 }
 
 // Similar to CUDA, OpenCL attributes do not receive a [[]] spelling because

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1ea74e5e97f64..e7c204fef2a09 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4027,6 +4027,9 @@ def warn_vector_mode_deprecated : Warning<
   "specifying vector types with the 'mode' attribute is deprecated; "
   "use the 'vector_size' attribute instead">,
   InGroup;
+def warn_deprecated_noreturn_spelling : Warning<
+  "the '[[_Noreturn]]' attribute spelling is deprecated in C2x; use "
+  "'[[noreturn]]' instead">, InGroup;
 def err_complex_mode_vector_type : Error<
   "type of machine mode does not support base vector types">;
 def err_enum_mode_vector_type : Error<

diff  --git a/clang/lib/Headers/stdnoreturn.h b/clang/lib/Headers/stdnoreturn.h
index e83cd8153752c..92fd4a98a87bf 100644
--- a/clang/lib/Headers/stdnoreturn.h
+++ b/clang/lib/Headers/stdnoreturn.h
@@ -13,4 +13,13 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
+#if __STDC_VERSION__ > 201710L &&  
\
+!defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+
+/* Including the header file in C2x is also deprecated. */
+#warning "the '' header is deprecated in C2x"
+#endif
+
 #endif /* __STDNORETURN_H */

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index ddc7221e3713e..6d0c8f0974767 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2175,6 +2175,16 @@ static void handleNoReturnAttr(Sema &S, Decl *D, const 
ParsedAttr &Attrs) {
   D->addAttr(::new (S.Context) NoReturnAttr(S.Context, Attrs));
 }
 
+static void handleStandardNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  // The [[_Noreturn]] spelling is deprecated in C2x, so if that was used,
+  // issue an appropriate diagnostic.
+  if (!S.getLangOpts().CPlusPlus &&
+  A.getSemanticSpelling() == CXX11NoReturnAttr::C2x_Noreturn)
+S.Diag(A.getLoc(), diag::warn_deprecated_noreturn_spelling) << 
A.getRange();
+
+  D->addAttr(::new (S.Context) CXX11NoReturnAttr(S.Context, A));
+}
+
 static void handleNoCfCheckAttr(Sema &S, Decl *D, const ParsedA

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

Thanks for the quick review. I've landed in 
5029dce492b3cf3ac191eda0b5bf268c3acac2e0 
, but if 
anyone has post commit review comments (I landed this pretty quickly after 
initial review), I'm happy to address them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[clang] f037082 - Fix the Sphinx build

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-02-14T09:43:47-05:00
New Revision: f037082714a0fd48568a1f10ef3e4db8919e1523

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

LOG: Fix the Sphinx build

Add a heading to appease the Sphinx bot, and add some basic
documentation for [[_Noreturn]].

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 0e871b4aaf1a..f61c9d00c444 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -487,10 +487,14 @@ pointer type.
 
 def CXX11NoReturnDocs : Documentation {
   let Category = DocCatFunction;
+  let Heading = "noreturn, _Noreturn";
   let Content = [{
 A function declared as ``[[noreturn]]`` shall not return to its caller. The
 compiler will generate a diagnostic for a function declared as ``[[noreturn]]``
 that appears to be capable of returning to its caller.
+
+The ``[[_Noreturn]]`` spelling is deprecated and only exists to ease code
+migration for code using ``[[noreturn]]`` after including .
   }];
 }
 



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

[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types

[Michael Colavita via Phabricator via cfe-commits]

colavitam added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119063

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

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

I think we dropped the ball on this - was it ever re-reverted?
Is it still worth trying to land this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I'm not sure this is a 'good' enough warning (by design?) to allow in -Wall 
either, Wall seems to be only "this is definitely a mistake" type issues, and I 
don't think this issue rises to that requirement (though others are likely more 
qualified  than I to judge).

Implementation generally looks right to me otherwise, with 1 question/comment.




Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

Do we really want this?  I guess I would think doing:


```
void MyFunc(auto whatever) {
  auto X = move(whatever);
```

when I MEAN std::move, just for it to pick up a non-std::move the 1st time is 
likely the same problem?  Or should it get a separate warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670

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

[PATCH] D118753: [PowerPC] Fix __builtin_pdepd and __builtin_pextd to be 64-bit and P10 only.

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118753

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

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

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

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

I think yes, go ahead!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119462

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

[PATCH] D119716: [clang][lex] NFC: De-duplicate some #include_next logic

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch addresses a FIXME and de-duplicates some `#include_next` logic

Depends on D119714 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119716

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

Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1249,32 +1249,9 @@
 }
 
 bool Preprocessor::EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II) {
-  // __has_include_next is like __has_include, except that we start
-  // searching after the current found directory.  If we can't do this,
-  // issue a diagnostic.
-  // FIXME: Factor out duplication with
-  // Preprocessor::HandleIncludeNextDirective.
-  const DirectoryLookup *Lookup = CurDirLookup;
-  const FileEntry *LookupFromFile = nullptr;
-  if (isInPrimaryFile() && getLangOpts().IsHeaderFile) {
-// If the main file is a header, then it's either for PCH/AST generation,
-// or libclang opened it. Either way, handle it as a normal include below
-// and do not complain about __has_include_next.
-  } else if (isInPrimaryFile()) {
-Lookup = nullptr;
-Diag(Tok, diag::pp_include_next_in_primary);
-  } else if (getCurrentLexerSubmodule()) {
-// Start looking up in the directory *after* the one in which the current
-// file would be found, if any.
-assert(getCurrentLexer() && "#include_next directive in macro?");
-LookupFromFile = getCurrentLexer()->getFileEntry();
-Lookup = nullptr;
-  } else if (!Lookup) {
-Diag(Tok, diag::pp_include_next_absolute_path);
-  } else {
-// Start looking up in the next directory.
-++Lookup;
-  }
+  const DirectoryLookup *Lookup;
+  const FileEntry *LookupFromFile;
+  getIncludeNextStart(Tok, Lookup, LookupFromFile);
 
   return EvaluateHasIncludeCommon(Tok, II, *this, Lookup, LookupFromFile);
 }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1778,6 +1778,40 @@
   return true;
 }
 
+void Preprocessor::getIncludeNextStart(Token &IncludeNextTok,
+   const DirectoryLookup *&Lookup,
+   const FileEntry *&LookupFromFile) const {
+  // #include_next is like #include, except that we start searching after
+  // the current found directory.  If we can't do this, issue a
+  // diagnostic.
+  Lookup = CurDirLookup;
+  LookupFromFile = nullptr;
+
+  if (isInPrimaryFile() && LangOpts.IsHeaderFile) {
+// If the main file is a header, then it's either for PCH/AST generation,
+// or libclang opened it. Either way, handle it as a normal include below
+// and do not complain about include_next.
+  } else if (isInPrimaryFile()) {
+Lookup = nullptr;
+Diag(IncludeNextTok, diag::pp_include_next_in_primary);
+  } else if (CurLexerSubmodule) {
+// Start looking up in the directory *after* the one in which the current
+// file would be found, if any.
+assert(CurPPLexer && "#include_next directive in macro?");
+LookupFromFile = CurPPLexer->getFileEntry();
+Lookup = nullptr;
+  } else if (!Lookup) {
+// The current file was not found by walking the include path. Either it
+// is the primary file (handled above), or it was found by absolute path,
+// or it was found relative to such a file.
+// FIXME: Track enough information so we know which case we're in.
+Diag(IncludeNextTok, diag::pp_include_next_absolute_path);
+  } else {
+// Start looking up in the next directory.
+++Lookup;
+  }
+}
+
 /// HandleIncludeDirective - The "\#include" tokens have just been read, read
 /// the file to be included from the lexer, then include it!  This is a common
 /// routine with functionality shared between \#include, \#include_next and
@@ -2375,34 +2409,9 @@
   Token &IncludeNextTok) {
   Diag(IncludeNextTok, diag::ext_pp_include_next_directive);
 
-  // #include_next is like #include, except that we start searching after
-  // the current found directory.  If we can't do this, issue a
-  // diagnostic.
-  const DirectoryLookup *Lookup = CurDirLookup;
-  const FileEntry *LookupFromFile = nullptr;
-  if (isInPrimaryFile() && LangOpts.IsHeaderFile) {
-// If the main file is a header, then it's either for PCH/AST generation,
-// or libclang opened it. Either way, handle it as a normal include below
-// and do not complain about include_next.
-  } else if (isInPrimaryFile()) {
-Lookup = nullptr;
-Diag(IncludeNextTok, 

[PATCH] D119612: [clang] Pass more flags to ld64.lld

[Leonard Grey via Phabricator via cfe-commits]

lgrey added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:242
 } else if (D.getLTOMode() == LTOK_Thin)
   // If we are using thin LTO, then create a directory instead.
   TmpPathName = D.GetTemporaryDirectory("thinlto");

thakis wrote:
> lgrey: Do you think this should interact with 
> 134275d994d5fb38edfeb587ba45c8f495c8bf66 in any way, or should have any other 
> interesting side effects?
> 
> From what I understand, it tells the linker to put temporary files created 
> for LTO in the given directory, but then the clang driver cleans up that 
> directory when it exits (potentially after running dsymutil). But most people 
> run dsymutil later, separately, so I think there should be no interactions 
> there.
IIUC this is orthogonal to the cache's temp files. Something's actually not 
adding up here for me (why was it looking at the LTO cache files in the first 
place?), but this change shouldn't affect the cache thing.


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

https://reviews.llvm.org/D119612

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

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Jun Zhang via Phabricator via cfe-commits]

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

Add triple info to avoid tests failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119528

Files:
  clang/test/Sema/warn-literal-range.c


Index: clang/test/Sema/warn-literal-range.c
===
--- /dev/null
+++ clang/test/Sema/warn-literal-range.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 -verify %s
+
+float f0 = 0.42f; // no-warning
+
+float f1 = 1.4E-46f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f2 = 3.4E+39f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+float f3 = 0x4.2p42f; // no-warning
+
+float f4 = 0x0.42p-1000f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f5 = 0x0.42p+1000f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+double d0 = 0.42; // no-warning
+
+double d1 = 3.6E-4952; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d2 = 1.7E+309; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+double d3 = 0x0.42p42; // no-warning
+
+double d4 = 0x0.42p-4200; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d5 = 0x0.42p+4200; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+long double ld0 = 0.42L; // no-warning
+
+long double ld1 = 3.6E-4952L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld2 = 1.2E+4932L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}
+
+long double ld3 = 0x0.42p42L; // no-warning
+
+long double ld4 = 0x0.42p-42000L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld5 = 0x0.42p+42000L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}


Index: clang/test/Sema/warn-literal-range.c
===
--- /dev/null
+++ clang/test/Sema/warn-literal-range.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 -verify %s
+
+float f0 = 0.42f; // no-warning
+
+float f1 = 1.4E-46f; // expected-warning {{magnitude of floating-point constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f2 = 3.4E+39f; // expected-warning {{magnitude of floating-point constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+float f3 = 0x4.2p42f; // no-warning
+
+float f4 = 0x0.42p-1000f; // expected-warning {{magnitude of floating-point constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f5 = 0x0.42p+1000f; // expected-warning {{magnitude of floating-point constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+double d0 = 0.42; // no-warning
+
+double d1 = 3.6E-4952; // expected-warning {{magnitude of floating-point constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d2 = 1.7E+309; // expected-warning {{magnitude of floating-point constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+double d3 = 0x0.42p42; // no-warning
+
+double d4 = 0x0.42p-4200; // expected-warning {{magnitude of floating-point constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d5 = 0x0.42p+4200; // expected-warning {{magnitude of floating-point constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+long double ld0 = 0.42L; // no-warning
+
+long double ld1 = 3.6E-4952L; // expected-warning {{magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951}}
+
+long double ld2 = 1.2E+4932L; // expected-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932}}
+
+long double ld3 = 0x0.42p42L; // no-warning
+
+long double ld4 = 0x0.42p-42000L; // expected-warning {{magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951}}
+
+long double ld5 = 0x0.42p+42000L; // expected-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149

[PATCH] D119719: [Docs][OpenCL] Update OpenCL 3.0 status on release 14

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, azabaznov.
Herald added subscribers: Naghasan, ebevhan, yaxunl.
Anastasia requested review of this revision.

This update only includes what is available on the release 14 branch.

Note that this table might need to be updated to account for the backports:
https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+label%3Aopencl+label%3Arelease%3Abackport


https://reviews.llvm.org/D119719

Files:
  clang/docs/OpenCLSupport.rst


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -371,43 +371,43 @@
 The following table provides an overview of features in OpenCL C 3.0 and their
 implementation status.
 
-+--+-+-+--+--+
-| Category | Feature   
| Status   | Reviews
  |
-+==+=+=+==+==+
-| Command line interface   | New value for ``-cl-std`` flag
| :good:`done` | https://reviews.llvm.org/D88300
  |
-+--+-+-+--+--+
-| Predefined macros| New version macro 
| :good:`done` | https://reviews.llvm.org/D88300
  |
-+--+-+-+--+--+
-| Predefined macros| Feature macros
| :good:`done` | https://reviews.llvm.org/D95776
  |
-+--+-+-+--+--+
-| Feature optionality  | Generic address space 
| :good:`done` | https://reviews.llvm.org/D95778 
and https://reviews.llvm.org/D103401 |
-+--+-+-+--+--+
-| Feature optionality  | Builtin function overloads with generic 
address space | :good:`done` | 
https://reviews.llvm.org/D105526
 |
-+--+-+-+--+--+
-| Feature optionality  | Program scope variables in global memory  
| :good:`done` | https://reviews.llvm.org/D103191   
  |
-+--+-+-+--+--+
-| Feature optionality  | 3D image writes including builtin functions   
| :part:`worked on`| https://reviews.llvm.org/D106260 
(frontend)  |
-+--+-+-+--+--+
-| Feature optionality  | read_write images including builtin functions 
| :part:`worked on`| https://reviews.llvm.org/D104915 
(frontend) and https://reviews.llvm.org/D107539 (functions) |
-+--+-+-+--+

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

In D119528#3318913 , @aaron.ballman 
wrote:

> In D119528#3316311 , @junaire wrote:
>
>>> I think we should probably have test coverage for `long double` as well, 
>>> but I also wonder whether it makes sense to add coverage for the small 
>>> floating-point types (like `_Float16`) as well, or whether we already have 
>>> coverage for those elsewhere.
>>
>> Test long double is fine for me. But I'm not really sure if we need to test 
>> for these half-precision floats, as they are part of clang language 
>> extension and not supported in all targets. IMHO, maybe we can just simply 
>> test these normal or standard floats first?
>
> I'm fine with that approach. Thanks for adding the `long double` support, but 
> it looks like the precommit CI spotted an issue:
>
>   FAIL: Clang :: Sema/warn-literal-range.c (12332 of 29830)
>    TEST 'Clang :: Sema/warn-literal-range.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 1';   
> c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
> -internal-isystem 
> c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include 
> -nostdsysteminc -std=c99 -verify 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c
>   --
>   Exit Code: 1
>   
>   Command Output (stdout):
>   --
>   $ ":" "RUN: at line 1"
>   $ "c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
> "-internal-isystem" 
> "c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include" 
> "-nostdsysteminc" "-std=c99" "-verify" 
> "C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c"
>   # command stderr:
>   error: 'warning' diagnostics expected but not seen: 
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 29: magnitude of floating-point constant too small for type 'long 
> double'; minimum is 3.64519953188247460253E-4951
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 31: magnitude of floating-point constant too large for type 'long 
> double'; maximum is 1.18973149535723176502E+4932
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 35: magnitude of floating-point constant too small for type 'long 
> double'; minimum is 3.64519953188247460253E-4951
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 37: magnitude of floating-point constant too large for type 'long 
> double'; maximum is 1.18973149535723176502E+4932
>   
>   error: 'warning' diagnostics seen but not expected: 
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 29: magnitude of floating-point constant too small for type 'long 
> double'; minimum is 4.9406564584124654E-324
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 31: magnitude of floating-point constant too large for type 'long 
> double'; maximum is 1.7976931348623157E+308
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 35: magnitude of floating-point constant too small for type 'long 
> double'; minimum is 4.9406564584124654E-324
>   
> File 
> C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c 
> Line 37: magnitude of floating-point constant too large for type 'long 
> double'; maximum is 1.7976931348623157E+308
>   
>   8 errors generated.
>   
>   
>   error: command failed with exit status: 1
>   
>   --
>   
>   

Thanks for spending time looking at the CI issue, I thought it was something 
wrong with itself!

> You might need to add a target triple to the test to specify exactly which 
> target you are testing for. Bonus points if you pick a second target with 
> different range of values. You can do that with something like:
>
>   // RUN: %clang_cc1 -triple=whatever -std=c99 -verify=whatever %s
>   // RUN: %clang_cc1 -triple=whatever-else -std=c99 -verify=whatever-else %s
>   
>   long double ld5 = 0x0.42p+42000L; // whatever-warning {{magnitude of 
> floating-point constant too large for type 'long double'; maximum is 
> 1.18973149535723176502E+4932}} \
> 
> whatever-else-warning {{magnitude of floating-point constant too large for 
> type 'long double'; maximum is 12}} \
>
> (the identifier after `-verify=` is used in place of the `expected` in 
> diagnostic verification so you can vary which warnings are checked against 
> which RUN lines.)

Thanks for telling me these about the regression tests! But I have no LLVM 
development set up in Windows so I guess one triple test should be fine. Maybe 
I can use the info given by the CI

[PATCH] D117566: [clang][lex] Introduce `SearchDirIterator`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 408411.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117566

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

Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1157,9 +1157,9 @@
 /// EvaluateHasIncludeCommon - Process a '__has_include("path")'
 /// or '__has_include_next("path")' expression.
 /// Returns true if successful.
-static bool EvaluateHasIncludeCommon(Token &Tok,
- IdentifierInfo *II, Preprocessor &PP,
- const DirectoryLookup *LookupFrom,
+static bool EvaluateHasIncludeCommon(Token &Tok, IdentifierInfo *II,
+ Preprocessor &PP,
+ ConstSearchDirIterator LookupFrom,
  const FileEntry *LookupFromFile) {
   // Save the location of the current token.  If a '(' is later found, use
   // that location.  If not, use the end of this location instead.
@@ -1249,7 +1249,7 @@
 }
 
 bool Preprocessor::EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II) {
-  const DirectoryLookup *Lookup;
+  ConstSearchDirIterator Lookup = nullptr;
   const FileEntry *LookupFromFile;
   getIncludeNextStart(Tok, Lookup, LookupFromFile);
 
Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -66,7 +66,7 @@
 
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
-bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
+bool Preprocessor::EnterSourceFile(FileID FID, ConstSearchDirIterator CurDir,
SourceLocation Loc,
bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
@@ -100,7 +100,7 @@
 /// EnterSourceFileWithLexer - Add a source file to the top of the include stack
 ///  and start lexing tokens from it instead of the current buffer.
 void Preprocessor::EnterSourceFileWithLexer(Lexer *TheLexer,
-const DirectoryLookup *CurDir) {
+ConstSearchDirIterator CurDir) {
 
   // Add the current lexer to the include stack.
   if (CurPPLexer || CurTokenLexer)
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -817,13 +817,13 @@
 
 Optional Preprocessor::LookupFile(
 SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
-const DirectoryLookup *FromDir, const FileEntry *FromFile,
-const DirectoryLookup **CurDirArg, SmallVectorImpl *SearchPath,
+ConstSearchDirIterator FromDir, const FileEntry *FromFile,
+ConstSearchDirIterator *CurDirArg, SmallVectorImpl *SearchPath,
 SmallVectorImpl *RelativePath,
 ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
 bool *IsFrameworkFound, bool SkipCache) {
-  const DirectoryLookup *CurDirLocal = nullptr;
-  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+  ConstSearchDirIterator CurDirLocal = nullptr;
+  ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
 
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
@@ -877,8 +877,8 @@
   if (FromFile) {
 // We're supposed to start looking from after a particular file. Search
 // the include path until we find that file or run out of files.
-const DirectoryLookup *TmpCurDir = CurDir;
-const DirectoryLookup *TmpFromDir = nullptr;
+ConstSearchDirIterator TmpCurDir = CurDir;
+ConstSearchDirIterator TmpFromDir = nullptr;
 while (Optional FE = HeaderInfo.LookupFile(
Filename, FilenameLoc, isAngled, TmpFromDir, &TmpCurDir,
Includers, SearchPath, RelativePath, RequestingModule,
@@ -1779,7 +1779,7 @@
 }
 
 void Preprocessor::getIncludeNextStart(Token &IncludeNextTok,
-   const DirectoryLookup *&Lookup,
+   ConstSearchDirIterator &Lookup,
const FileEntry *&LookupFromFile) const {
   // #include_next is like #include, except that we start searching after
   // the current found directory.  If we can't do t

[clang] 6745b6a - [analyzer][NFCi] Use the correct BugType in CStringChecker.

[via cfe-commits]

Author: phyBrackets
Date: 2022-02-14T20:54:59+05:30
New Revision: 6745b6a0f18523d6b871f627ceb46a92b7c2c43d

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

LOG: [analyzer][NFCi] Use the correct BugType in CStringChecker.

There is different bug types for different types of bugs  but the 
**emitAdditionOverflowbug** seems to use bugtype **BT_NotCSting** but actually 
it have to use **BT_AdditionOverflow** .

Reviewed By: steakhal

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 6955efe8e6c2e..8483a1a47cea1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -257,7 +257,6 @@ class CStringChecker : public Checker< eval::Call,
   void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
-
   ProgramStateRef checkAdditionOverflow(CheckerContext &C,
 ProgramStateRef state,
 NonLoc left,
@@ -420,7 +419,6 @@ ProgramStateRef 
CStringChecker::CheckBufferAccess(CheckerContext &C,
 
 SVal BufEnd =
 svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
-
 State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
 // If the buffer isn't large enough, abort.
@@ -622,8 +620,8 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, 
ProgramStateRef State,
 void CStringChecker::emitAdditionOverflowBug(CheckerContext &C,
  ProgramStateRef State) const {
   if (ExplodedNode *N = C.generateErrorNode(State)) {
-if (!BT_NotCString)
-  BT_NotCString.reset(
+if (!BT_AdditionOverflow)
+  BT_AdditionOverflow.reset(
   new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
  "Sum of expressions causes overflow."));
 
@@ -634,8 +632,8 @@ void CStringChecker::emitAdditionOverflowBug(CheckerContext 
&C,
 "This expression will create a string whose length is too big to "
 "be represented as a size_t";
 
-auto Report =
-std::make_unique(*BT_NotCString, WarningMsg, 
N);
+auto Report = 
std::make_unique(*BT_AdditionOverflow,
+   WarningMsg, N);
 C.emitReport(std::move(Report));
   }
 }



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

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

[Shivam Rajput via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6745b6a0f185: [analyzer][NFCi] Use the correct BugType in 
CStringChecker. (authored by phyBrackets).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119462

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -257,7 +257,6 @@
   void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
-
   ProgramStateRef checkAdditionOverflow(CheckerContext &C,
 ProgramStateRef state,
 NonLoc left,
@@ -420,7 +419,6 @@
 
 SVal BufEnd =
 svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
-
 State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
 // If the buffer isn't large enough, abort.
@@ -622,8 +620,8 @@
 void CStringChecker::emitAdditionOverflowBug(CheckerContext &C,
  ProgramStateRef State) const {
   if (ExplodedNode *N = C.generateErrorNode(State)) {
-if (!BT_NotCString)
-  BT_NotCString.reset(
+if (!BT_AdditionOverflow)
+  BT_AdditionOverflow.reset(
   new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
  "Sum of expressions causes overflow."));
 
@@ -634,8 +632,8 @@
 "This expression will create a string whose length is too big to "
 "be represented as a size_t";
 
-auto Report =
-std::make_unique(*BT_NotCString, WarningMsg, 
N);
+auto Report = 
std::make_unique(*BT_AdditionOverflow,
+   WarningMsg, N);
 C.emitReport(std::move(Report));
   }
 }


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -257,7 +257,6 @@
   void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
-
   ProgramStateRef checkAdditionOverflow(CheckerContext &C,
 ProgramStateRef state,
 NonLoc left,
@@ -420,7 +419,6 @@
 
 SVal BufEnd =
 svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
-
 State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
 // If the buffer isn't large enough, abort.
@@ -622,8 +620,8 @@
 void CStringChecker::emitAdditionOverflowBug(CheckerContext &C,
  ProgramStateRef State) const {
   if (ExplodedNode *N = C.generateErrorNode(State)) {
-if (!BT_NotCString)
-  BT_NotCString.reset(
+if (!BT_AdditionOverflow)
+  BT_AdditionOverflow.reset(
   new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
  "Sum of expressions causes overflow."));
 
@@ -634,8 +632,8 @@
 "This expression will create a string whose length is too big to "
 "be represented as a size_t";
 
-auto Report =
-std::make_unique(*BT_NotCString, WarningMsg, N);
+auto Report = std::make_unique(*BT_AdditionOverflow,
+   WarningMsg, N);
 C.emitReport(std::move(Report));
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D119462#3319031 , @steakhal wrote:

> I think yes, go ahead!

Thanks !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119462

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

[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch starts using the new iterator type in `ConstSearchDirIterator`.

Depends on D117566 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119721

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -704,9 +704,10 @@
 }
 
 void HeaderSearch::cacheLookupSuccess(LookupFileCacheInfo &CacheLookup,
-  unsigned HitIdx, SourceLocation Loc) {
-  CacheLookup.HitIdx = HitIdx;
-  noteLookupUsage(HitIdx, Loc);
+  ConstSearchDirIterator HitIt,
+  SourceLocation Loc) {
+  CacheLookup.HitIt = HitIt;
+  noteLookupUsage(&*HitIt - &*search_dir_begin(), Loc);
 }
 
 void HeaderSearch::noteLookupUsage(unsigned HitIdx, SourceLocation Loc) {
@@ -964,12 +965,13 @@
   CurDir = nullptr;
 
   // If this is a system #include, ignore the user #include locs.
-  unsigned i = isAngled ? AngledDirIdx : 0;
+  ConstSearchDirIterator It =
+  isAngled ? angled_dir_begin() : search_dir_begin();
 
   // If this is a #include_next request, start searching after the directory the
   // file was found in.
   if (FromDir)
-i = FromDir.Idx;
+It = FromDir;
 
   // Cache all of the lookups performed by this method.  Many headers are
   // multiply included, and the "pragma once" optimization prevents them from
@@ -977,12 +979,14 @@
   // (potentially huge) series of SearchDirs to find it.
   LookupFileCacheInfo &CacheLookup = LookupFileCache[Filename];
 
+  ConstSearchDirIterator NextIt = [](auto ItCopy) { return ++ItCopy; }(It);
+
   // If the entry has been previously looked up, the first value will be
   // non-zero.  If the value is equal to i (the start point of our search), then
   // this is a matching hit.
-  if (!SkipCache && CacheLookup.StartIdx == i+1) {
+  if (!SkipCache && CacheLookup.StartIt == NextIt) {
 // Skip querying potentially lots of directories for this lookup.
-i = CacheLookup.HitIdx;
+It = CacheLookup.HitIt;
 if (CacheLookup.MappedName) {
   Filename = CacheLookup.MappedName;
   if (IsMapped)
@@ -992,17 +996,17 @@
 // Otherwise, this is the first query, or the previous query didn't match
 // our search start.  We will fill in our found location below, so prime the
 // start point value.
-CacheLookup.reset(/*StartIdx=*/i+1);
+CacheLookup.reset(/*NewStartIt=*/NextIt);
   }
 
   SmallString<64> MappedName;
 
   // Check each directory in sequence to see if it contains this file.
-  for (; i != SearchDirs.size(); ++i) {
+  for (; It != search_dir_end(); ++It) {
 bool InUserSpecifiedSystemFramework = false;
 bool IsInHeaderMap = false;
 bool IsFrameworkFoundInDir = false;
-Optional File = SearchDirs[i].LookupFile(
+Optional File = It->LookupFile(
 Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
 SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
 IsInHeaderMap, MappedName);
@@ -1024,7 +1028,7 @@
 if (!File)
   continue;
 
-CurDir = ConstSearchDirIterator(*this, i);
+CurDir = It;
 
 // This file is a system header or C++ unfriendly if the dir is.
 HeaderFileInfo &HFI = getFileInfo(&File->getFileEntry());
@@ -1077,7 +1081,7 @@
   &File->getFileEntry(), isAngled, FoundByHeaderMap);
 
 // Remember this location for the next lookup we do.
-cacheLookupSuccess(CacheLookup, i, IncludeLoc);
+cacheLookupSuccess(CacheLookup, It, IncludeLoc);
 return File;
   }
 
@@ -1108,7 +1112,7 @@
   }
 
   cacheLookupSuccess(LookupFileCache[Filename],
- LookupFileCache[ScratchFilename].HitIdx, IncludeLoc);
+ LookupFileCache[ScratchFilename].HitIt, IncludeLoc);
   // FIXME: SuggestedModule.
   return File;
 }
@@ -1122,7 +1126,7 @@
   }
 
   // Otherwise, didn't find it. Remember we didn't find this.
-  CacheLookup.HitIdx = SearchDirs.size();
+  CacheLookup.HitIt = search_dir_end();
   return None;
 }
 
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -255,13 +255,13 @@
 
   /// Keeps track of each lookup performed by LookupFile.
   struct LookupFileCacheInfo {
-/// Starting index in SearchDirs that the cached search was performed from.
-/// If there is a hit and this value doesn't match the current query, the
-/// cache has to be ignored.
-unsigned S

[PATCH] D119722: [clang][lex] Use `SearchDirIterator` types in for loops

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ahoppen, Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch replaces a lot of index-based loops with iterators and ranges.

Depends on D117566 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119722

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -79,11 +79,6 @@
 
 ExternalHeaderFileInfoSource::~ExternalHeaderFileInfoSource() = default;
 
-const DirectoryLookup &ConstSearchDirIterator::operator*() const {
-  assert(*this && "Invalid iterator.");
-  return HS->SearchDirs[Idx];
-}
-
 HeaderSearch::HeaderSearch(std::shared_ptr HSOpts,
SourceManager &SourceMgr, DiagnosticsEngine &Diags,
const LangOptions &LangOpts,
@@ -304,21 +299,20 @@
SourceLocation ImportLoc,
bool AllowExtraModuleMapSearch) {
   Module *Module = nullptr;
-  unsigned Idx;
+  SearchDirIterator It = nullptr;
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (Idx = 0; Idx != SearchDirs.size(); ++Idx) {
-if (SearchDirs[Idx].isFramework()) {
+  for (It = search_dir_begin(); It != search_dir_end(); ++It) {
+if (It->isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules
   // named FooPrivate in buggy frameworks named Foo.
   SmallString<128> FrameworkDirName;
-  FrameworkDirName += SearchDirs[Idx].getFrameworkDir()->getName();
+  FrameworkDirName += It->getFrameworkDir()->getName();
   llvm::sys::path::append(FrameworkDirName, SearchName + ".framework");
   if (auto FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) {
-bool IsSystem
-  = SearchDirs[Idx].getDirCharacteristic() != SrcMgr::C_User;
+bool IsSystem = It->getDirCharacteristic() != SrcMgr::C_User;
 Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem);
 if (Module)
   break;
@@ -328,12 +322,12 @@
 // FIXME: Figure out how header maps and module maps will work together.
 
 // Only deal with normal search directories.
-if (!SearchDirs[Idx].isNormalDir())
+if (!It->isNormalDir())
   continue;
 
-bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory();
+bool IsSystem = It->isSystemHeaderDirectory();
 // Search for a module map file in this directory.
-if (loadModuleMapFile(SearchDirs[Idx].getDir(), IsSystem,
+if (loadModuleMapFile(It->getDir(), IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded) {
   // We just loaded a module map file; check whether the module is
   // available now.
@@ -345,7 +339,7 @@
 // Search for a module map in a subdirectory with the same name as the
 // module.
 SmallString<128> NestedModuleMapDirName;
-NestedModuleMapDirName = SearchDirs[Idx].getDir()->getName();
+NestedModuleMapDirName = It->getDir()->getName();
 llvm::sys::path::append(NestedModuleMapDirName, ModuleName);
 if (loadModuleMapFile(NestedModuleMapDirName, IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded){
@@ -357,13 +351,13 @@
 
 // If we've already performed the exhaustive search for module maps in this
 // search directory, don't do it again.
-if (SearchDirs[Idx].haveSearchedAllModuleMaps())
+if (It->haveSearchedAllModuleMaps())
   continue;
 
 // Load all module maps in the immediate subdirectories of this search
 // directory if ModuleName was from @import.
 if (AllowExtraModuleMapSearch)
-  loadSubdirectoryModuleMaps(SearchDirs[Idx]);
+  loadSubdirectoryModuleMaps(*It);
 
 // Look again for the module.
 Module = ModMap.findModule(ModuleName);
@@ -372,7 +366,7 @@
   }
 
   if (Module)
-noteLookupUsage(Idx, ImportLoc);
+noteLookupUsage(It.Idx, ImportLoc);
 
   return Module;
 }
@@ -707,7 +701,7 @@
   ConstSearchDirIterator HitIt,
   SourceLocation Loc) {
   CacheLookup.HitIt = HitIt;
-  noteLookupUsage(&*HitIt - &*search_dir_begin(), Loc);
+  noteLookupUsage(HitIt.Idx, Loc);
 }
 
 void HeaderSearch::noteLookupUsage(unsigned HitIdx, SourceLocation Loc) {
@@ -1453,10 +1447,7 @@
 }
 
 Optional HeaderSearch::searchDirIdx(const DirectoryLookup &DL) const {
-  for (unsigned I = 0; I < SearchDirs.size(); ++I)
-if (&SearchDirs[I] == &DL)
-  return I;
-  return None;
+  return &D

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@kparzysz I've tagged you due to the changes in RDFGraph, which I believe you 
are the owner of. The asserts are hit in 
`llvm/test/CodeGen/ARM/inlineasm-error-t-toofewregs.ll` - the Register 
Allocator Chokes on the test due to not having enough registers for the inline 
assembly. `llc` then reports the error, but continues running passes (including 
this one), which then core dumps on the asserts I've removed (The first one hit 
is in `DataFlowGraph::buildStmt`). When I run the same testcase with 
`-verify-machineinstrs`, there is no verification error, but the verifier seems 
to have decided that analysing physical reg liveness is too hard (see e.g. 
`MachineVerifier.cpp` line 2990). I haven't worked out how else to prevent this 
pass running if the register allocation is known to be wrong, without analysing 
every instruction before building the RDFGraph. I wasn't sure the right 
approach, so I wanted to post the patch and then discuss it, rather than the 
other way around (so you can see the testcase).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

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

[PATCH] D119710: [Docs][OpenCL] Release 14 notes

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:262
+- Added parsing support for optionality of device side enqueue and blocks (not
+  fully incomplete yet!).
+- Added missing support for optionality of various builtin functions:

incomplete -> complete



Comment at: clang/docs/ReleaseNotes.rst:277-279
+- Fix address space for temporaries (to be ``__private``).
+- Disallows static kernel functions.
+- Fix implicit definition of ``__cpp_threadsafe_static_init`` macro.

Nit: it would be better to use the same tense everywhere (fixed, added, ...).


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

https://reviews.llvm.org/D119710

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Tangential data point: I hacked up this check to find //all// unqualified `std` 
functions and ran it on libc++'s tests and also on some of my own codebase 
(including re2, asio, protobuf). I'm fixing the issues it turned up in libc++'s 
tests. In the other codebases, there was a lot of `min` and `max` in multiple 
codebases; several uses of `back_inserter` and `getline` across different 
codebases; scattered probably-accidental use of algorithms in protobuf (`sort`, 
`stable_sort`, `binary_search`); one file contained `using std::setw` etc.; and 
asio makes very heavy use of unqualified `declval`.
This is irrelevant to Corentin's interests with this PR; I'm just dumping the 
finding here in case anyone's ever interested in generalizing this check and 
wants to know what it would find.




Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

erichkeane wrote:
> Do we really want this?  I guess I would think doing:
> 
> 
> ```
> void MyFunc(auto whatever) {
>   auto X = move(whatever);
> ```
> 
> when I MEAN std::move, just for it to pick up a non-std::move the 1st time is 
> likely the same problem?  Or should it get a separate warning?
That's a good point (IMHO). Perhaps instead of making this a specific case of 
"warn for unqualified call to things in `std` (namely `move` and `forward`)," 
we should make it a specific case of "warn for any unqualified use of //this 
identifier// (namely `move` and `forward`)." That's closer in spirit to Nico 
Josuttis's comment that `move` is almost like a keyword in modern C++, and 
therefore shouldn't be thrown around willy-nilly. Either you mean `std::move` 
(in which case qualify it), or you mean some other `my::move` (in which case 
qualify it), but using the bare word `move` is inappropriate in modern C++ no 
matter whether it //currently// finds something out of `std` or not.
I'm ambivalent between these two ways of looking at the issue. Maybe someone 
can think up a reason to prefer one or the other?

libc++'s tests do include several recently-added instances of `move` as a 
//variable name//, e.g. `auto copy(x); auto move(std::move(x));`. This confuses 
grep but would not confuse Clang, for better and worse. I don't expect that 
real code would ever do this, either.

@erichkeane's specific example is a //template//, which means it's going to be 
picked up by D72282 `clang-tidy bugprone-unintended-adl` also. Using ADL inside 
templates triggers multiple red flags simultaneously. Whereas this D119670 is 
the only thing that's going to catch unqualified `move` in //non-template// 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670

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

[clang] 744745a - [analyzer] Add failing test case demonstrating buggy taint propagation

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-02-14T16:55:55+01:00
New Revision: 744745ae195f0997e5bfd5aa2de47b9ea156b6a6

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

LOG: [analyzer] Add failing test case demonstrating buggy taint propagation

Recently we uncovered a serious bug in the `GenericTaintChecker`.
It was already flawed before D116025, but that was the patch that turned
this silent bug into a crash.

It happens if the `GenericTaintChecker` has a rule for a function, which
also has a definition.

  char *fgets(char *s, int n, FILE *fp) {
nested_call();   // no parameters!
return (char *)0;
  }

  // Within some function:
  fgets(..., tainted_fd);

When the engine inlines the definition and finds a function call within
that, the `PostCall` event for the call will get triggered sooner than the
`PostCall` for the original function.
This mismatch violates the assumption of the `GenericTaintChecker` which
wants to propagate taint information from the `PreCall` event to the
`PostCall` event, where it can actually bind taint to the return value
**of the same call**.

Let's get back to the example and go through step-by-step.
The `GenericTaintChecker` will see the `PreCall`
event, so it would 'remember' that it needs to taint the return value
and the buffer, from the `PostCall` handler, where it has access to the
return value symbol.
However, the engine will inline fgets and the `nested_call()` gets
evaluated subsequently, which produces an unimportant
`PreCall`, then a `PostCall` event, which is
observed by the `GenericTaintChecker`, which will unconditionally mark
tainted the 'remembered' arg indexes, trying to access a non-existing
argument, resulting in a crash.
If it doesn't crash, it will behave completely unintuitively, by marking
completely unrelated memory regions tainted, which is even worse.

The resulting assertion is something like this:
  Expr.h: const Expr *CallExpr::getArg(unsigned int) const: Assertion
  `Arg < getNumArgs() && "Arg access out of range!"' failed.

The gist of the backtrace:
  CallExpr::getArg(unsigned int) const
  SimpleFunctionCall::getArgExpr(unsigned int)
  CallEvent::getArgSVal(unsigned int) const
  GenericTaintChecker::checkPostCall(const CallEvent &, CheckerContext&) const

Prior to D116025, there was a check for the argument count before it
applied taint, however, it still suffered from the same underlying
issue/bug regarding propagation.

This path does not intend to fix the bug, rather start a discussion on
how to fix this.

---

Let me elaborate on how I see this problem.

This pre-call, post-call juggling is just a workaround.
The engine should by itself propagate taint where necessary right where
it invalidates regions.
For the tracked values, which potentially escape, we need to erase the
information we know about them; and this is exactly what is done by
invalidation.
However, in the case of taint, we basically want to approximate from the
opposite side of the spectrum.
We want to preserve taint in most cases, rather than cleansing them.

Now, we basically sanitize all escaping tainted regions implicitly,
since invalidation binds a fresh conjured symbol for the given region,
and that has not been associated with taint.

IMO this is a bad default behavior, we should be more aggressive about
preserving taint if not further spreading taint to the reachable
regions.

We have a couple of options for dealing with it (let's call it //tainting
policy//):
  1) Taint only the parameters which were tainted prior to the call.
  2) Taint the return value of the call, since it likely depends on the
 tainted input - if any arguments were tainted.
  3) Taint all escaped regions - (maybe transitively using the cluster
 algorithm) - if any arguments were tainted.
  4) Not taint anything - this is what we do right now :D

The `ExprEngine` should not deal with taint on its own. It should be done
by a checker, such as the `GenericTaintChecker`.
However, the `Pre`-`PostCall` checker callbacks are not designed for this.
`RegionChanges` would be a much better fit for modeling taint propagation.
What we would need in the `RegionChanges` callback is the `State` prior
invalidation, the `State` after the invalidation, and a `CheckerContext` in
which the checker can create transitions, where it would place `NoteTags`
for the modeled taint propagations and report errors if a taint sink
rule gets violated.
In this callback, we could query from the prior State, if the given
value was tainted; then act and taint if necessary according to the
checker's tainting policy.

By using RegionChanges for this, we would 'fix' the mentioned
propagation bug 'by-design'.

Reviewed By: Szelethus

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

Added: 
clang/test/Analysis/taint-checker-callback-order-has-definition.c
clang/test/Analysi

[clang] b099e1e - [analyzer] Fix taint propagation by remembering to the location context

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-02-14T16:55:55+01:00
New Revision: b099e1e562555fbc67e2e0abbc15074e14a85ff1

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

LOG: [analyzer] Fix taint propagation by remembering to the location context

Fixes the issue D118987 by mapping the propagation to the callsite's
LocationContext.
This way we can keep track of the in-flight propagations.

Note that empty propagation sets won't be inserted.

Reviewed By: NoQ, Szelethus

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/taint-checker-callback-order-has-definition.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 428778e6cfaa..66143f78932c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -38,6 +38,8 @@ using namespace clang;
 using namespace ento;
 using namespace taint;
 
+using llvm::ImmutableSet;
+
 namespace {
 
 class GenericTaintChecker;
@@ -434,7 +436,9 @@ template <> struct 
ScalarEnumerationTraits {
 /// to the call post-visit. The values are signed integers, which are either
 /// ReturnValueIndex, or indexes of the pointer/reference argument, which
 /// points to data, which should be tainted on return.
-REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, ArgIdxTy)
+REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *,
+   ImmutableSet)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy)
 
 void GenericTaintRuleParser::validateArgVector(const std::string &Option,
const ArgVecTy &Args) const {
@@ -685,22 +689,26 @@ void GenericTaintChecker::checkPostCall(const CallEvent 
&Call,
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
   ProgramStateRef State = C.getState();
+  const StackFrameContext *CurrentFrame = C.getStackFrame();
 
   // Depending on what was tainted at pre-visit, we determined a set of
   // arguments which should be tainted after the function returns. These are
   // stored in the state as TaintArgsOnPostVisit set.
-  TaintArgsOnPostVisitTy TaintArgs = State->get();
-  if (TaintArgs.isEmpty())
+  TaintArgsOnPostVisitTy TaintArgsMap = State->get();
+
+  const ImmutableSet *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
+  if (!TaintArgs)
 return;
+  assert(!TaintArgs->isEmpty());
 
   LLVM_DEBUG(for (ArgIdxTy I
-  : TaintArgs) {
+  : *TaintArgs) {
 llvm::dbgs() << "PostCall<";
 Call.dump(llvm::dbgs());
 llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
   });
 
-  for (ArgIdxTy ArgNum : TaintArgs) {
+  for (ArgIdxTy ArgNum : *TaintArgs) {
 // Special handling for the tainted return value.
 if (ArgNum == ReturnValueIndex) {
   State = addTaint(State, Call.getReturnValue());
@@ -714,7 +722,7 @@ void GenericTaintChecker::checkPostCall(const CallEvent 
&Call,
   }
 
   // Clear up the taint info from the state.
-  State = State->remove();
+  State = State->remove(CurrentFrame);
   C.addTransition(State);
 }
 
@@ -776,28 +784,33 @@ void GenericTaintRule::process(const GenericTaintChecker 
&Checker,
   };
 
   /// Propagate taint where it is necessary.
+  auto &F = State->getStateManager().get_context();
+  ImmutableSet Result = F.getEmptySet();
   ForEachCallArg(
-  [this, &State, WouldEscape, &Call](ArgIdxTy I, const Expr *E, SVal V) {
+  [this, WouldEscape, &Call, &Result, &F](ArgIdxTy I, const Expr *E,
+  SVal V) {
 if (PropDstArgs.contains(I)) {
   LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs());
  llvm::dbgs()
  << "> prepares tainting arg index: " << I << '\n';);
-  State = State->add(I);
+  Result = F.add(Result, I);
 }
 
 // TODO: We should traverse all reachable memory regions via the
 // escaping parameter. Instead of doing that we simply mark only the
 // referred memory region as tainted.
 if (WouldEscape(V, E->getType())) {
-  LLVM_DEBUG(if (!State->contains(I)) {
+  LLVM_DEBUG(if (!Result.contains(I)) {
 llvm::dbgs() << "PreCall<";
 Call.dump(llvm::dbgs());
 llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';
   });
-  State = State->add(I);
+  Result = F.add(Result, I);
 }
   });
 
+  if (!Result.isEmpty())
+State = State->set(C.getStackFram

[clang] bf5963b - [analyzer] Fix taint rule of fgets and setproctitle_init

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-02-14T16:55:55+01:00
New Revision: bf5963bf19670ea58facdf57492e147c13bb650f

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

LOG: [analyzer] Fix taint rule of fgets and setproctitle_init

There was a typo in the rule.
`{{0}, ReturnValueIndex}` meant that the discrete index is `0` and the
variadic index is `-1`.
What we wanted instead is that both `0` and `-1` are in the discrete index
list.

Instead of this, we wanted to express that both `0` and the
`ReturnValueIndex` is in the discrete arg list.

The manual inspection revealed that `setproctitle_init` also suffered a
probably incomplete propagation rule.

Reviewed By: Szelethus, gamesh411

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/taint-checker-callback-order-has-definition.c
clang/test/Analysis/taint-checker-callback-order-without-definition.c
clang/test/Analysis/taint-generic.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 66143f78932c..d15a4659a96e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -559,7 +559,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) 
const {
   {{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
   {{"fgetc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
   {{"fgetln"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{"fgets"}, TR::Prop({{2}}, {{0}, ReturnValueIndex})},
+  {{"fgets"}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
   {{"fscanf"}, TR::Prop({{0}}, {{}, 2})},
   {{"sscanf"}, TR::Prop({{0}}, {{}, 2})},
   {{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
@@ -632,7 +632,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) 
const {
   if (TR::UntrustedEnv(C)) {
 // void setproctitle_init(int argc, char *argv[], char *envp[])
 GlobalCRules.push_back(
-{{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)});
+{{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)});
 GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})});
   }
 

diff  --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c 
b/clang/test/Analysis/taint-checker-callback-order-has-definition.c
index a070077004fa..cdf55e3aef41 100644
--- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c
+++ b/clang/test/Analysis/taint-checker-callback-order-has-definition.c
@@ -25,11 +25,9 @@ void top(const char *fname, char *buf) {
   (void)fgets(buf, 42, fp); // Trigger taint propagation.
   // CHECK-NEXT: PreCall prepares tainting arg index: -1
   // CHECK-NEXT: PreCall prepares tainting arg index: 0
-  // CHECK-NEXT: PreCall prepares tainting arg index: 1
   // CHECK-NEXT: PreCall prepares tainting arg index: 2
-
+  //
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: -1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg 
index: 1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 2
 }

diff  --git 
a/clang/test/Analysis/taint-checker-callback-order-without-definition.c 
b/clang/test/Analysis/taint-checker-callback-order-without-definition.c
index 962e8b259298..f2e070739e97 100644
--- a/clang/test/Analysis/taint-checker-callback-order-without-definition.c
+++ b/clang/test/Analysis/taint-checker-callback-order-without-definition.c
@@ -19,16 +19,11 @@ void top(const char *fname, char *buf) {
 
   (void)fgets(buf, 42, fp); // Trigger taint propagation.
 
-  // FIXME: Why is the arg index 1 prepared for taint?
-  // Before the call it wasn't tainted, and it also shouldn't be tainted after 
the call.
-
   // CHECK-NEXT: PreCall prepares tainting arg index: -1
   // CHECK-NEXT: PreCall prepares tainting arg index: 0
-  // CHECK-NEXT: PreCall prepares tainting arg index: 1
   // CHECK-NEXT: PreCall prepares tainting arg index: 2
   //
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: -1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg 
index: 1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 2
 }

diff  --git a/clang/test/Analysis/taint-generic.c 
b/clang/test/Analysis/taint-generic.c
index 6979c0667764..0612e1b9f98b 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -58,9 +58,11 @@ extern FILE *stdin;
 
 #define bool _Bool
 
+char *getenv(const char *name);
 int fscanf(FILE *restrict stream, const char *restrict format, ...);
 int

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG744745ae195f: [analyzer] Add failing test case demonstrating 
buggy taint propagation (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118987

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/test/Analysis/taint-checker-callback-order-has-definition.c
  clang/test/Analysis/taint-checker-callback-order-without-definition.c

Index: clang/test/Analysis/taint-checker-callback-order-without-definition.c
===
--- /dev/null
+++ clang/test/Analysis/taint-checker-callback-order-without-definition.c
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,alpha.security.taint \
+// RUN:   -mllvm -debug-only=taint-checker \
+// RUN:   2>&1 | FileCheck %s
+
+struct _IO_FILE;
+typedef struct _IO_FILE FILE;
+FILE *fopen(const char *fname, const char *mode);
+
+char *fgets(char *s, int n, FILE *fp); // no-definition
+
+void top(const char *fname, char *buf) {
+  FILE *fp = fopen(fname, "r"); // Introduce taint.
+  // CHECK:  PreCall prepares tainting arg index: -1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
+
+  if (!fp)
+return;
+
+  (void)fgets(buf, 42, fp); // Trigger taint propagation.
+
+  // FIXME: Why is the arg index 1 prepared for taint?
+  // Before the call it wasn't tainted, and it also shouldn't be tainted after the call.
+
+  // CHECK-NEXT: PreCall prepares tainting arg index: -1
+  // CHECK-NEXT: PreCall prepares tainting arg index: 0
+  // CHECK-NEXT: PreCall prepares tainting arg index: 1
+  // CHECK-NEXT: PreCall prepares tainting arg index: 2
+  //
+  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 0
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 2
+}
Index: clang/test/Analysis/taint-checker-callback-order-has-definition.c
===
--- /dev/null
+++ clang/test/Analysis/taint-checker-callback-order-has-definition.c
@@ -0,0 +1,42 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,alpha.security.taint \
+// RUN:   -mllvm -debug-only=taint-checker \
+// RUN:   2>&1 | FileCheck %s
+
+// FIXME: We should not crash.
+// XFAIL: *
+
+struct _IO_FILE;
+typedef struct _IO_FILE FILE;
+FILE *fopen(const char *fname, const char *mode);
+
+void nested_call(void) {}
+
+char *fgets(char *s, int n, FILE *fp) {
+  nested_call();   // no-crash: we should not try adding taint to a non-existent argument.
+  return (char *)0;
+}
+
+void top(const char *fname, char *buf) {
+  FILE *fp = fopen(fname, "r");
+  // CHECK:  PreCall prepares tainting arg index: -1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
+
+  if (!fp)
+return;
+
+  (void)fgets(buf, 42, fp); // Trigger taint propagation.
+  // CHECK-NEXT: PreCall prepares tainting arg index: -1
+  // CHECK-NEXT: PreCall prepares tainting arg index: 0
+  // CHECK-NEXT: PreCall prepares tainting arg index: 1
+  // CHECK-NEXT: PreCall prepares tainting arg index: 2
+
+  // FIXME: We should propagate taint from PreCall -> PostCall.
+  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 0
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 2
+
+  // FIXME: We should not crash.
+  // CHECK: PLEASE submit a bug report
+}
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#define DEBUG_TYPE "taint-checker"
+
 using namespace clang;
 using namespace ento;
 using namespace taint;
@@ -691,6 +693,13 @@
   if (TaintArgs.isEmpty())
 return;
 
+  LLVM_DEBUG(for (ArgIdxTy I
+  : TaintArgs) {
+llvm::dbgs() << "PostCall<";
+Call.dump(llvm::dbgs());
+llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
+  });
+
   for (ArgIdxTy ArgNum : TaintArgs) {
 // Special handling for the tainted return value.
 if (ArgNum == ReturnValueIndex) {
@@ -768,15 +777,25 @@
 
   /// Propagate taint where it is necessary.
   ForEachCallArg(
-  [this, &State, WouldEscape](ArgIdxTy I, const Expr *E, SVal V) {
-if (PropDstArgs.contains(I))
+  [this, &State, WouldEscape, &Call](ArgIdxTy I, const Expr *E, SVal V) {
+if (PropDstArgs.con

[PATCH] D119128: [analyzer] Fix taint propagation by remembering to the location context

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb099e1e56255: [analyzer] Fix taint propagation by 
remembering to the location context (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119128

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/test/Analysis/taint-checker-callback-order-has-definition.c

Index: clang/test/Analysis/taint-checker-callback-order-has-definition.c
===
--- clang/test/Analysis/taint-checker-callback-order-has-definition.c
+++ clang/test/Analysis/taint-checker-callback-order-has-definition.c
@@ -3,9 +3,6 @@
 // RUN:   -mllvm -debug-only=taint-checker \
 // RUN:   2>&1 | FileCheck %s
 
-// FIXME: We should not crash.
-// XFAIL: *
-
 struct _IO_FILE;
 typedef struct _IO_FILE FILE;
 FILE *fopen(const char *fname, const char *mode);
@@ -31,12 +28,8 @@
   // CHECK-NEXT: PreCall prepares tainting arg index: 1
   // CHECK-NEXT: PreCall prepares tainting arg index: 2
 
-  // FIXME: We should propagate taint from PreCall -> PostCall.
-  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 1
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 2
-
-  // FIXME: We should not crash.
-  // CHECK: PLEASE submit a bug report
+  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 0
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 1
+  // CHECK-NEXT: PostCall actually wants to taint arg index: 2
 }
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -38,6 +38,8 @@
 using namespace ento;
 using namespace taint;
 
+using llvm::ImmutableSet;
+
 namespace {
 
 class GenericTaintChecker;
@@ -434,7 +436,9 @@
 /// to the call post-visit. The values are signed integers, which are either
 /// ReturnValueIndex, or indexes of the pointer/reference argument, which
 /// points to data, which should be tainted on return.
-REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, ArgIdxTy)
+REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *,
+   ImmutableSet)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy)
 
 void GenericTaintRuleParser::validateArgVector(const std::string &Option,
const ArgVecTy &Args) const {
@@ -685,22 +689,26 @@
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
   ProgramStateRef State = C.getState();
+  const StackFrameContext *CurrentFrame = C.getStackFrame();
 
   // Depending on what was tainted at pre-visit, we determined a set of
   // arguments which should be tainted after the function returns. These are
   // stored in the state as TaintArgsOnPostVisit set.
-  TaintArgsOnPostVisitTy TaintArgs = State->get();
-  if (TaintArgs.isEmpty())
+  TaintArgsOnPostVisitTy TaintArgsMap = State->get();
+
+  const ImmutableSet *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
+  if (!TaintArgs)
 return;
+  assert(!TaintArgs->isEmpty());
 
   LLVM_DEBUG(for (ArgIdxTy I
-  : TaintArgs) {
+  : *TaintArgs) {
 llvm::dbgs() << "PostCall<";
 Call.dump(llvm::dbgs());
 llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
   });
 
-  for (ArgIdxTy ArgNum : TaintArgs) {
+  for (ArgIdxTy ArgNum : *TaintArgs) {
 // Special handling for the tainted return value.
 if (ArgNum == ReturnValueIndex) {
   State = addTaint(State, Call.getReturnValue());
@@ -714,7 +722,7 @@
   }
 
   // Clear up the taint info from the state.
-  State = State->remove();
+  State = State->remove(CurrentFrame);
   C.addTransition(State);
 }
 
@@ -776,28 +784,33 @@
   };
 
   /// Propagate taint where it is necessary.
+  auto &F = State->getStateManager().get_context();
+  ImmutableSet Result = F.getEmptySet();
   ForEachCallArg(
-  [this, &State, WouldEscape, &Call](ArgIdxTy I, const Expr *E, SVal V) {
+  [this, WouldEscape, &Call, &Result, &F](ArgIdxTy I, const Expr *E,
+  SVal V) {
 if (PropDstArgs.contains(I)) {
   LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs());
  llvm::dbgs()
  << "> prepares tainting arg index: " << I << '\n';);
-  State = State->add(I);
+  Result = F.add(Result, I);
 }
 
 // TODO: We should trav

[PATCH] D119129: [analyzer] Fix taint rule of fgets and setproctitle_init

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf5963bf1967: [analyzer] Fix taint rule of fgets and 
setproctitle_init (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119129

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/test/Analysis/taint-checker-callback-order-has-definition.c
  clang/test/Analysis/taint-checker-callback-order-without-definition.c
  clang/test/Analysis/taint-generic.c


Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -58,9 +58,11 @@
 
 #define bool _Bool
 
+char *getenv(const char *name);
 int fscanf(FILE *restrict stream, const char *restrict format, ...);
 int sprintf(char *str, const char *format, ...);
 void setproctitle(const char *fmt, ...);
+void setproctitle_init(int argc, char *argv[], char *envp[]);
 typedef __typeof(sizeof(int)) size_t;
 
 // Define string functions. Use builtin for some of them. They all default to
@@ -404,3 +406,20 @@
 void testUnknownFunction(void (*foo)(void)) {
   foo(); // no-crash
 }
+
+void testProctitleFalseNegative() {
+  char flag[80];
+  fscanf(stdin, "%79s", flag);
+  char *argv[] = {"myapp", flag};
+  // FIXME: We should have a warning below: Untrusted data passed to sink.
+  setproctitle_init(1, argv, 0);
+}
+
+void testProctitle2(char *real_argv[]) {
+  char *app = getenv("APP_NAME");
+  if (!app)
+return;
+  char *argv[] = {app, "--foobar"};
+  setproctitle_init(1, argv, 0); // expected-warning {{Untrusted data 
is passed to a user-defined sink}}
+  setproctitle_init(1, real_argv, argv); // expected-warning {{Untrusted data 
is passed to a user-defined sink}}
+}
Index: clang/test/Analysis/taint-checker-callback-order-without-definition.c
===
--- clang/test/Analysis/taint-checker-callback-order-without-definition.c
+++ clang/test/Analysis/taint-checker-callback-order-without-definition.c
@@ -19,16 +19,11 @@
 
   (void)fgets(buf, 42, fp); // Trigger taint propagation.
 
-  // FIXME: Why is the arg index 1 prepared for taint?
-  // Before the call it wasn't tainted, and it also shouldn't be tainted after 
the call.
-
   // CHECK-NEXT: PreCall prepares tainting arg index: -1
   // CHECK-NEXT: PreCall prepares tainting arg index: 0
-  // CHECK-NEXT: PreCall prepares tainting arg index: 1
   // CHECK-NEXT: PreCall prepares tainting arg index: 2
   //
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: -1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg 
index: 1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 2
 }
Index: clang/test/Analysis/taint-checker-callback-order-has-definition.c
===
--- clang/test/Analysis/taint-checker-callback-order-has-definition.c
+++ clang/test/Analysis/taint-checker-callback-order-has-definition.c
@@ -25,11 +25,9 @@
   (void)fgets(buf, 42, fp); // Trigger taint propagation.
   // CHECK-NEXT: PreCall prepares tainting arg index: -1
   // CHECK-NEXT: PreCall prepares tainting arg index: 0
-  // CHECK-NEXT: PreCall prepares tainting arg index: 1
   // CHECK-NEXT: PreCall prepares tainting arg index: 2
-
+  //
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: -1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg 
index: 1
   // CHECK-NEXT: PostCall actually wants to taint arg 
index: 2
 }
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -559,7 +559,7 @@
   {{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
   {{"fgetc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
   {{"fgetln"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{"fgets"}, TR::Prop({{2}}, {{0}, ReturnValueIndex})},
+  {{"fgets"}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
   {{"fscanf"}, TR::Prop({{0}}, {{}, 2})},
   {{"sscanf"}, TR::Prop({{0}}, {{}, 2})},
   {{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
@@ -632,7 +632,7 @@
   if (TR::UntrustedEnv(C)) {
 // void setproctitle_init(int argc, char *argv[], char *envp[])
 GlobalCRules.push_back(
-{{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)});
+{{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)});
 GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})});
   }
 


Index: clang/test/Analysis/taint-generic.c
=

[clang] a31d00d - Fix test failure for targets with varying uwtable defaults

[Momchil Velikov via cfe-commits]

Author: Momchil Velikov
Date: 2022-02-14T15:59:26Z
New Revision: a31d00ddceb057291ef3a094c57ae36975e804fe

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

LOG: Fix test failure for targets with varying uwtable defaults

Depending on toolchain and ABI, a target might not output DWARF unwind tables 
by default.
Run the test for a target with a known behaviour, test coverage is not reduced.

Reviewed By: dmgreen

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

Added: 


Modified: 
clang/test/CodeGen/uwtable-attr.c

Removed: 




diff  --git a/clang/test/CodeGen/uwtable-attr.c 
b/clang/test/CodeGen/uwtable-attr.c
index 7436db979b6b9..e1b0c92f9794f 100644
--- a/clang/test/CodeGen/uwtable-attr.c
+++ b/clang/test/CodeGen/uwtable-attr.c
@@ -1,13 +1,15 @@
 // Test that function and modules attributes react on the command-line options,
 // it does not state the current behaviour makes sense in all cases (it does 
not).
 
-// RUN: %clang -S -emit-llvm -o - %s   
 | FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - %s -funwind-tables
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - %s -fno-unwind-tables 
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s  
  | FileCheck %s -check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -funwind-tables
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -fno-unwind-tables 
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
 
-// RUN: %clang -S -emit-llvm -o - -x c++ %s
 | FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -funwind-tables   
 -fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -fno-exceptions 
-fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck %s 
-check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s   
  | FileCheck %s 
-check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s   
   -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s 
-check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s  
-fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck 
%s -check-prefixes=CHECK,NO_TABLES
+
+// REQUIRES: x86-registered-target
 
 #ifdef __cplusplus
 extern "C" void g(void);



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

[PATCH] D119724: Fix test failure for targets with varying uwtable defaults

[Momchil Velikov 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 rGa31d00ddceb0: Fix test failure for targets with varying 
uwtable defaults (authored by chill).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119724

Files:
  clang/test/CodeGen/uwtable-attr.c


Index: clang/test/CodeGen/uwtable-attr.c
===
--- clang/test/CodeGen/uwtable-attr.c
+++ clang/test/CodeGen/uwtable-attr.c
@@ -1,13 +1,15 @@
 // Test that function and modules attributes react on the command-line options,
 // it does not state the current behaviour makes sense in all cases (it does 
not).
 
-// RUN: %clang -S -emit-llvm -o - %s   
 | FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - %s -funwind-tables
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - %s -fno-unwind-tables 
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s  
  | FileCheck %s -check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -funwind-tables
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -fno-unwind-tables 
-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
 
-// RUN: %clang -S -emit-llvm -o - -x c++ %s
 | FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -funwind-tables   
 -fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -fno-exceptions 
-fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck %s 
-check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s   
  | FileCheck %s 
-check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s   
   -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s 
-check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s  
-fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck 
%s -check-prefixes=CHECK,NO_TABLES
+
+// REQUIRES: x86-registered-target
 
 #ifdef __cplusplus
 extern "C" void g(void);


Index: clang/test/CodeGen/uwtable-attr.c
===
--- clang/test/CodeGen/uwtable-attr.c
+++ clang/test/CodeGen/uwtable-attr.c
@@ -1,13 +1,15 @@
 // Test that function and modules attributes react on the command-line options,
 // it does not state the current behaviour makes sense in all cases (it does not).
 
-// RUN: %clang -S -emit-llvm -o - %s| FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - %s -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - %s -fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s| FileCheck %s -check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - %s -fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
 
-// RUN: %clang -S -emit-llvm -o - -x c++ %s | FileCheck %s -check-prefixes=CHECK,DEFAULT
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
-// RUN: %clang -S -emit-llvm -o - -x c++ %s  -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,NO_TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s | FileCheck %s -check-prefixes=CHECK,DEFAULT
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s  -funwind-tables-fno-asynchronous-unwind-tables | FileCheck %s -check-prefixes=CHECK,TABLES
+// RUN: %clang -target x86_64-linux -S -emit-llvm -o - -x c++ %s 

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:4007
 
+  * ``bool AfterRequiresKeywordInRequiresClause`` If ``true``, put space 
between requires keyword in a requires clause and
+opening parentheses, if there is one.

curdeius wrote:
> Wouldn't `AfterRequiresInClause`/`AfterRequiresInExpression` be meaningful 
> enough?
+1, I have no problem with `AfterRequiresInClause` and 
`AfterRequiresInExpression`!


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

https://reviews.llvm.org/D113369

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

Quuxplusone wrote:
> erichkeane wrote:
> > Do we really want this?  I guess I would think doing:
> > 
> > 
> > ```
> > void MyFunc(auto whatever) {
> >   auto X = move(whatever);
> > ```
> > 
> > when I MEAN std::move, just for it to pick up a non-std::move the 1st time 
> > is likely the same problem?  Or should it get a separate warning?
> That's a good point (IMHO). Perhaps instead of making this a specific case of 
> "warn for unqualified call to things in `std` (namely `move` and `forward`)," 
> we should make it a specific case of "warn for any unqualified use of //this 
> identifier// (namely `move` and `forward`)." That's closer in spirit to Nico 
> Josuttis's comment that `move` is almost like a keyword in modern C++, and 
> therefore shouldn't be thrown around willy-nilly. Either you mean `std::move` 
> (in which case qualify it), or you mean some other `my::move` (in which case 
> qualify it), but using the bare word `move` is inappropriate in modern C++ no 
> matter whether it //currently// finds something out of `std` or not.
> I'm ambivalent between these two ways of looking at the issue. Maybe someone 
> can think up a reason to prefer one or the other?
> 
> libc++'s tests do include several recently-added instances of `move` as a 
> //variable name//, e.g. `auto copy(x); auto move(std::move(x));`. This 
> confuses grep but would not confuse Clang, for better and worse. I don't 
> expect that real code would ever do this, either.
> 
> @erichkeane's specific example is a //template//, which means it's going to 
> be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. Using ADL 
> inside templates triggers multiple red flags simultaneously. Whereas this 
> D119670 is the only thing that's going to catch unqualified `move` in 
> //non-template// code.
> That's a good point (IMHO). Perhaps instead of making this a specific case of 
> "warn for unqualified call to things in `std` (namely `move` and `forward`)," 
> we should make it a specific case of "warn for any unqualified use of //this 
> identifier// (namely `move` and `forward`)." That's closer in spirit to Nico 
> Josuttis's comment that `move` is almost like a keyword in modern C++, and 
> therefore shouldn't be thrown around willy-nilly. Either you mean `std::move` 
> (in which case qualify it), or you mean some other `my::move` (in which case 
> qualify it), but using the bare word `move` is inappropriate in modern C++ no 
> matter whether it //currently// finds something out of `std` or not.

Ah! I guess that was just my interpretation of how this patch worked: Point out 
troublesome 'keyword-like-library-functions' used unqualified.  I think the 
alternative (warn for unqualified call to things in std) is so incredibly noisy 
as to be worthless, particularly in light of 'using' statements.

> @erichkeane's specific example is a //template//, which means it's going to 
> be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. Using ADL 
> inside templates triggers multiple red flags simultaneously. Whereas this 
> D119670 is the only thing that's going to catch unqualified `move` in 
> //non-template// code.

This was a template for convenience sake (so y'all couldn't "well actually" me 
on the type I chose), but good to know we have a warning like that!

What I was TRYING to point out a case where the person is using `move` or 
`forward` intending to have the `std` version, but accidentially ending up with 
thier own version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670

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

[PATCH] D119726: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added a reviewer: melver.
Herald added a subscriber: hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

For ASan this will effectively serve as a synonym for
__attribute__((no_sanitize("address")))

This is a reland of https://reviews.llvm.org/D114421


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119726

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2890,6 +2890,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int ignorelisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,31 +51,33 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[IGNORELISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false

[PATCH] D119541: [RISCV] Fix RISCVTargetInfo::initFeatureMap, add non-ISA features back after implication

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/test/Driver/riscv-default-features.c:4
+// RV32: "target-features"="+a,+c,+m,+relax,-save-restore"
+// RV64: "target-features"="+64bit,+a,+c,+m,+relax,-save-restore"
+

I think we may be missing are missing a `RUN` line for this case, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119541

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

erichkeane wrote:
> Quuxplusone wrote:
> > erichkeane wrote:
> > > Do we really want this?  I guess I would think doing:
> > > 
> > > 
> > > ```
> > > void MyFunc(auto whatever) {
> > >   auto X = move(whatever);
> > > ```
> > > 
> > > when I MEAN std::move, just for it to pick up a non-std::move the 1st 
> > > time is likely the same problem?  Or should it get a separate warning?
> > That's a good point (IMHO). Perhaps instead of making this a specific case 
> > of "warn for unqualified call to things in `std` (namely `move` and 
> > `forward`)," we should make it a specific case of "warn for any unqualified 
> > use of //this identifier// (namely `move` and `forward`)." That's closer in 
> > spirit to Nico Josuttis's comment that `move` is almost like a keyword in 
> > modern C++, and therefore shouldn't be thrown around willy-nilly. Either 
> > you mean `std::move` (in which case qualify it), or you mean some other 
> > `my::move` (in which case qualify it), but using the bare word `move` is 
> > inappropriate in modern C++ no matter whether it //currently// finds 
> > something out of `std` or not.
> > I'm ambivalent between these two ways of looking at the issue. Maybe 
> > someone can think up a reason to prefer one or the other?
> > 
> > libc++'s tests do include several recently-added instances of `move` as a 
> > //variable name//, e.g. `auto copy(x); auto move(std::move(x));`. This 
> > confuses grep but would not confuse Clang, for better and worse. I don't 
> > expect that real code would ever do this, either.
> > 
> > @erichkeane's specific example is a //template//, which means it's going to 
> > be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. Using ADL 
> > inside templates triggers multiple red flags simultaneously. Whereas this 
> > D119670 is the only thing that's going to catch unqualified `move` in 
> > //non-template// code.
> > That's a good point (IMHO). Perhaps instead of making this a specific case 
> > of "warn for unqualified call to things in `std` (namely `move` and 
> > `forward`)," we should make it a specific case of "warn for any unqualified 
> > use of //this identifier// (namely `move` and `forward`)." That's closer in 
> > spirit to Nico Josuttis's comment that `move` is almost like a keyword in 
> > modern C++, and therefore shouldn't be thrown around willy-nilly. Either 
> > you mean `std::move` (in which case qualify it), or you mean some other 
> > `my::move` (in which case qualify it), but using the bare word `move` is 
> > inappropriate in modern C++ no matter whether it //currently// finds 
> > something out of `std` or not.
> 
> Ah! I guess that was just my interpretation of how this patch worked: Point 
> out troublesome 'keyword-like-library-functions' used unqualified.  I think 
> the alternative (warn for unqualified call to things in std) is so incredibly 
> noisy as to be worthless, particularly in light of 'using' statements.
> 
> > @erichkeane's specific example is a //template//, which means it's going to 
> > be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. Using ADL 
> > inside templates triggers multiple red flags simultaneously. Whereas this 
> > D119670 is the only thing that's going to catch unqualified `move` in 
> > //non-template// code.
> 
> This was a template for convenience sake (so y'all couldn't "well actually" 
> me on the type I chose), but good to know we have a warning like that!
> 
> What I was TRYING to point out a case where the person is using `move` or 
> `forward` intending to have the `std` version, but accidentially ending up 
> with thier own version.
It is *likely* the same problem. The problem is the "likely" does a lot of 
heavy lifting here.
I think there is general agreement that std::move should be called qualified, 
not that `move` is somehow a special identifier that users should not use. 
These are almost contradictory statements - aka if we wanted to discourage 
people to name their function move, we wouldn't also need or want to force them 
to qualify their call.

It is very possible that `std::move` cannot be used at all in the TU because it 
is simply not declared, in which case the code would be perfectly fine.
A slightly more involved approach would be to try to detect whether std::move 
is declared by doing a lookup and warn in that case, but I'm not sure that 
brings much.
I certainly disagree that calling a function `move` is cause for warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670

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

[PATCH] D119727: [RISCV] Add the policy operand for nomask vector Multiply-Add IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, rogfer01, frasercrmck, kito-cheng, 
arcbbb, monkchiang, eopXD.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
khchen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, MaskRay.
Herald added projects: clang, LLVM.

The goal is support tail and mask policy in RVV builtins.
We focus on IR part first.

The nomask vector Multiply-Add need a policy operand because merge value could 
not be undef.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119727

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwmacc.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/memory-args.ll
  llvm/test/CodeGen/RISCV/rvv/unmasked-ta.ll
  llvm/test/CodeGen/RISCV/rvv/vfmacc.ll
  llvm/test/CodeGen/RISCV/rvv/vfmadd.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsac.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsub.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmacc.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmadd.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsac.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsub.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmacc.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmsac.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmacc.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmsac.ll
  llvm/test/CodeGen/RISCV/rvv/vmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmadd-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmadd-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsub-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsub-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
  llvm/test/CodeGen/RISCV/rvv/vwmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vwmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccsu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccsu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccus-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vwmaccus-rv64.ll

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Augie Fackler via Phabricator via cfe-commits]

durin42 added a comment.

As far as I can tell this patch broke the Rust compiler, but from the commit 
message it sounds like it shouldn't have?

https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8358#e85ad6f3-9992-4ea1-9cd3-d8db9f45f33e
 fails with

  Attribute 'uwtable' should have an Argument
  i8* (i64, i64)* @__rust_alloc
  in function __rust_alloc
  LLVM ERROR: Broken function found, compilation aborted!

Any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

cor3ntin wrote:
> erichkeane wrote:
> > Quuxplusone wrote:
> > > erichkeane wrote:
> > > > Do we really want this?  I guess I would think doing:
> > > > 
> > > > 
> > > > ```
> > > > void MyFunc(auto whatever) {
> > > >   auto X = move(whatever);
> > > > ```
> > > > 
> > > > when I MEAN std::move, just for it to pick up a non-std::move the 1st 
> > > > time is likely the same problem?  Or should it get a separate warning?
> > > That's a good point (IMHO). Perhaps instead of making this a specific 
> > > case of "warn for unqualified call to things in `std` (namely `move` and 
> > > `forward`)," we should make it a specific case of "warn for any 
> > > unqualified use of //this identifier// (namely `move` and `forward`)." 
> > > That's closer in spirit to Nico Josuttis's comment that `move` is almost 
> > > like a keyword in modern C++, and therefore shouldn't be thrown around 
> > > willy-nilly. Either you mean `std::move` (in which case qualify it), or 
> > > you mean some other `my::move` (in which case qualify it), but using the 
> > > bare word `move` is inappropriate in modern C++ no matter whether it 
> > > //currently// finds something out of `std` or not.
> > > I'm ambivalent between these two ways of looking at the issue. Maybe 
> > > someone can think up a reason to prefer one or the other?
> > > 
> > > libc++'s tests do include several recently-added instances of `move` as a 
> > > //variable name//, e.g. `auto copy(x); auto move(std::move(x));`. This 
> > > confuses grep but would not confuse Clang, for better and worse. I don't 
> > > expect that real code would ever do this, either.
> > > 
> > > @erichkeane's specific example is a //template//, which means it's going 
> > > to be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. 
> > > Using ADL inside templates triggers multiple red flags simultaneously. 
> > > Whereas this D119670 is the only thing that's going to catch unqualified 
> > > `move` in //non-template// code.
> > > That's a good point (IMHO). Perhaps instead of making this a specific 
> > > case of "warn for unqualified call to things in `std` (namely `move` and 
> > > `forward`)," we should make it a specific case of "warn for any 
> > > unqualified use of //this identifier// (namely `move` and `forward`)." 
> > > That's closer in spirit to Nico Josuttis's comment that `move` is almost 
> > > like a keyword in modern C++, and therefore shouldn't be thrown around 
> > > willy-nilly. Either you mean `std::move` (in which case qualify it), or 
> > > you mean some other `my::move` (in which case qualify it), but using the 
> > > bare word `move` is inappropriate in modern C++ no matter whether it 
> > > //currently// finds something out of `std` or not.
> > 
> > Ah! I guess that was just my interpretation of how this patch worked: Point 
> > out troublesome 'keyword-like-library-functions' used unqualified.  I think 
> > the alternative (warn for unqualified call to things in std) is so 
> > incredibly noisy as to be worthless, particularly in light of 'using' 
> > statements.
> > 
> > > @erichkeane's specific example is a //template//, which means it's going 
> > > to be picked up by D72282 `clang-tidy bugprone-unintended-adl` also. 
> > > Using ADL inside templates triggers multiple red flags simultaneously. 
> > > Whereas this D119670 is the only thing that's going to catch unqualified 
> > > `move` in //non-template// code.
> > 
> > This was a template for convenience sake (so y'all couldn't "well actually" 
> > me on the type I chose), but good to know we have a warning like that!
> > 
> > What I was TRYING to point out a case where the person is using `move` or 
> > `forward` intending to have the `std` version, but accidentially ending up 
> > with thier own version.
> It is *likely* the same problem. The problem is the "likely" does a lot of 
> heavy lifting here.
> I think there is general agreement that std::move should be called qualified, 
> not that `move` is somehow a special identifier that users should not use. 
> These are almost contradictory statements - aka if we wanted to discourage 
> people to name their function move, we wouldn't also need or want to force 
> them to qualify their call.
> 
> It is very possible that `std::move` cannot be used at all in the TU because 
> it is simply not declared, in which case the code would be perfectly fine.
> A slightly more involved approach would be to try to detect whether std::move 
> is declared by doing a lookup and warn in that case, but I'm not sure that 
> brings much.
> I certainly disagree that calling a function `move` is cause for warning.
> I certainly disagree that calling a function `move` is cause for warning.

Right, agreed. For example, `boost::move` exists, and that's 

[clang] 528deed - [clang] [test] Fix an apparent typo in SemaCXX/consteval-return-void.cpp.

[Arthur O'Dwyer via cfe-commits]

Author: Arthur O'Dwyer
Date: 2022-02-14T11:28:23-05:00
New Revision: 528deedd582f6f11cf1bba478e5e4645f4baf04f

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

LOG: [clang] [test] Fix an apparent typo in SemaCXX/consteval-return-void.cpp.

Reviewed as part of D119094.

Added: 


Modified: 
clang/test/SemaCXX/consteval-return-void.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/consteval-return-void.cpp 
b/clang/test/SemaCXX/consteval-return-void.cpp
index da1fd8b7cdf79..b21e8e821dc6e 100644
--- a/clang/test/SemaCXX/consteval-return-void.cpp
+++ b/clang/test/SemaCXX/consteval-return-void.cpp
@@ -10,8 +10,8 @@ template <> consteval int FunT2() { return; } // 
expected-error {{non-void
 enum E {};
 
 constexpr E operator+(E,E) { return; } // expected-error {{non-void constexpr 
function 'operator+' should return a value}}
-consteval E operator+(E,E) { return; }  // expected-error {{non-void consteval 
function 'operator+' should return a value}}
-template  constexpr E operator-(E,E) { return; } // expected-error 
{{non-void constexpr function 'operator-' should return a value}}
+consteval E operator-(E,E) { return; }  // expected-error {{non-void consteval 
function 'operator-' should return a value}}
+template  constexpr E operator+(E,E) { return; } // expected-error 
{{non-void constexpr function 'operator+' should return a value}}
 template  consteval E operator-(E,E) { return; } // expected-error 
{{non-void consteval function 'operator-' should return a value}}
 
 template  constexpr E operator*(E,E);



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

[clang] 3c8d2aa - [clang] Don't emit redundant warnings for 'return; '

[Arthur O'Dwyer via cfe-commits]

Author: Arthur O'Dwyer
Date: 2022-02-14T11:28:32-05:00
New Revision: 3c8d2aa87c1701ca16e13f06aea484637e03d005

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

LOG: [clang] Don't emit redundant warnings for 'return;'

when the function declaration's return type is already invalid for
some reason. This is relevant to 
https://github.com/llvm/llvm-project/issues/49188
because another way that the declaration's return type could become
invalid is that it might be `C auto` where `C` is false.

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

Added: 
clang/test/SemaCXX/deduced-return-void.cpp

Modified: 
clang/lib/Sema/SemaStmt.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 746eb82a5bdc7..6ccef38c2f61d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -4098,7 +4098,9 @@ StmtResult Sema::BuildReturnStmt(SourceLocation 
ReturnLoc, Expr *RetValExp,
   } else if (!RetValExp && !HasDependentReturnType) {
 FunctionDecl *FD = getCurFunctionDecl();
 
-if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
+if ((FD && FD->isInvalidDecl()) || FnRetType->containsErrors()) {
+  // The intended return type might have been "void", so don't warn.
+} else if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
   // C++11 [stmt.return]p2
   Diag(ReturnLoc, diag::err_constexpr_return_missing_expr)
   << FD << FD->isConsteval();

diff  --git a/clang/test/SemaCXX/deduced-return-void.cpp 
b/clang/test/SemaCXX/deduced-return-void.cpp
new file mode 100644
index 0..7b6c514eab71b
--- /dev/null
+++ b/clang/test/SemaCXX/deduced-return-void.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// Check that we don't get any extra warning for "return" without an
+// expression, in a function that might have been intended to return
+// void all along.
+auto f1() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in 
earlier return statement}}
+}
+
+decltype(auto) f2() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in 
earlier return statement}}
+}
+
+auto *g() {
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted 
return expression}}
+}
+
+decltype(h1) h1() { // expected-error {{use of undeclared identifier 'h1'}}
+  return;
+}



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

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

[Arthur O'Dwyer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c8d2aa87c17: [clang] Don't emit redundant warnings for 
'return;' (authored by arthur.j.odwyer).

Changed prior to commit:
  https://reviews.llvm.org/D119094?vs=406838&id=408434#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119094

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/deduced-return-void.cpp


Index: clang/test/SemaCXX/deduced-return-void.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deduced-return-void.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// Check that we don't get any extra warning for "return" without an
+// expression, in a function that might have been intended to return
+// void all along.
+auto f1() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in 
earlier return statement}}
+}
+
+decltype(auto) f2() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in 
earlier return statement}}
+}
+
+auto *g() {
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted 
return expression}}
+}
+
+decltype(h1) h1() { // expected-error {{use of undeclared identifier 'h1'}}
+  return;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4098,7 +4098,9 @@
   } else if (!RetValExp && !HasDependentReturnType) {
 FunctionDecl *FD = getCurFunctionDecl();
 
-if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
+if ((FD && FD->isInvalidDecl()) || FnRetType->containsErrors()) {
+  // The intended return type might have been "void", so don't warn.
+} else if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
   // C++11 [stmt.return]p2
   Diag(ReturnLoc, diag::err_constexpr_return_missing_expr)
   << FD << FD->isConsteval();


Index: clang/test/SemaCXX/deduced-return-void.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deduced-return-void.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// Check that we don't get any extra warning for "return" without an
+// expression, in a function that might have been intended to return
+// void all along.
+auto f1() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in earlier return statement}}
+}
+
+decltype(auto) f2() {
+  return 1;
+  return; // expected-error {{deduced as 'void' here but deduced as 'int' in earlier return statement}}
+}
+
+auto *g() {
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted return expression}}
+}
+
+decltype(h1) h1() { // expected-error {{use of undeclared identifier 'h1'}}
+  return;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4098,7 +4098,9 @@
   } else if (!RetValExp && !HasDependentReturnType) {
 FunctionDecl *FD = getCurFunctionDecl();
 
-if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
+if ((FD && FD->isInvalidDecl()) || FnRetType->containsErrors()) {
+  // The intended return type might have been "void", so don't warn.
+} else if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
   // C++11 [stmt.return]p2
   Diag(ReturnLoc, diag::err_constexpr_return_missing_expr)
   << FD << FD->isConsteval();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:6437
+  NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+return;

Quuxplusone wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > Quuxplusone wrote:
> > > > erichkeane wrote:
> > > > > Do we really want this?  I guess I would think doing:
> > > > > 
> > > > > 
> > > > > ```
> > > > > void MyFunc(auto whatever) {
> > > > >   auto X = move(whatever);
> > > > > ```
> > > > > 
> > > > > when I MEAN std::move, just for it to pick up a non-std::move the 1st 
> > > > > time is likely the same problem?  Or should it get a separate warning?
> > > > That's a good point (IMHO). Perhaps instead of making this a specific 
> > > > case of "warn for unqualified call to things in `std` (namely `move` 
> > > > and `forward`)," we should make it a specific case of "warn for any 
> > > > unqualified use of //this identifier// (namely `move` and `forward`)." 
> > > > That's closer in spirit to Nico Josuttis's comment that `move` is 
> > > > almost like a keyword in modern C++, and therefore shouldn't be thrown 
> > > > around willy-nilly. Either you mean `std::move` (in which case qualify 
> > > > it), or you mean some other `my::move` (in which case qualify it), but 
> > > > using the bare word `move` is inappropriate in modern C++ no matter 
> > > > whether it //currently// finds something out of `std` or not.
> > > > I'm ambivalent between these two ways of looking at the issue. Maybe 
> > > > someone can think up a reason to prefer one or the other?
> > > > 
> > > > libc++'s tests do include several recently-added instances of `move` as 
> > > > a //variable name//, e.g. `auto copy(x); auto move(std::move(x));`. 
> > > > This confuses grep but would not confuse Clang, for better and worse. I 
> > > > don't expect that real code would ever do this, either.
> > > > 
> > > > @erichkeane's specific example is a //template//, which means it's 
> > > > going to be picked up by D72282 `clang-tidy bugprone-unintended-adl` 
> > > > also. Using ADL inside templates triggers multiple red flags 
> > > > simultaneously. Whereas this D119670 is the only thing that's going to 
> > > > catch unqualified `move` in //non-template// code.
> > > > That's a good point (IMHO). Perhaps instead of making this a specific 
> > > > case of "warn for unqualified call to things in `std` (namely `move` 
> > > > and `forward`)," we should make it a specific case of "warn for any 
> > > > unqualified use of //this identifier// (namely `move` and `forward`)." 
> > > > That's closer in spirit to Nico Josuttis's comment that `move` is 
> > > > almost like a keyword in modern C++, and therefore shouldn't be thrown 
> > > > around willy-nilly. Either you mean `std::move` (in which case qualify 
> > > > it), or you mean some other `my::move` (in which case qualify it), but 
> > > > using the bare word `move` is inappropriate in modern C++ no matter 
> > > > whether it //currently// finds something out of `std` or not.
> > > 
> > > Ah! I guess that was just my interpretation of how this patch worked: 
> > > Point out troublesome 'keyword-like-library-functions' used unqualified.  
> > > I think the alternative (warn for unqualified call to things in std) is 
> > > so incredibly noisy as to be worthless, particularly in light of 'using' 
> > > statements.
> > > 
> > > > @erichkeane's specific example is a //template//, which means it's 
> > > > going to be picked up by D72282 `clang-tidy bugprone-unintended-adl` 
> > > > also. Using ADL inside templates triggers multiple red flags 
> > > > simultaneously. Whereas this D119670 is the only thing that's going to 
> > > > catch unqualified `move` in //non-template// code.
> > > 
> > > This was a template for convenience sake (so y'all couldn't "well 
> > > actually" me on the type I chose), but good to know we have a warning 
> > > like that!
> > > 
> > > What I was TRYING to point out a case where the person is using `move` or 
> > > `forward` intending to have the `std` version, but accidentially ending 
> > > up with thier own version.
> > It is *likely* the same problem. The problem is the "likely" does a lot of 
> > heavy lifting here.
> > I think there is general agreement that std::move should be called 
> > qualified, not that `move` is somehow a special identifier that users 
> > should not use. These are almost contradictory statements - aka if we 
> > wanted to discourage people to name their function move, we wouldn't also 
> > need or want to force them to qualify their call.
> > 
> > It is very possible that `std::move` cannot be used at all in the TU 
> > because it is simply not declared, in which case the code would be 
> > perfectly fine.
> > A slightly more involved approach would be to try to detect whether 
> > std::move is declared by doing a lookup and warn in that case, but I'm not 
> > sure that brings much.
> > I certainly disagree tha

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/Headers/stdnoreturn.h:19
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+

Is this actually useful? Isn't it sufficient to have the include-time warning 
for this header?



Comment at: clang/lib/Headers/stdnoreturn.h:22
+/* Including the header file in C2x is also deprecated. */
+#warning "the '' header is deprecated in C2x"
+#endif

Would be nice to mention the solution, as well. E.g.

"The '' header has been deprecated in C2x: including it no 
longer necessary in order to use 'noreturn'. "



Comment at: clang/test/Sema/c2x-noreturn.c:41-43
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+   // c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}

If you've written [[noreturn]] in your code, you're doing the right thing 
already. Why bother emitting a warning? The problem that should be corrected is 
at the point of inclusion of stdnoreturn.h, not the spelling here.

I'd suggest only warning about "[[_Noreturn]]" if the user actually //wrote// 
it like that, and not when it's expanded from the "noreturn" macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

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

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



Comment at: clang/docs/ClangFormatStyleOptions.rst:4007
 
+  * ``bool AfterRequiresKeywordInRequiresClause`` If ``true``, put space 
between requires keyword in a requires clause and
+opening parentheses, if there is one.

Quuxplusone wrote:
> curdeius wrote:
> > Wouldn't `AfterRequiresInClause`/`AfterRequiresInExpression` be meaningful 
> > enough?
> +1, I have no problem with `AfterRequiresInClause` and 
> `AfterRequiresInExpression`!
I will change the names accordingly before the commit.

They are much better.


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

https://reviews.llvm.org/D113369

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

[PATCH] D119716: [clang][lex] NFC: De-duplicate some #include_next logic

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added inline comments.



Comment at: clang/include/clang/Lex/Preprocessor.h:2207
+   const FileEntry *&LookupFromFile) const;
+
   /// Install the standard preprocessor pragmas:

I there a reason why this uses an out parameter instead of a returning a 
`std::pair` or something similar? If yes, I think it would be good to document 
which parameters are out parameters. `IncludeNextTok` looks suspiciously like 
an out-parameter as well but is not AFAICT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119716

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

In D114543#3319347 , @durin42 wrote:

> As far as I can tell this patch broke the Rust compiler, but from the commit 
> message it sounds like it shouldn't have?
>
> https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8358#e85ad6f3-9992-4ea1-9cd3-d8db9f45f33e
>  fails with
>
>   Attribute 'uwtable' should have an Argument
>   i8* (i64, i64)* @__rust_alloc
>   in function __rust_alloc
>   LLVM ERROR: Broken function found, compilation aborted!
>
> Any thoughts?

Yeah, that's puzzling. The attribute has an optional argument (or else we had 
to update ~3080 occurrences  of "uwtable" in tests), so reading it is
a bit tricky:  
https://github.com/llvm/llvm-project/blob/19b4e9d76ecc9a5343c093bc54d965734b996518/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L1631
That message is output here 
https://github.com/llvm/llvm-project/blob/19b4e9d76ecc9a5343c093bc54d965734b996518/llvm/lib/IR/Verifier.cpp#L1710
and I can trigger this line with

  $ cat x.ll
  define void @f() uwtable {
ret void
  }
  $ ./bin/opt -S --passes=verify x.ll
  ; ModuleID = 'x.ll'
  source_filename = "x.ll"
  
  ; Function Attrs: uwtable
  define void @f() #0 {
ret void
  }
  
  attributes #0 = { uwtable }
  $ ./bin/opt  x.ll -o x.bc
  $ ./bin/opt --verify  x.bc -S
  ; ModuleID = 'x.bc'
  source_filename = "x.ll"
  
  ; Function Attrs: uwtable
  define void @f() #0 {
ret void
  }
  
  attributes #0 = { uwtable }
  $ 

Could there be a mismatch between two `llvm-project` versions, somehow?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Augie Fackler via Phabricator via cfe-commits]

durin42 added a comment.

In D114543#3319576 , @chill wrote:

> In D114543#3319347 , @durin42 wrote:
>
>> As far as I can tell this patch broke the Rust compiler, but from the commit 
>> message it sounds like it shouldn't have?
>>
>> https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8358#e85ad6f3-9992-4ea1-9cd3-d8db9f45f33e
>>  fails with
>>
>>   Attribute 'uwtable' should have an Argument
>>   i8* (i64, i64)* @__rust_alloc
>>   in function __rust_alloc
>>   LLVM ERROR: Broken function found, compilation aborted!
>>
>> Any thoughts?
>
> Yeah, that's puzzling. The attribute has an optional argument (or else we had 
> to update ~3080 occurrences  of "uwtable" in tests), so reading it is
> a bit tricky:  
> https://github.com/llvm/llvm-project/blob/19b4e9d76ecc9a5343c093bc54d965734b996518/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L1631
> That message is output here 
> https://github.com/llvm/llvm-project/blob/19b4e9d76ecc9a5343c093bc54d965734b996518/llvm/lib/IR/Verifier.cpp#L1710
> and I can trigger this line with
>
>   $ cat x.ll
>   define void @f() uwtable {
>   ret void
>   }
>   $ ./bin/opt -S --passes=verify x.ll
>   ; ModuleID = 'x.ll'
>   source_filename = "x.ll"
>   
>   ; Function Attrs: uwtable
>   define void @f() #0 {
> ret void
>   }
>   
>   attributes #0 = { uwtable }
>   $ ./bin/opt  x.ll -o x.bc
>   $ ./bin/opt --verify  x.bc -S
>   ; ModuleID = 'x.bc'
>   source_filename = "x.ll"
>   
>   ; Function Attrs: uwtable
>   define void @f() #0 {
> ret void
>   }
>   
>   attributes #0 = { uwtable }
>   $ 
>
> Could there be a mismatch between two `llvm-project` versions, somehow?

I don't believe so: we build LLVM from HEAD and build Rust directly against 
LLVM. Is the parameter optional if uwtable is set programmatically, or only 
when we're reading IR streams?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D119574: [clang] Expose -fprofile-use in clang-cl

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Sorry, that bot is green now as of 437d4e01fe4c057509dff30efd560049ad07bc99 
, so it 
looks like it was just bad timing that led me to suspect your commit, since it 
touched the test file.

I'm not sure why the bots test output is so limited, they usually point to the 
failing assert or provide the output from lit. But either way, sorry for the 
trouble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119574

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

[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:710
+  CacheLookup.HitIt = HitIt;
+  noteLookupUsage(&*HitIt - &*search_dir_begin(), Loc);
 }

I haven’t looked into this in total details but `&*` looks a little awkward to 
me. Dereference a pointer/iteration and then get its pointer value back? 

Wouldn’t this hit the same issue that we saw before if `serach_dir_begin` is 
allocated in a different bump allocator begin than `HitIt`?

If possible, would `std::distance` communicate the intent more clearly?



Comment at: clang/lib/Lex/HeaderSearch.cpp:982
 
+  ConstSearchDirIterator NextIt = [](auto ItCopy) { return ++ItCopy; }(It);
+

What’s the reason that this can’t be? The current lambda-based implementation 
looks a little over-complicated to me. But maybe I’m missing something.
```
ConstSearchDirIterator NextIt = ItCopy;
++NextIt;
```

or even something equivalent to 
```
ConstSearchDirIterator NextIt = std::next(ItCopy);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119721

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

[clang] 00cd6c0 - [Preprocessor] Reduce the memory overhead of `#define` directives (Recommit)

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2022-02-14T09:27:44-08:00
New Revision: 00cd6c04202acf71f74c670b2dd4343929d1f45f

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

LOG: [Preprocessor] Reduce the memory overhead of `#define` directives 
(Recommit)

Recently we observed high memory pressure caused by clang during some parallel 
builds.
We discovered that we have several projects that have a large number of #define 
directives
in their TUs (on the order of millions), which caused huge memory consumption 
in clang due
to a lot of allocations for MacroInfo. We would like to reduce the memory 
overhead of
clang for a single #define to reduce the memory overhead for these files, to 
allow us to
reduce the memory pressure on the system during highly parallel builds. This 
change achieves
that by removing the SmallVector in MacroInfo and instead storing the tokens in 
an array
allocated using the bump pointer allocator, after all tokens are lexed.

The added unit test with 100 #define directives illustrates the problem. 
Prior to this
change, on arm64 macOS, clang's PP bump pointer allocator allocated 272007616 
bytes, and
used roughly 272 bytes per #define. After this change, clang's PP bump pointer 
allocator
allocates 120002016 bytes, and uses only roughly 120 bytes per #define.

For an example test file that we have internally with 7.8 million #define 
directives, this
change produces the following improvement on arm64 macOS: Persistent allocation 
footprint for
this test case file as it's being compiled to LLVM IR went down 22% from 5.28 
GB to 4.07 GB
and the total allocations went down 14% from 8.26 GB to 7.05 GB. Furthermore, 
this change
reduced the total number of allocations made by the system for this clang 
invocation from
1454853 to 133663, an order of magnitude improvement.

The recommit fixes the LLDB build failure.

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

Added: 
clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Modified: 
clang/include/clang/Lex/MacroInfo.h
clang/lib/Lex/MacroInfo.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/unittests/Lex/CMakeLists.txt
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/MacroInfo.h 
b/clang/include/clang/Lex/MacroInfo.h
index 0347a7a37186b..1947bc8fc509e 100644
--- a/clang/include/clang/Lex/MacroInfo.h
+++ b/clang/include/clang/Lex/MacroInfo.h
@@ -54,11 +54,14 @@ class MacroInfo {
   /// macro, this includes the \c __VA_ARGS__ identifier on the list.
   IdentifierInfo **ParameterList = nullptr;
 
+  /// This is the list of tokens that the macro is defined to.
+  const Token *ReplacementTokens = nullptr;
+
   /// \see ParameterList
   unsigned NumParameters = 0;
 
-  /// This is the list of tokens that the macro is defined to.
-  SmallVector ReplacementTokens;
+  /// \see ReplacementTokens
+  unsigned NumReplacementTokens = 0;
 
   /// Length in characters of the macro definition.
   mutable unsigned DefinitionLength;
@@ -230,26 +233,47 @@ class MacroInfo {
   bool isWarnIfUnused() const { return IsWarnIfUnused; }
 
   /// Return the number of tokens that this macro expands to.
-  unsigned getNumTokens() const { return ReplacementTokens.size(); }
+  unsigned getNumTokens() const { return NumReplacementTokens; }
 
   const Token &getReplacementToken(unsigned Tok) const {
-assert(Tok < ReplacementTokens.size() && "Invalid token #");
+assert(Tok < NumReplacementTokens && "Invalid token #");
 return ReplacementTokens[Tok];
   }
 
-  using tokens_iterator = SmallVectorImpl::const_iterator;
+  using const_tokens_iterator = const Token *;
 
-  tokens_iterator tokens_begin() const { return ReplacementTokens.begin(); }
-  tokens_iterator tokens_end() const { return ReplacementTokens.end(); }
-  bool tokens_empty() const { return ReplacementTokens.empty(); }
-  ArrayRef tokens() const { return ReplacementTokens; }
+  const_tokens_iterator tokens_begin() const { return ReplacementTokens; }
+  const_tokens_iterator tokens_end() const {
+return ReplacementTokens + NumReplacementTokens;
+  }
+  bool tokens_empty() const { return NumReplacementTokens == 0; }
+  ArrayRef tokens() const {
+return llvm::makeArrayRef(ReplacementTokens, NumReplacementTokens);
+  }
 
-  /// Add the specified token to the replacement text for the macro.
-  void AddTokenToBody(const Token &Tok) {
+  llvm::MutableArrayRef
+  allocateTokens(unsigned NumTokens, llvm::BumpPtrAllocator &PPAllocator) {
+assert(ReplacementTokens == nullptr && NumReplacementTokens == 0 &&
+   "Token list already allocated!");
+NumReplacementTokens = NumTokens;
+Token *NewReplacementT

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Headers/stdnoreturn.h:19
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+

jyknight wrote:
> Is this actually useful? Isn't it sufficient to have the include-time warning 
> for this header?
I think it gives a more precise diagnostic than the header inclusion one, and 
it will diagnose closer to where the use happens instead of at the include. 
Might not be the most useful, but it I think there's some utility.



Comment at: clang/lib/Headers/stdnoreturn.h:22
+/* Including the header file in C2x is also deprecated. */
+#warning "the '' header is deprecated in C2x"
+#endif

jyknight wrote:
> Would be nice to mention the solution, as well. E.g.
> 
> "The '' header has been deprecated in C2x: including it no 
> longer necessary in order to use 'noreturn'. "
I can add that, but it is necessary in order to use 'noreturn' as a function 
specifier, so that wording isn't quite right. e.g., `noreturn void func(void);` 
is valid C today if you `#include `



Comment at: clang/test/Sema/c2x-noreturn.c:41-43
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+   // c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}

jyknight wrote:
> If you've written [[noreturn]] in your code, you're doing the right thing 
> already. Why bother emitting a warning? The problem that should be corrected 
> is at the point of inclusion of stdnoreturn.h, not the spelling here.
> 
> I'd suggest only warning about "[[_Noreturn]]" if the user actually //wrote// 
> it like that, and not when it's expanded from the "noreturn" macro.
> I'd suggest only warning about "[[_Noreturn]]" if the user actually wrote it 
> like that, and not when it's expanded from the "noreturn" macro.

I'm not keen on that design. The goal of this diagnostic is to let people know 
that they have a surprise in their code (that this macro is expanding to 
something they may not expect) at the location the surprise happens. Consider:

```
// TU.c
#include 
#include "my_header.h"

// my_header.h
[[noreturn]] void func(void);
```
my_header.h doesn't see the inclusion of stdnoreturn.h, so finding out about 
the macro may inform the developer to be more protective of using the attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

In D114543#3319587 , @durin42 wrote:

> 



> Is the parameter optional if uwtable is set programmatically, or only when 
> we're reading IR streams?

No, it's not optional, the attribute is added by 
https://github.com/llvm/llvm-project/blob/00cd6c04202acf71f74c670b2dd4343929d1f45f/llvm/include/llvm/IR/Function.h#L636
(although seting it to `None` is semantically as not setting it at all).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D119722: [clang][lex] Use `SearchDirIterator` types in for loops

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

Nice. I think this reads a lot better than the index-based implementation.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1450
 Optional HeaderSearch::searchDirIdx(const DirectoryLookup &DL) const 
{
-  for (unsigned I = 0; I < SearchDirs.size(); ++I)
-if (&SearchDirs[I] == &DL)
-  return I;
-  return None;
+  return &DL - &*SearchDirs.begin();
 }

Could we change this function to return an `unsigned` instead of 
`Optional` now?

Also, is `&DL - &*SearchDirs.begin()` safe and doesn’t trigger the issues we 
saw previously if start and end are allocated in different memory regions, 
because I don’t know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119722

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Augie Fackler via Phabricator via cfe-commits]

durin42 added a comment.

In D114543#3319638 , @chill wrote:

> In D114543#3319587 , @durin42 wrote:
>
>> 
>
>
>
>> Is the parameter optional if uwtable is set programmatically, or only when 
>> we're reading IR streams?
>
> No, it's not optional, the attribute is added by 
> https://github.com/llvm/llvm-project/blob/00cd6c04202acf71f74c670b2dd4343929d1f45f/llvm/include/llvm/IR/Function.h#L636
> (although seting it to `None` is semantically as not setting it at all).

Okay, that makes sense then. I think I was close to getting to that point by 
looking at this patch, but you saved me some time. Thanks!

/me off to write matching patch for Rust


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D119509: [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

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

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

Thanks.


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

https://reviews.llvm.org/D119509

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

[PATCH] D119708: [clang][lex] Remove `PPCallbacks::FileNotFound()`

[Alex Hoppen via Phabricator via cfe-commits]

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

Assuming that this is indeed never used, removing dead code always sounds good 
to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119708

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