[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Owen Pan via Phabricator via cfe-commits]

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

LGTM except for the nits.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1528
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if ((Style.Language == FormatStyle::LK_Java) &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {

Remove redundant parens.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1535
+addUnwrappedLine();
+} else if (Style.BraceWrapping.AfterFunction)
   addUnwrappedLine();

Add braces to "keep it uniform" with the `if` block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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

[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5475
 
+  if (IndexVT.getVectorElementType() == MVT::i64 && XLenVT == MVT::i32) {
+report_fatal_error("The V extension does not support EEW=64 for index "

craig.topper wrote:
> Can we truncate the index to nvxXi32 instead of erroring? Would that allow us 
> to preserve more test cases?
Done, thanks.



Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll:1033
 
-define <8 x i64> @mgather_baseidx_sext_v8i8_v8i64(i64* %base, <8 x i8> %idxs, 
<8 x i1> %m, <8 x i64> %passthru) {
-; RV32-LABEL: mgather_baseidx_sext_v8i8_v8i64:

craig.topper wrote:
> Can these test cases be preserved in an rv64 only test?
Done, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106518

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

[PATCH] D116795: [clang-format] Use range-for loops. NFC.

[Marek Kurdej via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116795

Files:
  clang/lib/Format/AffectedRangeManager.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/TokenAnalyzer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp

Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -343,11 +343,9 @@
 pushToken(FormatTok);
 addUnwrappedLine();
 
-for (SmallVectorImpl::iterator I = Lines.begin(),
-  E = Lines.end();
- I != E; ++I) {
-  Callback.consumeUnwrappedLine(*I);
-}
+for (const UnwrappedLine &Line : Lines)
+  Callback.consumeUnwrappedLine(Line);
+
 Callback.finishRun();
 Lines.clear();
 while (!PPLevelBranchIndex.empty() &&
@@ -3269,10 +3267,7 @@
 
 void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
   bool JustComments = Line->Tokens.empty();
-  for (SmallVectorImpl::const_iterator
-   I = CommentsBeforeNextToken.begin(),
-   E = CommentsBeforeNextToken.end();
-   I != E; ++I) {
+  for (FormatToken *Tok : CommentsBeforeNextToken) {
 // Line comments that belong to the same line comment section are put on the
 // same line since later we might want to reflow content between them.
 // Additional fine-grained breaking of line comment sections is controlled
@@ -3281,11 +3276,11 @@
 //
 // FIXME: Consider putting separate line comment sections as children to the
 // unwrapped line instead.
-(*I)->ContinuesLineCommentSection =
-continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
-if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
+Tok->ContinuesLineCommentSection =
+continuesLineCommentSection(*Tok, *Line, CommentPragmasRegex);
+if (isOnNewLine(*Tok) && JustComments && !Tok->ContinuesLineCommentSection)
   addUnwrappedLine();
-pushToken(*I);
+pushToken(Tok);
   }
   if (NewlineBeforeNext && JustComments)
 addUnwrappedLine();
Index: clang/lib/Format/TokenAnalyzer.cpp
===
--- clang/lib/Format/TokenAnalyzer.cpp
+++ clang/lib/Format/TokenAnalyzer.cpp
@@ -127,11 +127,8 @@
 
 LLVM_DEBUG({
   llvm::dbgs() << "Replacements for run " << Run << ":\n";
-  for (tooling::Replacements::const_iterator I = RunResult.first.begin(),
- E = RunResult.first.end();
-   I != E; ++I) {
-llvm::dbgs() << I->toString() << "\n";
-  }
+  for (const tooling::Replacement &Replacement : RunResult.first)
+llvm::dbgs() << Replacement.toString() << "\n";
 });
 for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
   delete AnnotatedLines[i];
Index: clang/lib/Format/FormatToken.cpp
===
--- clang/lib/Format/FormatToken.cpp
+++ clang/lib/Format/FormatToken.cpp
@@ -296,14 +296,11 @@
 const CommaSeparatedList::ColumnFormat *
 CommaSeparatedList::getColumnFormat(unsigned RemainingCharacters) const {
   const ColumnFormat *BestFormat = nullptr;
-  for (SmallVector::const_reverse_iterator
-   I = Formats.rbegin(),
-   E = Formats.rend();
-   I != E; ++I) {
-if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
-  if (BestFormat && I->LineCount > BestFormat->LineCount)
+  for (const ColumnFormat &Format : llvm::reverse(Formats)) {
+if (Format.TotalWidth <= RemainingCharacters || Format.Columns == 1) {
+  if (BestFormat && Format.LineCount > BestFormat->LineCount)
 break;
-  BestFormat = &*I;
+  BestFormat = &Format;
 }
   }
   return BestFormat;
Index: clang/lib/Format/AffectedRangeManager.cpp
===
--- clang/lib/Format/AffectedRangeManager.cpp
+++ clang/lib/Format/AffectedRangeManager.cpp
@@ -59,11 +59,9 @@
 
 bool AffectedRangeManager::affectsCharSourceRange(
 const CharSourceRange &Range) {
-  for (SmallVectorImpl::const_iterator I = Ranges.begin(),
-E = Ranges.end();
-   I != E; ++I) {
-if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) &&
-!SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin()))
+  for (const CharSourceRange &R : Ranges) {
+if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), R.getBegin()) &&
+!SourceMgr.isBeforeInTranslationUnit(R.getEnd(), Range.getBegin()))
   return true;
   }
   return false;

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D116318#3212351 , @curdeius wrote:

> I'm in the same position as @hazardyknusperkeks.
> If you need something to simplify the code you can add a helper 
> `getPreviousTokenOrNull` or something like that in your patch.
> But we certainly don't want to pay for the `if` check each time we call 
> `getPreviousToken`.

I think we are going overboard here. Based on the comments, the intent was to 
return null for the previous token of the first token. I was the first and only 
one who needed to call it, and if the caller must check if it's the first token 
every time anyway, why not just do it in the callee and make the function more 
robust. Perhaps we can rename the current one to something like 
`getPreviousTokenFast`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116318

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

[PATCH] D116795: [clang-format] Use range-for loops. NFC.

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM so much easier to read!! Thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116795

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

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

I agree performance of this if is unlikely to be a game changer performance wise


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116318

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

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for the reviews!




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1528
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if ((Style.Language == FormatStyle::LK_Java) &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {

owenpan wrote:
> Remove redundant parens.
I don't think they're redundant because of 2 reasons: nested if and comment 
inside.
Cf. 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1535
+addUnwrappedLine();
+} else if (Style.BraceWrapping.AfterFunction)
   addUnwrappedLine();

owenpan wrote:
> Add braces to "keep it uniform" with the `if` block.
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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

[PATCH] D116795: [clang-format] Use range-for loops. NFC.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/AffectedRangeManager.cpp:66
   return true;
   }
   return false;

Remove braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116795

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

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1528
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if ((Style.Language == FormatStyle::LK_Java) &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {

curdeius wrote:
> owenpan wrote:
> > Remove redundant parens.
> I don't think they're redundant because of 2 reasons: nested if and comment 
> inside.
> Cf. 
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.
By "parens" I meant `(` and `)`. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1528
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if ((Style.Language == FormatStyle::LK_Java) &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {

owenpan wrote:
> curdeius wrote:
> > owenpan wrote:
> > > Remove redundant parens.
> > I don't think they're redundant because of 2 reasons: nested if and comment 
> > inside.
> > Cf. 
> > https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.
> By "parens" I meant `(` and `)`. :)
Sorry, my bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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

[clang] 01f355f - [clang-format] Use range-for loops. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-07T10:01:09+01:00
New Revision: 01f355fe95f6b45db8bb239239b7ed978e094a13

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

LOG: [clang-format] Use range-for loops. NFC.

Reviewed By: MyDeveloperDay, owenpan

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

Added: 


Modified: 
clang/lib/Format/AffectedRangeManager.cpp
clang/lib/Format/FormatToken.cpp
clang/lib/Format/TokenAnalyzer.cpp
clang/lib/Format/UnwrappedLineParser.cpp

Removed: 




diff  --git a/clang/lib/Format/AffectedRangeManager.cpp 
b/clang/lib/Format/AffectedRangeManager.cpp
index 7ad1f7070d0a..3b735c4e6859 100644
--- a/clang/lib/Format/AffectedRangeManager.cpp
+++ b/clang/lib/Format/AffectedRangeManager.cpp
@@ -59,13 +59,10 @@ bool AffectedRangeManager::computeAffectedLines(
 
 bool AffectedRangeManager::affectsCharSourceRange(
 const CharSourceRange &Range) {
-  for (SmallVectorImpl::const_iterator I = Ranges.begin(),
-E = Ranges.end();
-   I != E; ++I) {
-if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) &&
-!SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin()))
+  for (const CharSourceRange &R : Ranges)
+if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), R.getBegin()) &&
+!SourceMgr.isBeforeInTranslationUnit(R.getEnd(), Range.getBegin()))
   return true;
-  }
   return false;
 }
 

diff  --git a/clang/lib/Format/FormatToken.cpp 
b/clang/lib/Format/FormatToken.cpp
index 57f8a5a45cbb..def5663d0449 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -296,14 +296,11 @@ void CommaSeparatedList::precomputeFormattingInfos(const 
FormatToken *Token) {
 const CommaSeparatedList::ColumnFormat *
 CommaSeparatedList::getColumnFormat(unsigned RemainingCharacters) const {
   const ColumnFormat *BestFormat = nullptr;
-  for (SmallVector::const_reverse_iterator
-   I = Formats.rbegin(),
-   E = Formats.rend();
-   I != E; ++I) {
-if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
-  if (BestFormat && I->LineCount > BestFormat->LineCount)
+  for (const ColumnFormat &Format : llvm::reverse(Formats)) {
+if (Format.TotalWidth <= RemainingCharacters || Format.Columns == 1) {
+  if (BestFormat && Format.LineCount > BestFormat->LineCount)
 break;
-  BestFormat = &*I;
+  BestFormat = &Format;
 }
   }
   return BestFormat;

diff  --git a/clang/lib/Format/TokenAnalyzer.cpp 
b/clang/lib/Format/TokenAnalyzer.cpp
index d83e837ca134..3f7722093175 100644
--- a/clang/lib/Format/TokenAnalyzer.cpp
+++ b/clang/lib/Format/TokenAnalyzer.cpp
@@ -127,11 +127,8 @@ std::pair 
TokenAnalyzer::process() {
 
 LLVM_DEBUG({
   llvm::dbgs() << "Replacements for run " << Run << ":\n";
-  for (tooling::Replacements::const_iterator I = RunResult.first.begin(),
- E = RunResult.first.end();
-   I != E; ++I) {
-llvm::dbgs() << I->toString() << "\n";
-  }
+  for (const tooling::Replacement &Replacement : RunResult.first)
+llvm::dbgs() << Replacement.toString() << "\n";
 });
 for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
   delete AnnotatedLines[i];

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 17187b7996aa..410b0557c4cd 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -343,11 +343,9 @@ void UnwrappedLineParser::parse() {
 pushToken(FormatTok);
 addUnwrappedLine();
 
-for (SmallVectorImpl::iterator I = Lines.begin(),
-  E = Lines.end();
- I != E; ++I) {
-  Callback.consumeUnwrappedLine(*I);
-}
+for (const UnwrappedLine &Line : Lines)
+  Callback.consumeUnwrappedLine(Line);
+
 Callback.finishRun();
 Lines.clear();
 while (!PPLevelBranchIndex.empty() &&
@@ -3269,10 +3267,7 @@ continuesLineCommentSection(const FormatToken &FormatTok,
 
 void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
   bool JustComments = Line->Tokens.empty();
-  for (SmallVectorImpl::const_iterator
-   I = CommentsBeforeNextToken.begin(),
-   E = CommentsBeforeNextToken.end();
-   I != E; ++I) {
+  for (FormatToken *Tok : CommentsBeforeNextToken) {
 // Line comments that belong to the same line comment section are put on 
the
 // same line since later we might want to reflow content between them.
 // Additional fine-grained breaking of line comment sections is controlled
@@ -3281,11 +3276,11 @@ void UnwrappedLineParser::flushComments(boo

[PATCH] D116795: [clang-format] Use range-for loops. NFC.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG01f355fe95f6: [clang-format] Use range-for loops. NFC. 
(authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D116795?vs=398060&id=398071#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116795

Files:
  clang/lib/Format/AffectedRangeManager.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/TokenAnalyzer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp

Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -343,11 +343,9 @@
 pushToken(FormatTok);
 addUnwrappedLine();
 
-for (SmallVectorImpl::iterator I = Lines.begin(),
-  E = Lines.end();
- I != E; ++I) {
-  Callback.consumeUnwrappedLine(*I);
-}
+for (const UnwrappedLine &Line : Lines)
+  Callback.consumeUnwrappedLine(Line);
+
 Callback.finishRun();
 Lines.clear();
 while (!PPLevelBranchIndex.empty() &&
@@ -3269,10 +3267,7 @@
 
 void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
   bool JustComments = Line->Tokens.empty();
-  for (SmallVectorImpl::const_iterator
-   I = CommentsBeforeNextToken.begin(),
-   E = CommentsBeforeNextToken.end();
-   I != E; ++I) {
+  for (FormatToken *Tok : CommentsBeforeNextToken) {
 // Line comments that belong to the same line comment section are put on the
 // same line since later we might want to reflow content between them.
 // Additional fine-grained breaking of line comment sections is controlled
@@ -3281,11 +3276,11 @@
 //
 // FIXME: Consider putting separate line comment sections as children to the
 // unwrapped line instead.
-(*I)->ContinuesLineCommentSection =
-continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
-if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
+Tok->ContinuesLineCommentSection =
+continuesLineCommentSection(*Tok, *Line, CommentPragmasRegex);
+if (isOnNewLine(*Tok) && JustComments && !Tok->ContinuesLineCommentSection)
   addUnwrappedLine();
-pushToken(*I);
+pushToken(Tok);
   }
   if (NewlineBeforeNext && JustComments)
 addUnwrappedLine();
Index: clang/lib/Format/TokenAnalyzer.cpp
===
--- clang/lib/Format/TokenAnalyzer.cpp
+++ clang/lib/Format/TokenAnalyzer.cpp
@@ -127,11 +127,8 @@
 
 LLVM_DEBUG({
   llvm::dbgs() << "Replacements for run " << Run << ":\n";
-  for (tooling::Replacements::const_iterator I = RunResult.first.begin(),
- E = RunResult.first.end();
-   I != E; ++I) {
-llvm::dbgs() << I->toString() << "\n";
-  }
+  for (const tooling::Replacement &Replacement : RunResult.first)
+llvm::dbgs() << Replacement.toString() << "\n";
 });
 for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
   delete AnnotatedLines[i];
Index: clang/lib/Format/FormatToken.cpp
===
--- clang/lib/Format/FormatToken.cpp
+++ clang/lib/Format/FormatToken.cpp
@@ -296,14 +296,11 @@
 const CommaSeparatedList::ColumnFormat *
 CommaSeparatedList::getColumnFormat(unsigned RemainingCharacters) const {
   const ColumnFormat *BestFormat = nullptr;
-  for (SmallVector::const_reverse_iterator
-   I = Formats.rbegin(),
-   E = Formats.rend();
-   I != E; ++I) {
-if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
-  if (BestFormat && I->LineCount > BestFormat->LineCount)
+  for (const ColumnFormat &Format : llvm::reverse(Formats)) {
+if (Format.TotalWidth <= RemainingCharacters || Format.Columns == 1) {
+  if (BestFormat && Format.LineCount > BestFormat->LineCount)
 break;
-  BestFormat = &*I;
+  BestFormat = &Format;
 }
   }
   return BestFormat;
Index: clang/lib/Format/AffectedRangeManager.cpp
===
--- clang/lib/Format/AffectedRangeManager.cpp
+++ clang/lib/Format/AffectedRangeManager.cpp
@@ -59,13 +59,10 @@
 
 bool AffectedRangeManager::affectsCharSourceRange(
 const CharSourceRange &Range) {
-  for (SmallVectorImpl::const_iterator I = Ranges.begin(),
-E = Ranges.end();
-   I != E; ++I) {
-if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) &&
-!SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin()))
+  for (const CharSourceRange &R : Ranges)
+if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), R.getBegin()) &&
+!SourceMgr.isBeforeInTranslationUnit

[clang] 91b9e67 - [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-07T10:06:49+01:00
New Revision: 91b9e6729c11cce8cf5fea727c6cb81ab8ab5ba4

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

LOG: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized 
blocks in Java.

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

Before this change, BraceWrapping: AfterFunction would affect synchronized 
blocks in Java, but they should be formatted w.r.t. BraceWrapping: 
AfterControlStatement.

Using the config:
```
BreakBeforeBraces: Custom
BraceWrapping:
  AfterControlStatement: false
  AfterFunction: true
```

would result in misformatted code like:
```
class Foo {
  void bar()
  {
synchronized (this)
{
  a();
  a();
}
  }
}
```

instead of:
```
class Foo {
  void bar()
  {
synchronized (this) {
  a();
  a();
}
  }
}
```

Reviewed By: MyDeveloperDay, owenpan

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 410b0557c4cd..5895c2efdef4 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1523,8 +1523,16 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
 // structural element.
 // FIXME: Figure out cases where this is not true, and add projections
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if (Style.Language == FormatStyle::LK_Java &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {
+  // If necessary, we could set the type to something 
diff erent than
+  // TT_FunctionLBrace.
+  if (Style.BraceWrapping.AfterControlStatement ==
+  FormatStyle::BWACS_Always)
+addUnwrappedLine();
+} else if (Style.BraceWrapping.AfterFunction) {
   addUnwrappedLine();
+}
 FormatTok->setType(TT_FunctionLBrace);
 parseBlock();
 addUnwrappedLine();

diff  --git a/clang/unittests/Format/FormatTestJava.cpp 
b/clang/unittests/Format/FormatTestJava.cpp
index 9a18c2853abc..84f6420b9e1a 100644
--- a/clang/unittests/Format/FormatTestJava.cpp
+++ b/clang/unittests/Format/FormatTestJava.cpp
@@ -431,6 +431,24 @@ TEST_F(FormatTestJava, SynchronizedKeyword) {
   verifyFormat("synchronized (mData) {\n"
"  // ...\n"
"}");
+
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Java);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  Style.BraceWrapping.AfterFunction = false;
+  verifyFormat("synchronized (mData)\n"
+   "{\n"
+   "  // ...\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("synchronized (mData) {\n"
+   "  // ...\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTestJava, AssertKeyword) {



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

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG91b9e6729c11: [clang-format] Fix `BraceWrapping: 
AfterFunction` affecting synchronized blocks… (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D116767?vs=397987&id=398074#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116767

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


Index: clang/unittests/Format/FormatTestJava.cpp
===
--- clang/unittests/Format/FormatTestJava.cpp
+++ clang/unittests/Format/FormatTestJava.cpp
@@ -431,6 +431,24 @@
   verifyFormat("synchronized (mData) {\n"
"  // ...\n"
"}");
+
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Java);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  Style.BraceWrapping.AfterFunction = false;
+  verifyFormat("synchronized (mData)\n"
+   "{\n"
+   "  // ...\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("synchronized (mData) {\n"
+   "  // ...\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTestJava, AssertKeyword) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1523,8 +1523,16 @@
 // structural element.
 // FIXME: Figure out cases where this is not true, and add projections
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if (Style.Language == FormatStyle::LK_Java &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {
+  // If necessary, we could set the type to something different than
+  // TT_FunctionLBrace.
+  if (Style.BraceWrapping.AfterControlStatement ==
+  FormatStyle::BWACS_Always)
+addUnwrappedLine();
+} else if (Style.BraceWrapping.AfterFunction) {
   addUnwrappedLine();
+}
 FormatTok->setType(TT_FunctionLBrace);
 parseBlock();
 addUnwrappedLine();


Index: clang/unittests/Format/FormatTestJava.cpp
===
--- clang/unittests/Format/FormatTestJava.cpp
+++ clang/unittests/Format/FormatTestJava.cpp
@@ -431,6 +431,24 @@
   verifyFormat("synchronized (mData) {\n"
"  // ...\n"
"}");
+
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Java);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  Style.BraceWrapping.AfterFunction = false;
+  verifyFormat("synchronized (mData)\n"
+   "{\n"
+   "  // ...\n"
+   "}",
+   Style);
+
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("synchronized (mData) {\n"
+   "  // ...\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTestJava, AssertKeyword) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1523,8 +1523,16 @@
 // structural element.
 // FIXME: Figure out cases where this is not true, and add projections
 // for them (the one we know is missing are lambdas).
-if (Style.BraceWrapping.AfterFunction)
+if (Style.Language == FormatStyle::LK_Java &&
+Line->Tokens.front().Tok->is(Keywords.kw_synchronized)) {
+  // If necessary, we could set the type to something different than
+  // TT_FunctionLBrace.
+  if (Style.BraceWrapping.AfterControlStatement ==
+  FormatStyle::BWACS_Always)
+addUnwrappedLine();
+} else if (Style.BraceWrapping.AfterFunction) {
   addUnwrappedLine();
+}
 FormatTok->setType(TT_FunctionLBrace);
 parseBlock();
 addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

[liushuai wang via Phabricator via cfe-commits]

MTC added a comment.

This patch has become very complicated now. I summarized this patch and give a 
figure to illustrate what we have reached. And @Sockke please add some comments 
to explain the complex part or other means to make this patch more readable.

F21496057: D107450.svg 


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

https://reviews.llvm.org/D107450

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:435-454
+bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
+  const size_t size = Lines.size();
+  if (size > 0 && Lines[size - 1].InPPDirective)
+return true;
+#if 1
+  const unsigned Position = Tokens->getPosition();
+  if (Position == 0)

curdeius wrote:
> Please remove unused code.
Will do pending D116318.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:485
+if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 ||
+IsPrecededByCommentOrPPDirective || // StartsWithBrace ||
+precededByCommentOrPPDirective()) {

curdeius wrote:
> Please remove unused code.
Will do.



Comment at: clang/unittests/Format/FormatTest.cpp:23224
+
+  EXPECT_EQ("if (a)\n"
+"  if (b)\n"

curdeius wrote:
> owenpan wrote:
> > owenpan wrote:
> > > MyDeveloperDay wrote:
> > > > any reason these can't be verifyFormats?
> > > Did you mean to add the expected part as a separate case? I don't think 
> > > it would add any value if there are no braces to remove in the first 
> > > place?
> > > Did you mean to add the expected part as a separate case? I don't think 
> > > it would add any value if there are no braces to remove in the first 
> > > place?
> > 
> > Nvm. I think you wanted something like `verifyFormat(PostformatCode, 
> > PreformatCode, Style)`? Yes, I could do that, but I would have to relax the 
> > restriction to calling `BracesRemover()` in Format.cpp, i.e. checking 
> > `isCpp()` instead.
> :+1: you should be able to use the 2-argument version of `verifyFormat`, no?
Will replace all `EXPECT_EQ`s with `verifyFormat`s. Question: when is the 
former preferred to the latter? It seems `EXPECT_EQ` is frequently used in 
FormatTest.cpp. 


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

https://reviews.llvm.org/D116316

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

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 398077.
Sockke marked an inline comment as done.
Sockke added a comment.

Add some checks for `null` and comments for codes.


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

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
@@ -246,3 +246,97 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&v);
+void showInt(int v1, int &&v2);
+void showPointer(const char *&&s);
+void showPointer2(const char *const &&s);
+void showTriviallyCopyable(TriviallyCopyable &&obj);
+void showTriviallyCopyablePointer(const TriviallyCopyable *&&obj);
+void testFunctions() {
+  int a = 10;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-10]]:20: note: consider changing the 1st parameter of 'showInt' from 'int &&' to 'const int &'
+  showInt(int());
+  showInt(a, std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-13]]:28: note: consider changing the 2nd parameter of 'showInt' from 'int &&' to 'const int &'
+  const char* s = "";
+  showPointer(std::move(s));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-16]]:32: note: consider changing the 1st parameter of 'showPointer' from 'const char *&&' to 'const char *'
+  showPointer2(std::move(s)); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-18]]:39: note: consider changing the 1st parameter of 'showPointer2' from 'const char *const &&' to 'const char *const'
+  TriviallyCopyable *obj = new TriviallyCopyable();
+  showTriviallyCopyable(std::move(*obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: std::move of the expression of the trivially-copyable type 'TriviallyCopyable' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-21]]:48: note: consider changing the 1st parameter of 'showTriviallyCopyable' from 'TriviallyCopyable &&' to 'const TriviallyCopyable &'
+  showTriviallyCopyablePointer(std::move(obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-23]]:62: note: consider changing the 1st parameter of 'showTriviallyCopyablePointer' from 'const TriviallyCopyable *&&' to 'const TriviallyCopyable *'
+}
+template 
+void forwardToShowInt(T && t) {
+  showInt(static_cast(t));
+}
+void testTemplate() {
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+}
+
+struct Tmp {
+  Tmp();
+  Tmp(int &&a);
+  Tmp(int v1, int &&a);
+  Tmp(const char *&&s);
+  Tmp(TriviallyCopyable&& obj);
+  Tmp(const TriviallyCopyable *&&obj);
+  void showTmp(TriviallyCopyable&& t);
+  static void showTmpStatic(TriviallyCopyable&& t);
+};
+void testMethods() {
+  Tmp t;
+  int a = 10;
+  Tmp t1(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-13]]:13: note: consider changing the 1st parameter of 'Tmp' from 'int &&' to 'const int &'
+  Tmp t2(a, std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-15]]:21: note: consider changing the 2nd parameter of 'Tmp' from 'int &&' to 'const int &'
+  const char* s = "";
+  Tmp t3(std::move(s));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-18]]:21: note: consider changing the 1st parameter of 'Tmp' from 'const char *&&' to 'const char *'
+  TriviallyCopyable *obj = new TriviallyCopyable(

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke added inline comments.



Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:161
+  // Generate notes for an invocation with an rvalue reference parameter.
+  const auto *ReceivingCallExpr = dyn_cast(ReceivingExpr);
+  const auto *ReceivingConstructExpr =

`ReceivingExpr` is not null if `IsRVRefParam` is true.



Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:165
+  // Skipping the invocation which is a template instantiation.
+  if ((!ReceivingCallExpr || !ReceivingCallExpr->getDirectCallee() ||
+   ReceivingCallExpr->getDirectCallee()->isTemplateInstantiation()) &&

Add a check for `getDirectCallee()`.



Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:175
+  ReceivingCallExpr
+  ? ReceivingCallExpr->getDirectCallee()->getUnderlyingDecl()
+  : ReceivingConstructExpr->getConstructor()->getUnderlyingDecl();

I have added a check for `getDirectCallee` before here.


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

https://reviews.llvm.org/D107450

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

[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thank you for digging into this!

(BTW the parsing part of this is probably adjacent to a fix for 
https://github.com/clangd/clangd/issues/121, AutoType also doesn't have enough 
locations for `decltype(auto)`. But definitely not something to touch in this 
patch)




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1012
+// token doesn't have it.
+DS.setTypeofParensRange(SourceRange(SourceLocation(), EndLoc));
 ConsumeAnnotationToken();

hokein wrote:
> This is unfortunate for a common case `decltype(expression) a;`, the 
> `ParseDecltypeSpecifier` is called twice to parse the decltype specifier: 
> 
> - first time, we parse the raw decltype token, and we annotate that (the `DS` 
> was created locally, and then was thrown away)
> - second time, we parsed the annotated decltype, and set another `DS`, at 
> this point, we lost the LParen information :(
> 
> One way to fix that is to add a new field `SourceLocation` in `Token` class 
> (there is 4-bytes padding size left, so it won't increase its size), but I'm 
> a little worried, it seems like an overkill to just fix this issue. 
> 
> 
What do you think about only storing the kw loc + endloc then?
This is enough to compute the correct sourcerange, which was the original bug 
we hit.

Our options are:
 - do the work to always compute it: nice to have but it does seem 
hard/intrusive
 - don't store/expose it
 - expose it but have it be unreliable

I don't know if 3 is better than 2 (without a use case it's hard to judge), 
maybe it just encourages writing buggy code. And if nothing else, 2 is smaller 
:-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793

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

[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D115942#3226101 , @craig.topper 
wrote:

> Does this have the same autoupgrade issues as @efriedma raised in 
> https://reviews.llvm.org/D86310

No. The differences are that i128 can be generated by front end while f80 
cannot without D115441  in the given 
datalayout (i.e., MSVC).
I also did some experiments, e.g., generating a bc file (without f80 type) with 
existing compiler and compile it with the one with this patch. There's no error 
during compilation. So I think we don't bother to do autoupgrade at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115942

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

[clang] c033f0d - [Clang][Sema] Avoid crashing for va_arg expressions with bool argument

[Jan Svoboda via cfe-commits]

Author: Egor Zhdan
Date: 2022-01-07T10:41:45+01:00
New Revision: c033f0d9b1c7816b0488a939475d48a100e4dcab

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

LOG: [Clang][Sema] Avoid crashing for va_arg expressions with bool argument

This change fixes a compiler crash that was introduced in 
https://reviews.llvm.org/D103611: `Sema::BuildVAArgExpr` attempted to retrieve 
a corresponding signed type for `bool` by calling 
`ASTContext::getCorrespondingSignedType`.

rdar://86580370

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/varargs.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d454e4877bcef..a2a1f76df96d2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15946,7 +15946,7 @@ ExprResult Sema::BuildVAArgExpr(SourceLocation 
BuiltinLoc,
   // promoted type and the underlying type are the same except for
   // signedness. Ask the AST for the correctly corresponding type and see
   // if that's compatible.
-  if (!PromoteType.isNull() &&
+  if (!PromoteType.isNull() && !UnderlyingType->isBooleanType() &&
   PromoteType->isUnsignedIntegerType() !=
   UnderlyingType->isUnsignedIntegerType()) {
 UnderlyingType =

diff  --git a/clang/test/SemaCXX/varargs.cpp b/clang/test/SemaCXX/varargs.cpp
index 7bec2f1e63030..bc2fe89a6ff8d 100644
--- a/clang/test/SemaCXX/varargs.cpp
+++ b/clang/test/SemaCXX/varargs.cpp
@@ -53,6 +53,8 @@ void promotable(int a, ...) {
 
   // Ensure that signed vs unsigned doesn't matter either.
   (void)__builtin_va_arg(ap, unsigned int);
+
+  (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 
'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior 
because arguments will be promoted to 'int'}}
 }
 
 #if __cplusplus >= 201103L



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

[PATCH] D116272: [Clang][Sema] Avoid crashing for va_arg expressions with bool argument

[Jan Svoboda 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 rGc033f0d9b1c7: [Clang][Sema] Avoid crashing for va_arg 
expressions with bool argument (authored by egorzhdan, committed by 
jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116272

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/varargs.cpp


Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -53,6 +53,8 @@
 
   // Ensure that signed vs unsigned doesn't matter either.
   (void)__builtin_va_arg(ap, unsigned int);
+
+  (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 
'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior 
because arguments will be promoted to 'int'}}
 }
 
 #if __cplusplus >= 201103L
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15946,7 +15946,7 @@
   // promoted type and the underlying type are the same except for
   // signedness. Ask the AST for the correctly corresponding type and see
   // if that's compatible.
-  if (!PromoteType.isNull() &&
+  if (!PromoteType.isNull() && !UnderlyingType->isBooleanType() &&
   PromoteType->isUnsignedIntegerType() !=
   UnderlyingType->isUnsignedIntegerType()) {
 UnderlyingType =


Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -53,6 +53,8 @@
 
   // Ensure that signed vs unsigned doesn't matter either.
   (void)__builtin_va_arg(ap, unsigned int);
+
+  (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
 }
 
 #if __cplusplus >= 201103L
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15946,7 +15946,7 @@
   // promoted type and the underlying type are the same except for
   // signedness. Ask the AST for the correctly corresponding type and see
   // if that's compatible.
-  if (!PromoteType.isNull() &&
+  if (!PromoteType.isNull() && !UnderlyingType->isBooleanType() &&
   PromoteType->isUnsignedIntegerType() !=
   UnderlyingType->isUnsignedIntegerType()) {
 UnderlyingType =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

In D116378#3226780 , @carlosgalvezp 
wrote:

> By the way, the similar problem exists in Clang compiler. I have written in 
> cfe-dev , 
> Discourse 
> 
>  and submitted an issue  
> without any feedback so far (I understand it's been vacation). Do you think 
> it would be beneficial to implement a similar fix there? I don't know if the 
> fix is as straightforward as in clang-tidy or if it will have to be done on a 
> per-check basis.

Speaking as just a user of the compiler and not a contributor (I tend to stick 
in Clang-Tidy land), yes, a fix would be great. Especially if you're getting 
unactionable warnings coming from STM32 libraries - it's a platform I have to 
develop for too. Not sure where in Clang you would make the change. At a 
cursory glance, the commits in `Clang/` I saw to do with stopping system header 
warnings from bubbling up were done in the individual warnings, not in common 
code. Best to check with someone who works in that area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116378

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> applies just as well to the proposal of force-wrapping at >= 2 imports

Absolutely.. but it justifies that this option has got to the point that its no 
longer one thing or the other based on our personal subjective opinions... now 
we need to break down what we need and make this a better feature

otherwise we just flip-flop between different groups of users calling it a 
regression. The ideal would be that whatever options we add, the default would 
give us exactly as we have today, almost warts and all. but the options would 
allow us to give all the possibilities we think people might want, or leave us 
room to expand into those options (moving to an enum normally helps that)

i.e.

  JavaScriptWrapImports: Never (false)
  JavaScriptWrapImports: Multiple (true)  // meaning more than 1
  JavaScriptWrapImports: Always 

Therefore wouldn't the current behaviour for  ColumnLimit: 0 be 
JavaScriptWrapImports: Always?  (or Never if you want them all on one line)


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

https://reviews.llvm.org/D116638

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

[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D115942#3226146 , @rnk wrote:

> Yeah, let's try to reach some resolution on that.

The things are different. We don't support f80 type on Windows 32 bits 
previously. It means we don't have the burden to upgrade, since there's no 
global/load/store/GEP/etc for f80.

> In the mean time, I discovered the `alignstack` parameter attribute:
> https://llvm.org/docs/LangRef.html#parameter-attributes
>
> Could that be used to solve this problem in the frontend instead?

It might be feasible, but I don't think it's a good idea. It looks to me more 
like a language specific alignment hint instead of medium that carrying target 
specific information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115942

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

[clang] 2cd2600 - Unaligned Access Warning Added

[Mubashar Ahmad via cfe-commits]

Author: Mubashar Ahmad
Date: 2022-01-07T09:54:20Z
New Revision: 2cd2600abaf3c604889b77ab408fdf89d7a21c48

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

LOG: Unaligned Access Warning Added

Added warning for potential cases of
unaligned access when option
-mno-unaligned-access has been specified

Added: 
clang/test/Sema/test-wunaligned-access.c
clang/test/Sema/test-wunaligned-access.cpp

Modified: 
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.h
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index d788c85179142..d870a9f06592a 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -590,4 +590,9 @@ def warn_padded_struct_size : Warning<
   InGroup, DefaultIgnore;
 def warn_unnecessary_packed : Warning<
   "packed attribute is unnecessary for %0">, InGroup, DefaultIgnore;
+
+// -Wunaligned-access
+def warn_unaligned_access : Warning<
+  "field %1 within its parent %0 has an alignment greater than its parent "
+  "this may be caused by %0 being packed and can lead to unaligned accesses">, 
InGroup, DefaultIgnore;
 }

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index c0642efaee4eb..bc4bb17b271d8 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -542,6 +542,7 @@ def ExplicitInitializeCall : 
DiagGroup<"explicit-initialize-call">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
+def UnalignedAccess : DiagGroup<"unaligned-access">;
 
 def PessimizingMove : DiagGroup<"pessimizing-move">;
 def ReturnStdMove : DiagGroup<"return-std-move">;

diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e39ec1c718d1..8d2fe4660a3cd 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2021,6 +2021,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   CharUnits UnpackedFieldAlign =
   !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
   CharUnits UnpackedFieldOffset = FieldOffset;
+  CharUnits OriginalFieldAlign = UnpackedFieldAlign;
 
   if (FieldPacked) {
 FieldAlign = CharUnits::One();
@@ -2105,6 +2106,22 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   // Remember max struct/class ABI-specified alignment.
   UnadjustedAlignment = std::max(UnadjustedAlignment, FieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign, PreferredAlign);
+
+  // For checking the alignment of inner fields against
+  // the alignment of its parent record.
+  if (const RecordDecl *RD = D->getParent()) {
+// Check if packed attribute or pragma pack is present.
+if (RD->hasAttr() || !MaxFieldAlignment.isZero())
+  if (FieldAlign < OriginalFieldAlign)
+if (D->getType()->isRecordType()) {
+  // If the offset is a multiple of the alignment of
+  // the type, raise the warning.
+  // TODO: Takes no account the alignment of the outer struct
+  if (FieldOffset % OriginalFieldAlign != 0)
+Diag(D->getLocation(), diag::warn_unaligned_access)
+<< Context.getTypeDeclType(RD) << D->getName();
+}
+  }
 }
 
 void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {

diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 9ffb5d73b2aad..fe62384eeb98d 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -221,6 +221,7 @@ getAArch64MicroArchFeaturesFromMcpu(const Driver &D, 
StringRef Mcpu,
 void aarch64::getAArch64TargetFeatures(const Driver &D,
const llvm::Triple &Triple,
const ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs,
std::vector &Features,
bool ForAS) {
   Arg *A;
@@ -464,10 +465,14 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
 
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access)) {
-if (A->getOption().matches(options::OPT

[PATCH] D116439: [clang-tidy] Fix `readability-const-return-type` for pure virtual function.

[liushuai wang via Phabricator via cfe-commits]

MTC added subscribers: flx, MTC.
MTC added a comment.

FYI: I'm from Bytedance Inc , @Sockke, and I are 
fixing the AutoFix bugs recently, most of them will lead to the compilation 
error.  For this bug, fixing the override virtual methods but missing the pure 
virtual base method, which will cause the compilation error. These annoying 
bugs will hinder the large-scale deployment of clang-tidy AutoFix in the 
production environment.




Comment at: 
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp:101
+  functionDecl(returns(isConstQualified()),
+   anyOf(isDefinition(), cxxMethodDecl(isPure(
+  .bind("func"),

Like @flx comments in https://reviews.llvm.org/D116593, the better choice is 
that we suppress the fix for the virtual method. What do you think @Sockke?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116439

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

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz updated this revision to Diff 398087.
salman-javed-nz added a comment.

Comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116085

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/NoLintPragmaHandler.cpp
  clang-tools-extra/clang-tidy/NoLintPragmaHandler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/2nd-translation-unit.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -58,30 +58,24 @@
 // NOLINTEND(some-other-check)
 // NOLINTEND(google-explicit-constructor)
 
-// NOLINTBEGIN(google-explicit-constructor)
-// NOLINTBEGIN(some-other-check)
-class C7 { C7(int i); };
-// NOLINTEND(google-explicit-constructor)
-// NOLINTEND(some-other-check)
-
 // NOLINTBEGIN(google-explicit-constructor)
 // NOLINTBEGIN
-class C8 { C8(int i); };
+class C7 { C7(int i); };
 // NOLINTEND
 // NOLINTEND(google-explicit-constructor)
 
 // NOLINTBEGIN
 // NOLINTBEGIN(google-explicit-constructor)
-class C9 { C9(int i); };
+class C8 { C8(int i); };
 // NOLINTEND(google-explicit-constructor)
 // NOLINTEND
 
 // NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
-class C10 { C10(int i); };
+class C9 { C9(int i); };
 // NOLINTEND(not-closed-bracket-is-treated-as-skip-all
 
 // NOLINTBEGIN without-brackets-skip-all, another-check
-class C11 { C11(int i); };
+class C10 { C10(int i); };
 // NOLINTEND without-brackets-skip-all, another-check
 
 #define MACRO(X) class X { X(int i); };
@@ -114,28 +108,28 @@
 MACRO_NO_LINT_INSIDE_MACRO
 
 // NOLINTBEGIN(google*)
-class C12 { C12(int i); };
+class C11 { C11(int i); };
 // NOLINTEND(google*)
 
 // NOLINTBEGIN(*explicit-constructor)
-class C15 { C15(int i); };
+class C12 { C12(int i); };
 // NOLINTEND(*explicit-constructor)
 
 // NOLINTBEGIN(*explicit*)
-class C16 { C16(int i); };
+class C13 { C13(int i); };
 // NOLINTEND(*explicit*)
 
 // NOLINTBEGIN(-explicit-constructor)
-class C17 { C17(int x); };
+class C14 { C14(int x); };
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit
 // NOLINTEND(-explicit-constructor)
 
 // NOLINTBEGIN(google*,-google*)
-class C18 { C18(int x); };
+class C15 { C15(int x); };
 // NOLINTEND(google*,-google*)
 
 // NOLINTBEGIN(*,-google*)
-class C19 { C19(int x); };
+class C16 { C16(int x); };
 // NOLINTEND(*,-google*)
 
 int array1[10];
@@ -154,4 +148,4 @@
 int array4[10];
 // NOLINTEND(*-avoid-c-arrays)
 
-// CHECK-MESSAGES: Suppressed 27 warnings (27 NOLINT).
+// CHECK-MESSAGES: Suppressed 26 warnings (26 NOLINT).
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
@@ -11,3 +11,6 @@

[clang] 359b4e6 - [clang-format] Use prefix increment and decrement. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-07T11:19:53+01:00
New Revision: 359b4e6cdb7ae60c51e33ce71443843b71cb643d

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

LOG: [clang-format] Use prefix increment and decrement. NFC.

Added: 


Modified: 
clang/lib/Format/DefinitionBlockSeparator.cpp
clang/lib/Format/Format.cpp
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/lib/Format/SortJavaScriptImports.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/WhitespaceManager.cpp

Removed: 




diff  --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index ba51594f3f69..7748ad4a3bfe 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -102,7 +102,7 @@ void DefinitionBlockSeparator::separateBlocks(
 TargetToken = TargetToken->Next;
   if (!TargetToken) {
 while (I < Lines.size() && !Lines[I]->First->is(tok::r_brace))
-  I++;
+  ++I;
   }
 } else if (CurrentLine->First->closesScope()) {
   if (OpeningLineIndex > Lines.size())

diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d1bc378766cd..ffb1b14b76b6 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2214,7 +2214,7 @@ class Cleaner : public TokenAnalyzer {
   unsigned St = Idx, End = Idx;
   while ((End + 1) < Tokens.size() &&
  Tokens[End]->Next == Tokens[End + 1]) {
-End++;
+++End;
   }
   auto SR = CharSourceRange::getCharRange(Tokens[St]->Tok.getLocation(),
   Tokens[End]->Tok.getEndLoc());
@@ -2450,7 +2450,7 @@ std::string replaceCRLF(const std::string &Code) {
   do {
 Pos = Code.find("\r\n", LastPos);
 if (Pos == LastPos) {
-  LastPos++;
+  ++LastPos;
   continue;
 }
 if (Pos == std::string::npos) {

diff  --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp 
b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index 9c00d243f34a..951a98258051 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -260,7 +260,7 @@ std::pair 
NamespaceEndCommentsFixer::analyze(
   // remove end comment, it will be merged in next one
   updateEndComment(EndCommentPrevTok, std::string(), SourceMgr, 
&Fixes);
 }
-CompactedNamespacesCount++;
+++CompactedNamespacesCount;
 AllNamespaceNames = "::" + NamespaceName + AllNamespaceNames;
 continue;
   }

diff  --git a/clang/lib/Format/SortJavaScriptImports.cpp 
b/clang/lib/Format/SortJavaScriptImports.cpp
index 77dc0d683e5f..52c222a8b3dc 100644
--- a/clang/lib/Format/SortJavaScriptImports.cpp
+++ b/clang/lib/Format/SortJavaScriptImports.cpp
@@ -260,13 +260,13 @@ class JavaScriptImportSorter : public TokenAnalyzer {
   while (Start != References.end() && Start->FormattingOff) {
 // Skip over all imports w/ disabled formatting.
 ReferencesSorted.push_back(*Start);
-Start++;
+++Start;
   }
   SmallVector SortChunk;
   while (Start != References.end() && !Start->FormattingOff) {
 // Skip over all imports w/ disabled formatting.
 SortChunk.push_back(*Start);
-Start++;
+++Start;
   }
   llvm::stable_sort(SortChunk);
   mergeModuleReferences(SortChunk);

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 5241685630a5..22c56537bb9e 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1564,9 +1564,9 @@ class AnnotatingParser {
 int ParenLevel = 0;
 while (Current) {
   if (Current->is(tok::l_paren))
-ParenLevel++;
+++ParenLevel;
   if (Current->is(tok::r_paren))
-ParenLevel--;
+--ParenLevel;
   if (ParenLevel < 1)
 break;
   Current = Current->Next;
@@ -1590,9 +1590,9 @@ class AnnotatingParser {
 break;
 }
 if (TemplateCloser->is(tok::less))
-  NestingLevel++;
+  ++NestingLevel;
 if (TemplateCloser->is(tok::greater))
-  NestingLevel--;
+  --NestingLevel;
 if (NestingLevel < 1)
   break;
 TemplateCloser = TemplateCloser->Next;

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 5895c2efdef4..2bf5cedd2ae4 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2449,7 +2449,7 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) 
{
 addUnwrappedLine();
 if 

[clang] 11c67e5 - [clang][driver] Don't pass -Wunaligned-access to cc1as

[Archibald Elliott via cfe-commits]

Author: Archibald Elliott
Date: 2022-01-07T10:45:26Z
New Revision: 11c67e5a4e99f51ec66c9781710f81955cfd5e24

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

LOG: [clang][driver] Don't pass -Wunaligned-access to cc1as

This is to fix some failing assembler tests.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index fe62384eeb98d..89a77a368ef02 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -467,11 +467,13 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
options::OPT_munaligned_access)) {
 if (A->getOption().matches(options::OPT_mno_unaligned_access)) {
   Features.push_back("+strict-align");
-  CmdArgs.push_back("-Wunaligned-access");
+  if (!ForAS)
+CmdArgs.push_back("-Wunaligned-access");
 }
   } else if (Triple.isOSOpenBSD()) {
 Features.push_back("+strict-align");
-CmdArgs.push_back("-Wunaligned-access");
+if (!ForAS)
+  CmdArgs.push_back("-Wunaligned-access");
   }
 
   if (Args.hasArg(options::OPT_ffixed_x1))

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 69f1a8337517b..1055d7800b63e 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -771,7 +771,8 @@ void arm::getARMTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
   // Kernel code has more strict alignment requirements.
   if (KernelOrKext) {
 Features.push_back("+strict-align");
-CmdArgs.push_back("-Wunaligned-access");
+if (!ForAS)
+  CmdArgs.push_back("-Wunaligned-access");
   } else if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
   options::OPT_munaligned_access)) {
 if (A->getOption().matches(options::OPT_munaligned_access)) {
@@ -784,7 +785,8 @@ void arm::getARMTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
 D.Diag(diag::err_target_unsupported_unaligned) << "v8m.base";
 } else {
   Features.push_back("+strict-align");
-  CmdArgs.push_back("-Wunaligned-access");
+  if (!ForAS)
+CmdArgs.push_back("-Wunaligned-access");
 }
   } else {
 // Assume pre-ARMv6 doesn't support unaligned accesses.
@@ -806,17 +808,20 @@ void arm::getARMTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
   if (VersionNum < 6 ||
   Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m) {
 Features.push_back("+strict-align");
-CmdArgs.push_back("-Wunaligned-access");
+if (!ForAS)
+  CmdArgs.push_back("-Wunaligned-access");
   }
 } else if (Triple.isOSLinux() || Triple.isOSNaCl() ||
Triple.isOSWindows()) {
   if (VersionNum < 7) {
 Features.push_back("+strict-align");
-CmdArgs.push_back("-Wunaligned-access");
+if (!ForAS)
+  CmdArgs.push_back("-Wunaligned-access");
   }
 } else {
   Features.push_back("+strict-align");
-  CmdArgs.push_back("-Wunaligned-access");
+  if (!ForAS)
+CmdArgs.push_back("-Wunaligned-access");
 }
   }
 



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

[PATCH] D116221: [AArch64][ARM][Clang] Unaligned Access Warning Added

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

This caused some driver test failures. They have been fixed in 
https://reviews.llvm.org/rG11c67e5a4e99f51ec66c9781710f81955cfd5e24


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

https://reviews.llvm.org/D116221

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

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

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

HazardyKnusperkeks added a comment.

In D115060#3224176 , @owenpan wrote:

>> But I suspect it is the Assignment of the `PreviousLine` since this is not 
>> existent every time.
>
> Yep!
>
>> So I see the following solutions:
>>
>> 1. Only name `NextLine`, and use `I[-1]`.
>> 2. `const auto HasPreviousLine = I != AnnotatedLines.begin(); const auto 
>> &PreviousLine = HasPreviousLine ? *I[-1] : *I;` which is safe, since 
>> `PreviousLine` is only used if `HasPreviousLine` is true, but is a bit 
>> confusing. It would get an explaining comment.
>> 3. Rearrange the statements so that we can have only one check if there is a 
>> previous line and define `PreviousLine` inside that `if`. It remains to be 
>> seen if that's NFC.
>>
>> I would prefer option 3, but if that would change the behavior would go for 
>> option 2.
>
> There is option 4: change the type of `PreviousLine` to a pointer, and 
> replace `PreviousLine.` with `PreviousLine->`. (You can rename `PreviousLine` 
> if you think the naming is inconsistent with `NextLine`.) This would keep the 
> patch being NFC.

I'm open for that, but I think the current diff (plus corrected formatting) is 
better. And at least according to our tests it is NFC.


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

https://reviews.llvm.org/D115060

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

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Moving the handling of empty record blocks might be NFC? Maybe option 4 for now 
and the refactoring in another patch?




Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:288
+// Try to merge a control statement block with left brace unwrapped.
+if (TheLine->Last->is(tok::l_brace) &&
 TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {

Is `TheLine->First != TheLine->Last` redundant?



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:310
+   : 0;
+  }
+  if (TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,

I don't think you can drop `else` here.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:317
+   : 0;
+  }
+  if (TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&

Ditto.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:335-360
+  // Handle empty record blocks where the brace has already been wrapped.
+  if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last) {
+bool EmptyBlock = NextLine.First->is(tok::r_brace);
+
+const FormatToken *Tok = PreviousLine.First;
+if (Tok && Tok->is(tok::comment))
+  Tok = Tok->getNextNonComment();

Handling empty record blocks here instead of earlier may have unknown side 
effects?


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

https://reviews.llvm.org/D115060

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

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Cool!

I'd suggesting adding a note to llvm/docs/ReleaseNotes.rst at the same time :-)




Comment at: clang/include/clang/Basic/CodeGenOptions.def:142
 
+CODEGENOPT(HotPatch, 1, 0) ///< Supports the Microsoft /HOTPATCH flag and would
+   ///< generate a 'patchable-function' attribute.

nit: I'd s/would generate/generages/



Comment at: clang/include/clang/Driver/Options.td:2493
   MarshallingInfoInt>;
+def fhotpatch : Flag<["-"], "fhotpatch">, Group, Flags<[CC1Option]>,
+  HelpText<"Ensure that all functions can be hotpatched at runtime">,

Is this flag also exposed as a driver flag, or is it cc1 only?

I wonder if we should go for a less generic name here, perhaps -fms-hotpatch? 
For example, the flag above is also about hotpatching, but a different 
mechanism.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:648
   Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
+  Options.Hotpatch = CodeGenOpts.HotPatch;
 

Since it's currently only supported on x86_64, we should probably diagnose 
trying to use it for other target somewhere.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5998
+  Args.AddLastArg(CmdArgs, options::OPT_fhotpatch);
 
   if (TC.SupportsProfiling()) {

Can we also pass /functionpadmin when clang-cl invokes the linker, like cl does?



Comment at: clang/test/Driver/cl-options.c:489
 // RUN: /homeparams \
 // RUN: /hotpatch \
 // RUN: /JMC \

We should drop this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116511

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

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

I suppose the idea is that all `Module` creations should go through 
`makeModule`, right? In that case I think we should either

- make the `Module` constructor private and `ModuleMap` a friend of `Module`
- or at least add a doc comment to the `Module` constructor that says `Module`s 
should only be created using `ModuleMap::makeModule`.

Or are there other places that also create `Module`s but are not supposed to go 
through `ModuleMap::makeModule`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751

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

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

Adjusting the indices seem pretty fragile to me. Any reason why you wanted to 
stick with indices as keys instead of switching to something else like I 
suggested here ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D116221: [AArch64][ARM][Clang] Unaligned Access Warning Added

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the patch!

1. When committing code, please add "Differential Revision: 
https://reviews.llvm.org/D116221"; as last line to your commit description, to 
link the commit to the review. See `git log` for many examples.

2. This breaks tests on Windows, see e.g. 
http://45.33.8.238/win/52066/step_7.txt Please take a look, and revert for now 
if it takes a while to fix.


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

https://reviews.llvm.org/D116221

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

[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 398101.
hokein added a comment.

don't expose LParen loc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang/include/clang/AST/TypeLoc.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/AST/SourceLocationTest.cpp

Index: clang/unittests/AST/SourceLocationTest.cpp
===
--- clang/unittests/AST/SourceLocationTest.cpp
+++ clang/unittests/AST/SourceLocationTest.cpp
@@ -215,6 +215,33 @@
   EXPECT_TRUE(Verifier.match("long a;", typeLoc()));
 }
 
+TEST(TypeLoc, DecltypeTypeLocRange) {
+  llvm::Annotations Code(R"(
+$full1[[decltype(1)]] a;
+struct A {struct B{};} var;
+$full2[[decltype(var)]]::B c;
+  )");
+  auto AST = tooling::buildASTFromCodeWithArgs(Code.code(), /*Args=*/{});
+  ASTContext &Ctx = AST->getASTContext();
+  const auto &SM = Ctx.getSourceManager();
+
+  auto MatchedLocs = clang::ast_matchers::match(
+  typeLoc(loc(decltypeType())).bind("target"), Ctx);
+  ASSERT_EQ(MatchedLocs.size(), 2u);
+  auto verify = [&](SourceRange ActualRange,
+const llvm::Annotations::Range &Expected) {
+auto ActualCharRange =
+Lexer::getAsCharRange(ActualRange, SM, Ctx.getLangOpts());
+EXPECT_EQ(SM.getFileOffset(ActualCharRange.getBegin()), Expected.Begin);
+EXPECT_EQ(SM.getFileOffset(ActualCharRange.getEnd()), Expected.End);
+  };
+  const auto *Target1 = MatchedLocs[0].getNodeAs("target");
+  verify(Target1->getSourceRange(), Code.range("full1"));
+
+  const auto *Target2 = MatchedLocs[1].getNodeAs("target");
+  verify(Target2->getSourceRange(), Code.range("full2"));
+}
+
 TEST(TypeLoc, LongDoubleRange) {
   RangeVerifier Verifier;
   Verifier.expectRange(1, 1, 1, 6);
@@ -559,7 +586,7 @@
 
 TEST(FriendDecl, FriendDecltypeRange) {
   RangeVerifier Verifier;
-  Verifier.expectRange(4, 1, 4, 8);
+  Verifier.expectRange(4, 1, 4, 22);
   EXPECT_TRUE(Verifier.match("struct A;\n"
  "A foo();\n"
  "struct A {\n"
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -427,7 +427,8 @@
 }
 
 void TypeLocWriter::VisitDecltypeTypeLoc(DecltypeTypeLoc TL) {
-  Record.AddSourceLocation(TL.getNameLoc());
+  Record.AddSourceLocation(TL.getDecltypeLoc());
+  Record.AddSourceLocation(TL.getRParenLoc());
 }
 
 void TypeLocWriter::VisitUnaryTransformTypeLoc(UnaryTransformTypeLoc TL) {
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6628,7 +6628,8 @@
 }
 
 void TypeLocReader::VisitDecltypeTypeLoc(DecltypeTypeLoc TL) {
-  TL.setNameLoc(readSourceLocation());
+  TL.setDecltypeLoc(readSourceLocation());
+  TL.setRParenLoc(readSourceLocation());
 }
 
 void TypeLocReader::VisitUnaryTransformTypeLoc(UnaryTransformTypeLoc TL) {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -6228,15 +6228,15 @@
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   E.get() != T->getUnderlyingExpr()) {
-Result = getDerived().RebuildDecltypeType(E.get(), TL.getNameLoc());
+Result = getDerived().RebuildDecltypeType(E.get(), TL.getDecltypeLoc());
 if (Result.isNull())
   return QualType();
   }
   else E.get();
 
   DecltypeTypeLoc NewTL = TLB.push(Result);
-  NewTL.setNameLoc(TL.getNameLoc());
-
+  NewTL.setDecltypeLoc(TL.getDecltypeLoc());
+  NewTL.setRParenLoc(TL.getRParenLoc());
   return Result;
 }
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5973,6 +5973,11 @@
   Sema::GetTypeFromParser(DS.getRepAsType(), &TInfo);
   TL.setUnderlyingTInfo(TInfo);
 }
+void VisitDecltypeTypeLoc(DecltypeTypeLoc TL) {
+  assert(DS.getTypeSpecType() == DeclSpec::TST_decltype);
+  TL.setDecltypeLoc(DS.getTypeSpecTypeLoc());
+  TL.setRParenLoc(DS.getTypeofParensRange().getEnd());
+}
 void VisitUnaryTransformTypeLoc(UnaryTransformTypeLoc TL) {
   // FIXME: This holds only because we only have one unary transform.
   assert(DS.getTypeSpecType() == DeclSpec::TST_underlyingType);
Index: clang/lib/Sema/SemaExprCXX.cpp
=

[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> (BTW the parsing part of this is probably adjacent to a fix for 
> https://github.com/clangd/clangd/issues/121, AutoType also doesn't have 
> enough locations for decltype(auto). But definitely not something to touch in 
> this patch)

Yeah, that is my next step, this is an extending change of `AutoTypeLoc`.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1012
+// token doesn't have it.
+DS.setTypeofParensRange(SourceRange(SourceLocation(), EndLoc));
 ConsumeAnnotationToken();

sammccall wrote:
> hokein wrote:
> > This is unfortunate for a common case `decltype(expression) a;`, the 
> > `ParseDecltypeSpecifier` is called twice to parse the decltype specifier: 
> > 
> > - first time, we parse the raw decltype token, and we annotate that (the 
> > `DS` was created locally, and then was thrown away)
> > - second time, we parsed the annotated decltype, and set another `DS`, at 
> > this point, we lost the LParen information :(
> > 
> > One way to fix that is to add a new field `SourceLocation` in `Token` class 
> > (there is 4-bytes padding size left, so it won't increase its size), but 
> > I'm a little worried, it seems like an overkill to just fix this issue. 
> > 
> > 
> What do you think about only storing the kw loc + endloc then?
> This is enough to compute the correct sourcerange, which was the original bug 
> we hit.
> 
> Our options are:
>  - do the work to always compute it: nice to have but it does seem 
> hard/intrusive
>  - don't store/expose it
>  - expose it but have it be unreliable
> 
> I don't know if 3 is better than 2 (without a use case it's hard to judge), 
> maybe it just encourages writing buggy code. And if nothing else, 2 is 
> smaller :-)
Yeah, having the decltype loc + rparen loc should be enough to fix our bug, and 
it might be a better idea than having a partial working `getLParenLoc`.

> I don't know if 3 is better than 2 (without a use case it's hard to judge), 
> maybe it just encourages writing buggy code. And if nothing else, 2 is 
> smaller :-)

We don't have a concrete use case of the paren range of `decltype`, so 2 is 
probably the best (though having a getRParenLoc API without a paired 
getLParenLoc feels incomplete).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793

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

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D116751#3227142 , @ahoppen wrote:

> I suppose the idea is that all `Module` creations should go through 
> `makeModule`, right? In that case I think we should either
>
> - make the `Module` constructor private and `ModuleMap` a friend of `Module`
> - or at least add a doc comment to the `Module` constructor that says 
> `Module`s should only be created using `ModuleMap::makeModule`.
>
> Or are there other places that also create `Module`s but are not supposed to 
> go through `ModuleMap::makeModule`?

The intent here is for all calls to the `Module` constructor in `ModuleMap` to 
go through `makeModule()`. This will make it possible to ensure a callback is 
always invoked when a `Module` is constructed in D113676 
.

Making the constructor private doesn't aid that goal, so I'd be inclined to do 
that in a separate patch. But would that be the right thing to do?
We don't instantiate `Module` outside of `ModuleMap` in the upstream repo, but 
I don't think there's anything fundamental that would prevent downstream 
projects doing that. Do we care about such use-cases though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751

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

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D116750#3227143 , @ahoppen wrote:

> Adjusting the indices seem pretty fragile to me. Any reason why you wanted to 
> stick with indices as keys instead of switching to something else like I 
> suggested here ?

I've implemented your suggestion locally like so:

  llvm::SpecificBumpPtrAllocator SearchDirsAlloc;
  std::vector SearchDirs;
  llvm::DenseMap SearchDirToHSEntry;
  ...

In the end, I decided to stick with just updating the indices to avoid creating 
unnecessary indirection (and replacing `.` with `->` everywhere).

I don't have any preference though, so if you feel strongly about the 
fragility, I can use addresses to refer to `DirectoryLookup` objects instead of 
indices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D116793#3227175 , @hokein wrote:

>> (BTW the parsing part of this is probably adjacent to a fix for 
>> https://github.com/clangd/clangd/issues/121, AutoType also doesn't have 
>> enough locations for decltype(auto). But definitely not something to touch 
>> in this patch)
>
> Yeah, that is my next step, this is an extending change of `AutoTypeLoc`.

Yay :-)




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1012
+// token doesn't have it.
+DS.setTypeofParensRange(SourceRange(SourceLocation(), EndLoc));
 ConsumeAnnotationToken();

hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > This is unfortunate for a common case `decltype(expression) a;`, the 
> > > `ParseDecltypeSpecifier` is called twice to parse the decltype specifier: 
> > > 
> > > - first time, we parse the raw decltype token, and we annotate that (the 
> > > `DS` was created locally, and then was thrown away)
> > > - second time, we parsed the annotated decltype, and set another `DS`, at 
> > > this point, we lost the LParen information :(
> > > 
> > > One way to fix that is to add a new field `SourceLocation` in `Token` 
> > > class (there is 4-bytes padding size left, so it won't increase its 
> > > size), but I'm a little worried, it seems like an overkill to just fix 
> > > this issue. 
> > > 
> > > 
> > What do you think about only storing the kw loc + endloc then?
> > This is enough to compute the correct sourcerange, which was the original 
> > bug we hit.
> > 
> > Our options are:
> >  - do the work to always compute it: nice to have but it does seem 
> > hard/intrusive
> >  - don't store/expose it
> >  - expose it but have it be unreliable
> > 
> > I don't know if 3 is better than 2 (without a use case it's hard to judge), 
> > maybe it just encourages writing buggy code. And if nothing else, 2 is 
> > smaller :-)
> Yeah, having the decltype loc + rparen loc should be enough to fix our bug, 
> and it might be a better idea than having a partial working `getLParenLoc`.
> 
> > I don't know if 3 is better than 2 (without a use case it's hard to judge), 
> > maybe it just encourages writing buggy code. And if nothing else, 2 is 
> > smaller :-)
> 
> We don't have a concrete use case of the paren range of `decltype`, so 2 is 
> probably the best (though having a getRParenLoc API without a paired 
> getLParenLoc feels incomplete).
Agree. Would it make sense to keep the internal names concrete but expose 
publicly only as begin/endLoc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793

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

[PATCH] D116699: [clangd] Polish clangd/inlayHints and expose them by default.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks this LGTM just a couple nits.
my biggest concern is old endpoint will vanish all of a sudden and it might 
create some confusion, especially for editors that turn on inlayhints without 
an explicit user interaction.
but i suppose plugin maintainers that implement the extension shouldn't have a 
hard time finding out the new endpoint and migrating to it. (and I don't think 
we've got (m)any plugins implementing the extension today.) So all should be 
fine.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:611
 
+  // FIXME: once inlayHints can disabled per-file in config, always advertise.
   if (Opts.InlayHints)

s/can/can be



Comment at: clang-tools-extra/clangd/InlayHints.cpp:168
+  addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+   InlayHintKind::ParameterHint, "", Name, ": ");
 }

nit: parameter name comments for `""` and `": "` (same for other calls)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:320
 
-  void addInlayHint(SourceRange R, InlayHintKind Kind, llvm::StringRef Label) {
+  // We pass HintSide rather than SourceLocation because we want to ensure we
+  // it is in the same file as the common file range.

drop the last `we`



Comment at: clang-tools-extra/clangd/InlayHints.h:26
+/// Compute and return inlay hints for a file.
+///
+/// If RestrictRange is set, return only hints whose location is in that range.

nit: drop the empty line?



Comment at: clang-tools-extra/clangd/Protocol.h:1555
+  ///
+  /// For example, an parameter hint may be positioned before an argument.
+  Position position;

s/an parameter/a parameter/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116699

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

[clang] 3aec4b3 - Revert "Unaligned Access Warning Added"

[Archibald Elliott via cfe-commits]

Author: Archibald Elliott
Date: 2022-01-07T13:07:30Z
New Revision: 3aec4b3d348d69e6c1ead7cba54677b855293983

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

LOG: Revert "Unaligned Access Warning Added"

This reverts commits:
- 2cd2600abaf3c604889b77ab408fdf89d7a21c48
- 11c67e5a4e99f51ec66c9781710f81955cfd5e24

Due to test failures on Windows.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.h
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 
clang/test/Sema/test-wunaligned-access.c
clang/test/Sema/test-wunaligned-access.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index d870a9f06592a..d788c85179142 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -590,9 +590,4 @@ def warn_padded_struct_size : Warning<
   InGroup, DefaultIgnore;
 def warn_unnecessary_packed : Warning<
   "packed attribute is unnecessary for %0">, InGroup, DefaultIgnore;
-
-// -Wunaligned-access
-def warn_unaligned_access : Warning<
-  "field %1 within its parent %0 has an alignment greater than its parent "
-  "this may be caused by %0 being packed and can lead to unaligned accesses">, 
InGroup, DefaultIgnore;
 }

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index bc4bb17b271d8..c0642efaee4eb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -542,7 +542,6 @@ def ExplicitInitializeCall : 
DiagGroup<"explicit-initialize-call">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
-def UnalignedAccess : DiagGroup<"unaligned-access">;
 
 def PessimizingMove : DiagGroup<"pessimizing-move">;
 def ReturnStdMove : DiagGroup<"return-std-move">;

diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8d2fe4660a3cd..3e39ec1c718d1 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2021,7 +2021,6 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   CharUnits UnpackedFieldAlign =
   !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
   CharUnits UnpackedFieldOffset = FieldOffset;
-  CharUnits OriginalFieldAlign = UnpackedFieldAlign;
 
   if (FieldPacked) {
 FieldAlign = CharUnits::One();
@@ -2106,22 +2105,6 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   // Remember max struct/class ABI-specified alignment.
   UnadjustedAlignment = std::max(UnadjustedAlignment, FieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign, PreferredAlign);
-
-  // For checking the alignment of inner fields against
-  // the alignment of its parent record.
-  if (const RecordDecl *RD = D->getParent()) {
-// Check if packed attribute or pragma pack is present.
-if (RD->hasAttr() || !MaxFieldAlignment.isZero())
-  if (FieldAlign < OriginalFieldAlign)
-if (D->getType()->isRecordType()) {
-  // If the offset is a multiple of the alignment of
-  // the type, raise the warning.
-  // TODO: Takes no account the alignment of the outer struct
-  if (FieldOffset % OriginalFieldAlign != 0)
-Diag(D->getLocation(), diag::warn_unaligned_access)
-<< Context.getTypeDeclType(RD) << D->getName();
-}
-  }
 }
 
 void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {

diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 89a77a368ef02..9ffb5d73b2aad 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -221,7 +221,6 @@ getAArch64MicroArchFeaturesFromMcpu(const Driver &D, 
StringRef Mcpu,
 void aarch64::getAArch64TargetFeatures(const Driver &D,
const llvm::Triple &Triple,
const ArgList &Args,
-   llvm::opt::ArgStringList &CmdArgs,
std::vector &Features,
bool ForAS) {
   Arg *A;
@@ -465,16 +464,10 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
 
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D116386#3226174 , 
@LegalizeAdulthood wrote:

> Aaron, I think your comments are useful and I would be inclined to agree with 
> you if I
> was the original author of this check.

Sorry, I hope I didn't give the impression I thought you had done anything 
wrong here, because you definitely haven't. I appreciate all of your efforts 
(and am glad to see you back in the community), and my resigning from the 
review is not indicative of me not wanting to collaborate with you! It's more a 
matter of: I've got ~50 code reviews in my queue as of this morning, on top of 
my own work, and I didn't want you to have to ping this for months on end only 
to never get feedback from me because C++ Core Guideline reviews always stay at 
the bottom of my queue due to the amount of effort involved in most of them.

> I treat the guidelines as just that: guidelines,
> not rules.  In the context of clang-tidy I think you're correct that some 
> guidelines
> are easily turned into usable diagnostics and a subset of those can become 
> enforceable
> rules with suggested fixits.
>
> In this case, the check only issues diagnostics, not fixits.  When the 
> diagnostics result
> in many false positives (as per the open bug), then I think it's reasonable 
> to narrow the
> scope of the check to omit the false positives.

This is not how clang-tidy typically handles coding guideline-specific rules. 
The general rule in clang-tidy is for checks based on coding guidelines to be 
configured to follow the guideline by default (with options to help tune 
things). If of sufficient value, we will add an alias check in `bugprone` (or 
another module that makes sense) and give it different default configuration 
settings than the guideline check, but users expect something that claims to 
check a guideline to actually check that guideline.

(The alternative is: we go back to the guideline authors and ask them to please 
improve their guidance and then we implement whatever the new rule says. But 
the end result is the same -- for checks in a module specific to a published 
set of guidelines, deviations from the spec are considered bugs to be fixed 
, and the guideline text is the final arbiter of what the correct 
behavior is.)

> The worst thing a "guideline" checker can do is to constantly nag you about 
> false
> positives.  This trains people to not run the checkers and/or ignore all 
> their complaints.

We're in strong agreement that guideline checkers with untenable false positive 
rates are a very bad thing. I think this particular check is of insufficient 
quality to have been added in the first place because it's based on a set of 
rules that are not generally enforceable in a way that isn't a constantly 
nagging for reasonable real world code.


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

https://reviews.llvm.org/D116386

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

[PATCH] D116221: [AArch64][ARM][Clang] Unaligned Access Warning Added

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Reverted for the moment in rG3aec4b3d348d69e6c1ead7cba54677b855293983 
 while we 
fix the windows tests.


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

https://reviews.llvm.org/D116221

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

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

I just want to make sure that we don’t accidentally introduce a new 
instantiation of a `Module` that doesn’t go through `makeModule` in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751

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

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Alex Hoppen via Phabricator via cfe-commits]

ahoppen added a comment.

I would prefer to use `std::shared_ptr` instead of `unsigned`. 
IIUC we already basically have a level of indirection because if we want to get 
the `DirectoryLookup`, we need to find it by its index in `SearchDirs`. And 
updating the indices just seems like a hack to me that can be solved more 
cleanly.

Wouldn’t we also need to update the indices in `SearchDirsUsage` and 
`ModuleToSearchDirIdx`? Or replace those by `std::shared_ptr` 
as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D116751#3227243 , @ahoppen wrote:

> I just want to make sure that we don’t accidentally introduce a new 
> instantiation of a `Module` that doesn’t go through `makeModule` in the 
> future.

I understand that, but I'm not sure that's a desirable restriction in all 
situations. Moreover, `Module` resides in the `clangBasic` library while 
`ModuleMap` resides in `clangLex`. Befriending `ModuleMap` or otherwise tying 
`Module` to `ModuleMap` violates the library layering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751

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

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Alex Hoppen via Phabricator via cfe-commits]

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

Ah, in that case it makes sense as-is. I just assumed they lived in the same 
library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751

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

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

The fact that va_list is in the std namespace does leak out into 
__builtin_dump_struct, possibly the odd other place, and of course to AST 
consumers.

I think it'd make sense to keep ASTContext as putting it in the std namespace 
for C++ (like it does for Arm, and used to for AArch64) but also have 
ItaniumMangle override it to the std namespace so that non-C++ gets the right 
mangling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116774

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

[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Sam McCall via Phabricator via cfe-commits]

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

LG thank you!

Keeping/removing the getRParenLoc() accessor is up to you, I see arguments both 
ways.




Comment at: clang/include/clang/AST/TypeLoc.h:2001
+// FIXME: add LParenLoc, it is tricky to support due to the limitation of
+// annotated-decldtype token.
+struct DecltypeTypeLocInfo {

nit: decltype (typo)



Comment at: clang/include/clang/AST/TypeLoc.h:2015
+
+  SourceLocation getRParenLoc() const { return getLocalData()->RParenLoc; }
+  void setRParenLoc(SourceLocation Loc) { getLocalData()->RParenLoc = Loc; }

So concretely I think I'm talking about just removing this getter, since 
getEndLoc() will by default to the end of getLocalSourceRange().

Probably the setter would stay as is?

Anyway, up to you, it's harmless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793

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

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Yeah, in some cases, we'd be just replacing the index-based indirection with 
address-based indirection (e.g. when working with `SearchDirToHSEntry`).

However, most existing `HeaderSearch` algorithms already work with the index 
and use that to get the `DirectoryLookup` from `SearchDirs`. We'd be adding 
another level of indirection for them: first they'd need to get 
`DirectoryLookup *` from `SearchDirs` with the index and then dereference the 
pointer. Maybe that's not very important from performance perspective 
(especially if we bump-ptr-allocate them), but something to be aware of IMO.

The `SearchDirsUsage` member is a bit-vector that's being inserted into the 
same way `SearchDirs` is, so no need to fiddle with indices there. As for 
`ModuleToSearchDirIdx`: yes, we'd need to update indices the same way we do 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 398115.
simoll added a comment.

Rebased onto vector conditionals changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/Sema/ext_vector_casts.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-size-conditional.cpp
  clang/test/SemaOpenCL/ext_vectors.cl

Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++2021
 
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef __attribute__((ext_vector_type(8))) bool BoolVector; // expected-error {{invalid vector element type 'bool'}}
 
 void test_ext_vector_accessors(float4 V) {
   V = V.wzyx;
Index: clang/test/SemaCXX/vector-size-conditional.cpp
===
--- clang/test/SemaCXX/vector-size-conditional.cpp
+++ clang/test/SemaCXX/vector-size-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((ext_vector_type(8)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
   all_dependent(four_ints, four_uints, two_uints); // expected-note {{in instantiation of}}
   all_dependent(four_ints, four_uints, four_uints);
 }
Index: clang/test/SemaCXX/vector-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-bool.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using EightInts = int __attribute__((__vector_size_

[PATCH] D116699: [clangd] Polish clangd/inlayHints and expose them by default.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added a comment.

In D116699#3227203 , @kadircet wrote:

> thanks this LGTM just a couple nits.
> my biggest concern is old endpoint will vanish all of a sudden and it might 
> create some confusion, especially for editors that turn on inlayhints without 
> an explicit user interaction.
> but i suppose plugin maintainers that implement the extension shouldn't have 
> a hard time finding out the new endpoint and migrating to it. (and I don't 
> think we've got (m)any plugins implementing the extension today.) So all 
> should be fine.

Yes, transition pains.

To be clear, *this* change isn't breaking anyone: the protocol changes in this 
patch are backwards-compatible. So the number of plugins implementing today 
isn't super-relevant.
But we're adding a new API, and encouraging people to use it, and we'll remove 
it in future. The silver lining: we can choose how long to support the two in 
parallel, I'd be surprised if it was a significant burden (I expect *much* less 
than semantic highlights, which is a complicated protocol).

Anyway I don't want to gloss over the bumpiness, it's a tradeoff to get this in 
front of users.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:168
+  addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+   InlayHintKind::ParameterHint, "", Name, ": ");
 }

kadircet wrote:
> nit: parameter name comments for `""` and `": "` (same for other calls)
We have inlay hints now, who needs comments?!



Comment at: clang-tools-extra/clangd/InlayHints.h:26
+/// Compute and return inlay hints for a file.
+///
+/// If RestrictRange is set, return only hints whose location is in that range.

kadircet wrote:
> nit: drop the empty line?
Done.
(FWIW I find comments a bit easier to read formatted like git commits: one line 
summary that stands alone, and a blank line before details. But this certainly 
isn't something we do consistently)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116699

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

[clang-tools-extra] 7c19fdd - [clangd] Polish clangd/inlayHints and expose them by default.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-07T15:12:43+01:00
New Revision: 7c19fdd59939249c23384f0900d49aab4a5f0695

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

LOG: [clangd] Polish clangd/inlayHints and expose them by default.

This means it's a "real feature" in clangd 14, albeit one that requires special
client support.

- remove "preview" from the flag description
- expose the `clangdInlayHints` capability by default
- provide `position` as well as `range`
- support `InlayHintsParams.range` to restrict the range retrieved
- inlay hint list is in document order (sorted by position)

Still to come: control feature via config rather than flag.

Fixes https://github.com/clangd/clangd/issues/313
Protocol doc is in https://github.com/llvm/clangd-www/pull/56/files

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

Added: 
clang-tools-extra/clangd/test/inlayHints.test

Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/InlayHints.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index edde19f96202..76e3806f421e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -608,6 +608,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  // FIXME: once inlayHints can be disabled in config, always advertise.
   if (Opts.InlayHints)
 ServerCaps["clangdInlayHintsProvider"] = true;
 
@@ -1210,7 +1211,8 @@ void ClangdLSPServer::onCallHierarchyIncomingCalls(
 
 void ClangdLSPServer::onInlayHints(const InlayHintsParams &Params,
Callback> Reply) {
-  Server->inlayHints(Params.textDocument.uri.file(), std::move(Reply));
+  Server->inlayHints(Params.textDocument.uri.file(), Params.range,
+ std::move(Reply));
 }
 
 void ClangdLSPServer::applyConfiguration(

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index bd7c1a05b0e2..0a5af762ad32 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -63,8 +63,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
   return !T.hidden(); // only enable non-hidden tweaks.
 };
 
-/// Enable preview of InlayHints feature.
-bool InlayHints = false;
+/// Enable InlayHints feature.
+bool InlayHints = true;
 
 /// Limit the number of references returned (0 means no limit).
 size_t ReferencesLimit = 0;

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index e106c31ce0df..69f37662c4ab 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -759,12 +759,13 @@ void ClangdServer::incomingCalls(
  });
 }
 
-void ClangdServer::inlayHints(PathRef File,
+void ClangdServer::inlayHints(PathRef File, llvm::Optional 
RestrictRange,
   Callback> CB) {
-  auto Action = [CB = std::move(CB)](Expected InpAST) mutable {
+  auto Action = [RestrictRange(std::move(RestrictRange)),
+ CB = std::move(CB)](Expected InpAST) mutable {
 if (!InpAST)
   return CB(InpAST.takeError());
-CB(clangd::inlayHints(InpAST->AST));
+CB(clangd::inlayHints(InpAST->AST, std::move(RestrictRange)));
   };
   WorkScheduler->runWithAST("InlayHints", File, std::move(Action));
 }

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 0589f4fc4214..14be307e6fda 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -264,7 +264,8 @@ class ClangdServer {
  Callback>);
 
   /// Resolve inlay hints for a given document.
-  void inlayHints(PathRef File, Callback>);
+  void inlayHints(PathRef File, llvm::Optional RestrictRange,
+  Callback>);
 
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,

diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 6b9c8b1eee54..60d20a38ecd2 100644
--- a/clang-tools-extra/clangd/Inl

[PATCH] D116699: [clangd] Polish clangd/inlayHints and expose them by default.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG7c19fdd59939: [clangd] Polish clangd/inlayHints and expose 
them by default. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D116699?vs=397702&id=398122#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116699

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/InlayHints.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/inlayHints.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -23,20 +23,23 @@
 
 namespace {
 
-using ::testing::UnorderedElementsAre;
+using ::testing::ElementsAre;
 
 std::vector hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
   std::vector Result;
-  for (auto &Hint : inlayHints(AST)) {
+  for (auto &Hint : inlayHints(AST, /*RestrictRange=*/llvm::None)) {
 if (Hint.kind == Kind)
   Result.push_back(Hint);
   }
   return Result;
 }
 
+enum HintSide { Left, Right };
+
 struct ExpectedHint {
   std::string Label;
   std::string RangeName;
+  HintSide Side = Left;
 
   friend std::ostream &operator<<(std::ostream &Stream,
   const ExpectedHint &Hint) {
@@ -46,9 +49,13 @@
 
 MATCHER_P2(HintMatcher, Expected, Code, "") {
   return arg.label == Expected.Label &&
- arg.range == Code.range(Expected.RangeName);
+ arg.range == Code.range(Expected.RangeName) &&
+ arg.position ==
+ ((Expected.Side == Left) ? arg.range.start : arg.range.end);
 }
 
+MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
+
 template 
 void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
  ExpectedHints... Expected) {
@@ -58,18 +65,23 @@
   auto AST = TU.build();
 
   EXPECT_THAT(hintsOfKind(AST, Kind),
-  UnorderedElementsAre(HintMatcher(Expected, Source)...));
+  ElementsAre(HintMatcher(Expected, Source)...));
 }
 
+// Hack to allow expression-statements operating on parameter packs in C++14.
+template  void ignore(T &&...) {}
+
 template 
 void assertParameterHints(llvm::StringRef AnnotatedSource,
   ExpectedHints... Expected) {
+  ignore(Expected.Side = Left...);
   assertHints(InlayHintKind::ParameterHint, AnnotatedSource, Expected...);
 }
 
 template 
 void assertTypeHints(llvm::StringRef AnnotatedSource,
  ExpectedHints... Expected) {
+  ignore(Expected.Side = Right...);
   assertHints(InlayHintKind::TypeHint, AnnotatedSource, Expected...);
 }
 
@@ -631,6 +643,18 @@
   ExpectedHint{": int", "var"});
 }
 
+TEST(InlayHints, RestrictRange) {
+  Annotations Code(R"cpp(
+auto a = false;
+[[auto b = 1;
+auto c = '2';]]
+auto d = 3.f;
+  )cpp");
+  auto AST = TestTU::withCode(Code.code()).build();
+  EXPECT_THAT(inlayHints(AST, Code.range()),
+  ElementsAre(labelIs(": int"), labelIs(": char")));
+}
+
 // FIXME: Low-hanging fruit where we could omit a type hint:
 //  - auto x = TypeName(...);
 //  - auto x = (TypeName) (...);
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -327,8 +327,14 @@
 Hidden,
 };
 
-opt InlayHints{"inlay-hints", cat(Features),
- desc("Enable preview of InlayHints feature"), init(false)};
+opt InlayHints{
+"inlay-hints",
+cat(Features),
+desc("Enable InlayHints feature"),
+init(ClangdLSPServer::Options().InlayHints),
+// FIXME: allow inlayHints to be disabled in Config and remove this option.
+Hidden,
+};
 
 opt WorkerThreadsCount{
 "j",
Index: clang-tools-extra/clangd/test/inlayHints.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/inlayHints.test
@@ -0,0 +1,45 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
+  "uri":"test:///main.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":"int foo(int bar);\nint x = foo(42);\nint y = foo(42);"
+}}}
+---
+{"jsonrpc":"2.0","id":1,"me

[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 398123.
pengfei added a comment.

Structure the layout spelling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115942

Files:
  clang/lib/Basic/Targets/X86.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/long-double-abi-align.ll
  llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
  llvm/test/CodeGen/X86/scalar-fp-to-i64.ll

Index: llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
@@ -909,8 +909,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:flds __real@5f00
 ; X86-AVX512-WIN-NEXT:xorl %edx, %edx
@@ -985,8 +985,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:flds __real@5f00
 ; X86-SSE3-WIN-NEXT:xorl %edx, %edx
@@ -1061,8 +1061,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:flds __real@5f00
 ; X86-SSE2-WIN-NEXT:xorl %edx, %edx
@@ -1161,8 +1161,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:flds __real@5f00
 ; X87-WIN-NEXT:fucom %st(1)
@@ -1235,8 +1235,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:fisttpll (%esp)
 ; X86-AVX512-WIN-NEXT:movl (%esp), %eax
@@ -1275,8 +1275,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:fisttpll (%esp)
 ; X86-SSE3-WIN-NEXT:movl (%esp), %eax
@@ -1315,8 +1315,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X86-SSE2-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
@@ -1379,8 +1379,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X87-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
Index: llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
@@ -344,8 +344,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:fisttpll (%esp)
 ; X86-AVX512-WIN-NEXT:movl (%esp), %eax
@@ -382,8 +382,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:fistt

[PATCH] D116806: [clang-format] Ensure we can correctly parse lambda in the template argument list

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, JohelEGP.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/46505

The presence of a lambda in an argument template list ignored the [] as a 
lambda at all, this caused the contents of the <> to be incorrectly analyzed.

  struct Y : X < [] {
return 0;
  } > {};

Fixes: #46505


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116806

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23161,6 +23161,16 @@
Style);
 }
 
+TEST_F(FormatTest, ShortTemplatedArgumentLists) {
+  auto Style = getLLVMStyle();
+
+  verifyFormat("struct Y : X<[] { return 0; }> {};", Style);
+  verifyFormat("struct Y<[] { return 0; }> {};", Style);
+
+  verifyFormat("struct Z : X {};", Style);
+  verifyFormat("template  struct X {};", Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2869,6 +2869,9 @@
 if (!tryToParseBracedList())
   break;
   }
+  if (FormatTok->is(tok::l_square))
+if (!tryToParseLambda())
+  break;
   if (FormatTok->Tok.is(tok::semi))
 return;
   if (Style.isCSharp() && FormatTok->is(Keywords.kw_where)) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23161,6 +23161,16 @@
Style);
 }
 
+TEST_F(FormatTest, ShortTemplatedArgumentLists) {
+  auto Style = getLLVMStyle();
+
+  verifyFormat("struct Y : X<[] { return 0; }> {};", Style);
+  verifyFormat("struct Y<[] { return 0; }> {};", Style);
+
+  verifyFormat("struct Z : X {};", Style);
+  verifyFormat("template  struct X {};", Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2869,6 +2869,9 @@
 if (!tryToParseBracedList())
   break;
   }
+  if (FormatTok->is(tok::l_square))
+if (!tryToParseLambda())
+  break;
   if (FormatTok->Tok.is(tok::semi))
 return;
   if (Style.isCSharp() && FormatTok->is(Keywords.kw_where)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116792: [AST] lookup in parent DeclContext for transparent DeclContext

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I had to do something similar for this at one point: 
https://github.com/llvm/llvm-project/commit/90010c2e1d60c6a9a4a0b30a113d4dae2b7214eb

I seem to remember hitting this assert, and from my end, I think I decided even 
calling 'lookup' with the linkage spec to be a mistake (BTW, you might consider 
updating that 'Encloses' for 'export' as well!).

Is there any mechanism in the parent call of 'lookup' here to make it get the 
right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116792

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

[PATCH] D116721: [Tooling] When transferring compile commands between files, always use '--'

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116721

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

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu added a comment.

I don't have commit access; can someone commit this for me? Please use author 
"Alex Xu (Hello71) ". Thanks!


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

https://reviews.llvm.org/D116337

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

[PATCH] D116755: Revert "[CodeGen] Mark fma as const for Android"

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu added a comment.

I don't have commit access; can someone commit this for me? Please use author 
"Alex Xu (Hello71) ". Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116755

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

[PATCH] D116764: [clang][OpenMP5.1] Initial parsing/sema for 'indirect' clause

[Alexey Bataev via Phabricator via cfe-commits]

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

LG with a nit




Comment at: clang/lib/Parse/ParseOpenMP.cpp:1844
+  if (!IsDeviceTypeClause && !IsIndirectClause &&
+   DTCI.Kind == OMPD_begin_declare_target) {
 Diag(Tok, diag::err_omp_declare_target_unexpected_clause)

Formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116764

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

[clang] 4681ae9 - [clang-format] Use range-for loops. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-07T16:06:11+01:00
New Revision: 4681ae9353ed89d28a95f4d07e8050f8772ae724

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

LOG: [clang-format] Use range-for loops. NFC.

* Avoid if check on every element of the loop when printing symbols.

Added: 


Modified: 
clang/lib/Format/QualifierAlignmentFixer.cpp
clang/lib/Format/SortJavaScriptImports.cpp
clang/lib/Format/TokenAnalyzer.cpp
clang/lib/Format/TokenAnnotator.h

Removed: 




diff  --git a/clang/lib/Format/QualifierAlignmentFixer.cpp 
b/clang/lib/Format/QualifierAlignmentFixer.cpp
index ec19a38537683..a53db5d11848d 100644
--- a/clang/lib/Format/QualifierAlignmentFixer.cpp
+++ b/clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -88,11 +88,11 @@ std::pair 
QualifierAlignmentFixer::analyze(
   // Don't make replacements that replace nothing.
   tooling::Replacements NonNoOpFixes;
 
-  for (auto I = Fixes.begin(), E = Fixes.end(); I != E; ++I) {
-StringRef OriginalCode = Code.substr(I->getOffset(), I->getLength());
+  for (const tooling::Replacement &Fix : Fixes) {
+StringRef OriginalCode = Code.substr(Fix.getOffset(), Fix.getLength());
 
-if (!OriginalCode.equals(I->getReplacementText())) {
-  auto Err = NonNoOpFixes.add(*I);
+if (!OriginalCode.equals(Fix.getReplacementText())) {
+  auto Err = NonNoOpFixes.add(Fix);
   if (Err)
 llvm::errs() << "Error adding replacements : "
  << llvm::toString(std::move(Err)) << "\n";
@@ -396,9 +396,9 @@ LeftRightQualifierAlignmentFixer::analyze(
   tok::TokenKind QualifierToken = getTokenFromQualifier(Qualifier);
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
-  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
-FormatToken *First = AnnotatedLines[I]->First;
-const auto *Last = AnnotatedLines[I]->Last;
+  for (AnnotatedLine *Line : AnnotatedLines) {
+FormatToken *First = Line->First;
+const auto *Last = Line->Last;
 
 for (const auto *Tok = First; Tok && Tok != Last && Tok->Next;
  Tok = Tok->Next) {

diff  --git a/clang/lib/Format/SortJavaScriptImports.cpp 
b/clang/lib/Format/SortJavaScriptImports.cpp
index 52c222a8b3dc2..21f0bdd7323d4 100644
--- a/clang/lib/Format/SortJavaScriptImports.cpp
+++ b/clang/lib/Format/SortJavaScriptImports.cpp
@@ -338,10 +338,12 @@ class JavaScriptImportSorter : public TokenAnalyzer {
 // Stitch together the module reference start...
 Buffer += getSourceText(Reference.Range.getBegin(), 
Reference.SymbolsStart);
 // ... then the references in order ...
-for (auto I = Symbols.begin(), E = Symbols.end(); I != E; ++I) {
-  if (I != Symbols.begin())
+if (!Symbols.empty()) {
+  Buffer += getSourceText(Symbols.front().Range);
+  for (const JsImportedSymbol &Symbol : llvm::drop_begin(Symbols)) {
 Buffer += ",";
-  Buffer += getSourceText(I->Range);
+Buffer += getSourceText(Symbol.Range);
+  }
 }
 // ... followed by the module reference end.
 Buffer += getSourceText(Reference.SymbolsEnd, Reference.Range.getEnd());
@@ -410,9 +412,8 @@ class JavaScriptImportSorter : public TokenAnalyzer {
  << ", cat: " << Reference.Category
  << ", url: " << Reference.URL
  << ", prefix: " << Reference.Prefix;
-for (size_t I = 0; I < Reference.Symbols.size(); ++I)
-  llvm::dbgs() << ", " << Reference.Symbols[I].Symbol << " as "
-   << Reference.Symbols[I].Alias;
+for (const JsImportedSymbol &Symbol : Reference.Symbols)
+  llvm::dbgs() << ", " << Symbol.Symbol << " as " << Symbol.Alias;
 llvm::dbgs() << ", text: " << getSourceText(Reference.Range);
 llvm::dbgs() << "}\n";
   });

diff  --git a/clang/lib/Format/TokenAnalyzer.cpp 
b/clang/lib/Format/TokenAnalyzer.cpp
index 3f77220931753..d0754e0c11128 100644
--- a/clang/lib/Format/TokenAnalyzer.cpp
+++ b/clang/lib/Format/TokenAnalyzer.cpp
@@ -127,8 +127,8 @@ std::pair 
TokenAnalyzer::process() {
 
 LLVM_DEBUG({
   llvm::dbgs() << "Replacements for run " << Run << ":\n";
-  for (const tooling::Replacement &Replacement : RunResult.first)
-llvm::dbgs() << Replacement.toString() << "\n";
+  for (const tooling::Replacement &Fix : RunResult.first)
+llvm::dbgs() << Fix.toString() << "\n";
 });
 for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
   delete AnnotatedLines[i];

diff  --git a/clang/lib/Format/TokenAnnotator.h 
b/clang/lib/Format/TokenAnnotator.h
index 6e5e62cd4d82a..384a671c981f2 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -53,10 +53,9 @@ class AnnotatedLine {
 // left th

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

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

Use pointers instead of indices to identify `DirectoryLookup` objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/CMakeLists.txt
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
@@ -24,6 +25,12 @@
 namespace clang {
 namespace {
 
+static std::shared_ptr createTargetOptions() {
+  auto TargetOpts = std::make_shared();
+  TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
+  return TargetOpts;
+}
+
 // The test fixture.
 class HeaderSearchTest : public ::testing::Test {
 protected:
@@ -31,12 +38,10 @@
   : VFS(new llvm::vfs::InMemoryFileSystem), FileMgr(FileMgrOpts, VFS),
 DiagID(new DiagnosticIDs()),
 Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
-SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions),
+SourceMgr(Diags, FileMgr), TargetOpts(createTargetOptions()),
+Target(TargetInfo::CreateTargetInfo(Diags, TargetOpts)),
 Search(std::make_shared(), SourceMgr, Diags,
-   LangOpts, Target.get()) {
-TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
-Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
-  }
+   LangOpts, Target.get()) {}
 
   void addSearchDir(llvm::StringRef Dir) {
 VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
@@ -47,6 +52,27 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void setSearchDirs(llvm::ArrayRef QuotedDirs,
+ llvm::ArrayRef AngledDirs) {
+auto AddPath = [&](StringRef Dir, bool IsAngled) {
+  VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+   /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+  auto Group = IsAngled ? frontend::IncludeDirGroup::Angled
+: frontend::IncludeDirGroup::Quoted;
+  Search.getHeaderSearchOpts().AddPath(Dir, Group,
+   /*IsFramework=*/false,
+   /*IgnoreSysRoot=*/true);
+};
+
+for (llvm::StringRef Dir : QuotedDirs)
+  AddPath(Dir, /*IsAngled=*/false);
+for (llvm::StringRef Dir : AngledDirs)
+  AddPath(Dir, /*IsAngled=*/true);
+
+clang::ApplyHeaderSearchOptions(Search, Search.getHeaderSearchOpts(),
+LangOpts, Target->getTriple());
+  }
+
   void addHeaderMap(llvm::StringRef Filename,
 std::unique_ptr Buf,
 bool isAngled = false) {
@@ -63,6 +89,17 @@
 Search.AddSearchPath(DL, isAngled);
   }
 
+  void createModule(StringRef Mod) {
+std::string ModDir = ("/" + Mod).str();
+std::string ModHeader = (Mod + ".h").str();
+VFS->addFile(
+ModDir + "/module.modulemap", 0,
+llvm::MemoryBuffer::getMemBufferCopy(
+("module " + Mod + " { header \"" + ModHeader + "\" }").str()));
+VFS->addFile(ModDir + "/" + ModHeader, 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -224,5 +261,35 @@
   EXPECT_EQ(FI->Framework.str(), "Foo");
 }
 
+TEST_F(HeaderSearchTest, SearchPathUsage) {
+  Search.getHeaderSearchOpts().ImplicitModuleMaps = true;
+
+  setSearchDirs(/*QuotedDirs=*/{"/M0"}, /*AngledDirs=*/{"/M2", "/M3"});
+  createModule("M0");
+  createModule("M2");
+  createModule("M3");
+
+  {
+Module *M2 = Search.lookupModule("M2");
+EXPECT_NE(M2, nullptr);
+std::vector ExpectedSearchDirUsageAfterM2{false, true, false};
+EXPECT_EQ(Search.getSearchDirUsage(), ExpectedSearchDirUsageAfterM2);
+std::vector ExpectedUserEntryUsageAfterM2{false, true, false};
+EXPECT_EQ(Search.computeUserEntryUsage(), ExpectedUserEntryUsageAfterM2);
+  }
+
+  addSearchDir("/M1");
+  createModule("M1");
+
+  {
+Module *M1 = Search.lookupModule("M1");
+EXPECT_NE(M1, nullptr);
+std::vector ExpectedSearchDirUsageAfterM1{false, true, true, false};
+EXPECT_EQ(Search.getSearchDirUsage(), ExpectedSearchDirUsageAfterM1);
+std::vector ExpectedUserEntryUsageAfterM1{false, true, false};
+EXPECT_EQ(Search.computeUserEntryUsage(), ExpectedUserEntryUsageAfterM1);
+  }
+}
+
 } // namespace
 } // namespace 

[clang] b2ed9f3 - [Clang] Implement the rest of __builtin_elementwise_* functions.

[Florian Hahn via cfe-commits]

Author: Jun Zhang
Date: 2022-01-07T15:11:36Z
New Revision: b2ed9f3f44d084bf2aae0af1b84f76a68a7c477b

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

LOG: [Clang] Implement the rest of __builtin_elementwise_* functions.

The patch implement the rest of __builtin_elementwise_* functions
specified in D111529, including:
* __builtin_elementwise_floor
* __builtin_elementwise_roundeven
* __builtin_elementwise_trunc

Signed-off-by: Jun 

Reviewed By: fhahn

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

Added: 


Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtins-elementwise-math.c
clang/test/Sema/builtins-elementwise-math.c

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 45d4451637495..f73efdde3e2b1 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -647,6 +647,9 @@ BUILTIN(__builtin_elementwise_abs, "v.", "nct")
 BUILTIN(__builtin_elementwise_max, "v.", "nct")
 BUILTIN(__builtin_elementwise_min, "v.", "nct")
 BUILTIN(__builtin_elementwise_ceil, "v.", "nct")
+BUILTIN(__builtin_elementwise_floor, "v.", "nct")
+BUILTIN(__builtin_elementwise_roundeven, "v.", "nct")
+BUILTIN(__builtin_elementwise_trunc, "v.", "nct")
 BUILTIN(__builtin_reduce_max, "v.", "nct")
 BUILTIN(__builtin_reduce_min, "v.", "nct")
 BUILTIN(__builtin_reduce_xor, "v.", "nct")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 50f59a2abab85..e98c9d5fd8444 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3143,6 +3143,15 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__builtin_elementwise_ceil:
 return RValue::get(
 emitUnaryBuiltin(*this, E, llvm::Intrinsic::ceil, "elt.ceil"));
+  case Builtin::BI__builtin_elementwise_floor:
+return RValue::get(
+emitUnaryBuiltin(*this, E, llvm::Intrinsic::floor, "elt.floor"));
+  case Builtin::BI__builtin_elementwise_roundeven:
+return RValue::get(emitUnaryBuiltin(*this, E, llvm::Intrinsic::roundeven,
+"elt.roundeven"));
+  case Builtin::BI__builtin_elementwise_trunc:
+return RValue::get(
+emitUnaryBuiltin(*this, E, llvm::Intrinsic::trunc, "elt.trunc"));
 
   case Builtin::BI__builtin_elementwise_max: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 03608a339e55e..908cb78fbb0a7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2189,9 +2189,12 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
 break;
   }
 
-  // __builtin_elementwise_ceil restricts the element type to floating point
+  // These builtins restrict the element type to floating point
   // types only.
-  case Builtin::BI__builtin_elementwise_ceil: {
+  case Builtin::BI__builtin_elementwise_ceil:
+  case Builtin::BI__builtin_elementwise_floor:
+  case Builtin::BI__builtin_elementwise_roundeven:
+  case Builtin::BI__builtin_elementwise_trunc: {
 if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
   return ExprError();
 

diff  --git a/clang/test/CodeGen/builtins-elementwise-math.c 
b/clang/test/CodeGen/builtins-elementwise-math.c
index c0c948ffea78b..a4785b8f28f22 100644
--- a/clang/test/CodeGen/builtins-elementwise-math.c
+++ b/clang/test/CodeGen/builtins-elementwise-math.c
@@ -205,3 +205,51 @@ void test_builtin_elementwise_ceil(float f1, float f2, 
double d1, double d2,
   // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
   vf2 = __builtin_elementwise_ceil(vf1);
 }
+
+void test_builtin_elementwise_floor(float f1, float f2, double d1, double d2,
+float4 vf1, float4 vf2) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_floor(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.floor.f32(float [[F1]])
+  f2 = __builtin_elementwise_floor(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.floor.f64(double [[D1]])
+  d2 = __builtin_elementwise_floor(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 
16
+  // CHECK-NEXT: call <4 x float> @llvm.floor.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_floor(vf1);
+}
+
+void test_builtin_elementwise_roundeven(float f1, float f2, double d1, double 
d2,
+float4 vf1, float4 vf2) {
+  // CHECK-LABEL: define void @test_builtin_

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Florian Hahn 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 rGb2ed9f3f44d0: [Clang] Implement the rest of 
__builtin_elementwise_* functions. (authored by junaire, committed by fhahn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/builtins-elementwise-math.c

Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -156,3 +156,66 @@
   uv = __builtin_elementwise_ceil(uv);
   // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
 }
+
+void test_builtin_elementwise_floor(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_floor(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_floor();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_floor(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_floor(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_floor(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_floor(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
+
+void test_builtin_elementwise_roundeven(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_roundeven(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_roundeven();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_roundeven(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_roundeven(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_roundeven(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_roundeven(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
+
+void test_builtin_elementwise_trunc(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_trunc(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_trunc();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_trunc(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_trunc(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_trunc(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_trunc(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
Index: clang/test/CodeGen/builtins-elementwise-math.c
===
--- clang/test/CodeGen/builtins-elementwise-math.c
+++ clang/test/CodeGen/builtins-elementwise-math.c
@@ -205,3 +205,51 @@
   // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
   vf2 = __builtin_elementwise_ceil(vf1);
 }
+
+void test_builtin_elementwise_floor(float f1, float f2, double d1, double d2,
+float4 vf1, float4 vf2) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_floor(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.floor.f32(float [[F1]])
+  f2 = __builtin_elementwise_floor(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.floor.f64(double [[D1]])
+  d2 = __builtin_elementwise_floor(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.floor.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_flo

[PATCH] D116806: [clang-format] Ensure we can correctly parse lambda in the template argument list

[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.




Comment at: clang/unittests/Format/FormatTest.cpp:23171
+  verifyFormat("struct Z : X {};", Style);
+  verifyFormat("template  struct X {};", Style);
+}

Is this line necessary? It doesn't seem to test anything new, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116806

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

[PATCH] D116509: [Builtins] Add missing the macro 'y' description in comments

[Ties Stuij via Phabricator via cfe-commits]

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

LGTM, thanks!

Nit: perhaps make the commit message a little bit clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116509

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

[clang-tools-extra] e56a9c9 - Remove redundant return statements (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2022-01-07T07:42:35-08:00
New Revision: e56a9c9b5b83923b8ed7b21994e6f9f11166aae0

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

LOG: Remove redundant return statements (NFC)

Identified by readability-redundant-control-flow.

Added: 


Modified: 
clang-tools-extra/clangd/TUScheduler.cpp
mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
mlir/lib/Dialect/Affine/Utils/Utils.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 33df549d238de..43b4d8ca8881f 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -856,7 +856,6 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
   // LatestPreamble is only populated by ASTWorker thread.
   return LatestPreamble || !PreambleRequests.empty() || Done;
 });
-return;
   };
   startTask(TaskName, std::move(Task), UpdateType{WantDiags, ContentChanged},
 TUScheduler::NoInvalidation);

diff  --git a/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp 
b/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
index 036cc6b03f76e..fe15d447940c6 100644
--- a/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
@@ -1324,7 +1324,6 @@ static Operation *vectorizeAffineForOp(AffineForOp forOp,
   /*bodyBuilder=*/[](OpBuilder &, Location, Value, ValueRange) {
 // Make sure we don't create a default terminator in the loop body as
 // the proper terminator will be added during vectorization.
-return;
   });
 
   // Register loop-related replacements:

diff  --git a/mlir/lib/Dialect/Affine/Utils/Utils.cpp 
b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
index 480c09aa97a1f..1037040333cb5 100644
--- a/mlir/lib/Dialect/Affine/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
@@ -537,7 +537,6 @@ static bool hasNoInterveningEffect(Operation *start, T 
memOp) {
 // Otherwise, conservatively assume generic operations have the effect
 // on the operation
 hasSideEffect = true;
-return;
   };
 
   // Check all paths from ancestor op `parent` to the operation `to` for the



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

[PATCH] D116377: [libTooling] Adds more support for constructing object access expressions.

[Andy Soffer via Phabricator via cfe-commits]

asoffer added inline comments.



Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:73
+  cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+  const auto QuacksLikeASmartPointer = cxxRecordDecl(
+  hasMethod(cxxMethodDecl(hasOverloadedOperatorName("->"),

Naming nit: This could be confusing for anyone not familiar with the "duck 
typing" idiom.
BehavesLikeASmartPointer?



Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:78
+  returns(qualType(references(type()));
+  const auto SmartPointer = qualType(hasDeclaration(
+  cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer;

I think std::optional satisfies these requirements but should not be considered 
a smart-pointer.



Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:79
+  const auto SmartPointer = qualType(hasDeclaration(
+  cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer;
+  return match(SmartPointer, Ty, Context).size() > 0;

The known smart pointers quack like smart pointers, so this is redundant, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116377

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

[PATCH] D113676: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

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

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Preprocessor/search-path-usage-modules.m
  clang/test/Preprocessor/search-path-usage.m

Index: clang/test/Preprocessor/search-path-usage.m
===
--- clang/test/Preprocessor/search-path-usage.m
+++ clang/test/Preprocessor/search-path-usage.m
@@ -128,19 +128,3 @@
 // expected-remark-re {{search path used: '{{.*}}/b-missing.hmap'}}
 #endif
 #endif
-
-// Check that search paths with module maps are reported.
-//
-// RUN: mkdir %t/modulemap_abs
-// RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g"\
-// RUN:   %S/Inputs/search-path-usage/modulemap_abs/module.modulemap.template \
-// RUN: > %t/modulemap_abs/module.modulemap
-// RUN: %clang_cc1 -Eonly %s -Rsearch-path-usage   \
-// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules \
-// RUN:   -I %t/modulemap_abs  \
-// RUN:   -I %S/Inputs/search-path-usage/a \
-// RUN:   -DMODMAP_ABS -verify
-#ifdef MODMAP_ABS
-@import b; // \
-// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}}
-#endif
Index: clang/test/Preprocessor/search-path-usage-modules.m
===
--- /dev/null
+++ clang/test/Preprocessor/search-path-usage-modules.m
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -Eonly %t/test-both.m   -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m
+// RUN: %clang_cc1 -Eonly %t/test-both.m   -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m
+
+// RUN: %clang_cc1 -Eonly %t/test-first.m  -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-first.m
+// RUN: %clang_cc1 -Eonly %t/test-second.m -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-second.m
+
+//--- sp1/module.modulemap
+module mod1 { header "mod1.h" }
+//--- sp1/mod1.h
+int module1();
+
+//--- sp2/module.modulemap
+module mod2 { header "mod2.h" }
+//--- sp2/mod2.h
+int module2();
+
+//--- test-both.m
+@import mod1;
+@import mod2;
+// CHECK: search path used: '{{.*}}/sp1'
+// CHECK: search path used: '{{.*}}/sp2'
+
+//--- test-first.m
+@import mod1;
+// CHECK-NOT: search path used: '{{.*}}/sp2'
+// CHECK: search path used: '{{.*}}/sp1'
+// CHECK-NOT: search path used: '{{.*}}/sp2'
+
+//--- test-second.m
+@import mod2;
+// CHECK-NOT: search path used: '{{.*}}/sp1'
+// CHECK: search path used: '{{.*}}/sp2'
+// CHECK-NOT: search path used: '{{.*}}/sp1'
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3907,7 +3907,7 @@
 // An implicitly-loaded module file should have its module listed in some
 // module map file that we've already loaded.
 Module *M =
-PP.getHeaderSearchInfo().lookupModule(F.ModuleName, F.ImportLoc);
+PP.getHeaderSearchInfo().lookupModule(F.ModuleName, SourceLocation());
 auto &Map = PP.getHeaderSearchInfo().getModuleMap();
 const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr;
 // Don't emit module relocation error if we have -fno-validate-pch
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -107,8 +107,13 @@
 Module *ModuleMap::makeModule(StringRef Name, SourceLocation DefinitionLoc,
   Module *Parent, bool IsFramework, bool IsExplicit,
   unsigned VisibilityID) {
-  return new Module(Name, DefinitionLoc, Parent, IsFramework, IsExplicit,
-VisibilityID);
+  Module *Result = new Module(Name, DefinitionLoc, Parent, IsFramework,
+  IsExplicit, VisibilityID);
+
+  for (const auto &Callback : Callbacks)
+Callback->moduleMapModuleCreated(Result);
+
+  return Result;
 }
 
 Module::ExportDecl
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/Header

[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 398151.
zero9178 added a comment.

Utilize operator< of std::pair to simplify the comparison A LOT, simplify using 
llvm::find_if and add additional test case


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

https://reviews.llvm.org/D116775

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaCXX/typo-correction.cpp


Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -757,3 +757,34 @@
 b = g_volatile_uchar // expected-error {{did you mean 'g_volatile_char'}}
   };
 }
+
+namespace PR47272
+{
+
+namespace Views {
+int Take(); // expected-note{{'Views::Take' declared here}}
+}
+
+namespace [[deprecated("use Views instead")]] View {
+using Views::Take;
+}
+
+namespace B {
+int pr47272();  // expected-note{{'B::pr47272' declared here}}
+}
+
+namespace [[deprecated]] A {
+using B::pr47272;
+}
+
+namespace [[deprecated]] C {
+using B::pr47272;
+}
+
+void function() {
+  int x = ::Take(); // expected-error{{no member named 'Take' in the global 
namespace; did you mean 'Views::Take'?}}
+  int y = ::pr47272(); // expected-error{{no member named 'pr47272' in the 
global namespace; did you mean 'B::pr47272'?}}
+}
+}
+
+
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -4307,18 +4307,35 @@
   if (!CList.empty() && !CList.back().isResolved())
 CList.pop_back();
   if (NamedDecl *NewND = Correction.getCorrectionDecl()) {
-std::string CorrectionStr = Correction.getAsString(SemaRef.getLangOpts());
-for (TypoResultList::iterator RI = CList.begin(), RIEnd = CList.end();
- RI != RIEnd; ++RI) {
-  // If the Correction refers to a decl already in the result list,
-  // replace the existing result if the string representation of Correction
-  // comes before the current result alphabetically, then stop as there is
-  // nothing more to be done to add Correction to the candidate set.
-  if (RI->getCorrectionDecl() == NewND) {
-if (CorrectionStr < RI->getAsString(SemaRef.getLangOpts()))
-  *RI = Correction;
-return;
-  }
+auto RI = llvm::find_if(CList, [NewND](const TypoCorrection &typoCorr) {
+  return typoCorr.getCorrectionDecl() == NewND;
+});
+if (RI != CList.end()) {
+  // The Correction refers to a decl already in the list. No insertion is
+  // necessary and all further cases will return.
+
+  auto IsDeprecated = [](Decl *decl) {
+while (decl) {
+  if (decl->isDeprecated())
+return true;
+  decl = llvm::dyn_cast_or_null(decl->getDeclContext());
+}
+return false;
+  };
+
+  // Prefer non deprecated Corrections over deprecated and only then
+  // sort using an alphabetical order
+  std::pair newKey = {
+  IsDeprecated(Correction.getFoundDecl()),
+  Correction.getAsString(SemaRef.getLangOpts())};
+
+  std::pair prevKey = {
+  IsDeprecated(RI->getFoundDecl()),
+  RI->getAsString(SemaRef.getLangOpts())};
+
+  if (newKey < prevKey)
+*RI = Correction;
+  return;
 }
   }
   if (CList.empty() || Correction.isResolved())


Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -757,3 +757,34 @@
 b = g_volatile_uchar // expected-error {{did you mean 'g_volatile_char'}}
   };
 }
+
+namespace PR47272
+{
+
+namespace Views {
+int Take(); // expected-note{{'Views::Take' declared here}}
+}
+
+namespace [[deprecated("use Views instead")]] View {
+using Views::Take;
+}
+
+namespace B {
+int pr47272();  // expected-note{{'B::pr47272' declared here}}
+}
+
+namespace [[deprecated]] A {
+using B::pr47272;
+}
+
+namespace [[deprecated]] C {
+using B::pr47272;
+}
+
+void function() {
+  int x = ::Take(); // expected-error{{no member named 'Take' in the global namespace; did you mean 'Views::Take'?}}
+  int y = ::pr47272(); // expected-error{{no member named 'pr47272' in the global namespace; did you mean 'B::pr47272'?}}
+}
+}
+
+
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -4307,18 +4307,35 @@
   if (!CList.empty() && !CList.back().isResolved())
 CList.pop_back();
   if (NamedDecl *NewND = Correction.getCorrectionDecl()) {
-std::string CorrectionStr = Correction.getAsString(SemaRef.getLangOpts());
-for (TypoResultList::iterator RI = CList.begin(), RIEnd = CList.end();
- RI != RIEnd; ++RI) {
-  // If the Correction refers to a decl already in the result list,
-  // replace the existing 

[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

[Markus Böck via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaLookup.cpp:4345
+  return;
 }
   }

Quuxplusone wrote:
> It seems like this code is overdue for a "compare these two corrections" 
> //predicate//. The simple version would be
> ```
> auto IsBetter = [&](const TypoCorrection& TC1, const TypoCorrection& TC2) {
> std::pair Key1 = { IsDeprecated(TC1.getFoundDecl()), 
> TC1.getAsString(SemaRef.getLangOpts()) };
> std::pair Key2 = { IsDeprecated(TC2.getFoundDecl()), 
> TC2.getAsString(SemaRef.getLangOpts()) };
> return Key1 < Key2;  // prefer non-deprecated, alphabetically earlier 
> declarations
> };
> for (TypoCorrection& TC : CList) {
> if (IsBetter(Correction, TC))
> TC = Correction;
> }
> ```
> Actually, you could even replicate the hoisting of the loop-invariant stuff, 
> the way the old code did:
> ```
> std::pair CorrectionKey = { 
> IsDeprecated(Correction.getFoundDecl()), 
> Correction.getAsString(SemaRef.getLangOpts()) };
> auto CorrectionIsBetterThan = [&](const TypoCorrection& TC) {
> std::pair Key = { IsDeprecated(TC.getFoundDecl()), 
> TC.getAsString(SemaRef.getLangOpts()) };
> return CorrectionKey < Key;  // prefer non-deprecated, alphabetically 
> earlier declarations
> };
> for (TypoCorrection& TC : CList) {
> if (CorrectionIsBetterThan(TC))
> TC = Correction;
> }
> ```
Thanks I really liked the idea of using std::pairs comparison for that. 
I chose to separate the concerns, by first finding the declaration in the list 
and only then using the comparison of the found decl and the new correction. 


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

https://reviews.llvm.org/D116775

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

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

The two false positive warnings in the kernel are fixed with this patch and 
there were no new warnings introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116059

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis added a comment.

The current behavior when `ColumnLimit: 0` and `JavaScriptWrapImports: false` 
formats this:

  import {aaa} from "abc";
  import {aaa, bbb, ccc} from "def";
  import {aaa, bbb} from "defghi";
  import {aaa, long, ccc,} from "ghi";
  import {
aaa,
ccc,
bbb
  } from "jkl";

to this:

  import {aaa} from "abc";
  import {aaa,
  bbb,
  ccc} from "def";
  import {aaa,
  bbb} from "defghi";
  import {aaa,
  ccc,
  long,} from "ghi";
  import {
aaa,
bbb,
ccc
  } from "jkl";


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

https://reviews.llvm.org/D116638

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

[PATCH] D114706: [analyzer] Fix sensitive argument logic in GenericTaintChecker

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> The semantics of taint sinks is that if ANY of the arguments is tainted, a
> warning should be emmitted. Before this change, if there were multiple
> arguments that are sensitive, and if the first arg is not tainted, but any of
> the noninitial are tainted, a warning is not emitted. After this change the
> correct semantics is reflected in code.

It worked well with custom sinks, didn't it? So, as I see, this patch is about 
system calls and subscript indices. Could you please update the description to 
have this information?




Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:951-957
+  // Find the first argument that receives a tainted value.
+  // The report is emitted as a side-effect.
+  return llvm::find_if(SensitiveArgs, [this, &C, &Call](unsigned Idx) {
+   return Idx < Call.getNumArgs() &&
+  generateReportIfTainted(Call.getArgExpr(Idx), MsgCustomSink,
+  C);
+ }) != SensitiveArgs.end();

steakhal wrote:
> These algorithms are not supposed to have side-effects. Even though we 
> technically could have side-effects, I would still apply it separately.
> That being said, this seems to be a copy-paste version of the previous hunk, 
> like 3rd time.

> That being said, this seems to be a copy-paste version of the previous hunk, 
> like 3rd time.
+1, to put it into a named function



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114706

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

[PATCH] D116814: Accept string literal decay in conditional operator

[Elvis Stansvik via Phabricator via cfe-commits]

estan created this revision.
Herald added subscribers: carlosgalvezp, kbarton, nemanjai.
estan edited the summary of this revision.
estan edited the summary of this revision.
estan edited the summary of this revision.
estan edited the summary of this revision.
estan updated this revision to Diff 398165.
estan added a comment.
estan added reviewers: aaron.ballman, alexfh, JonasToth, george.burgess.iv.
estan published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Format code with git-clang-format


The cppcoreguidelines-pro-bounds-array-to-pointer-decay check currently accepts

  const char *b = i ? "foo" : "foobar";

but not

  const char *a = i ? "foo" : "bar";

This is because the AST is slightly different in the latter case (see 
https://godbolt.org/z/MkHVvs).

This fixes up the inconsistency by making it accept the latter form as well.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116814

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -49,3 +49,14 @@
   void *a[2];
   f2(static_cast(a)); // OK, explicit cast
 }
+
+void issue31155(int i) {
+  const char *a = i ? "foo" : "bar";// OK, decay string literal to pointer
+  const char *b = i ? "foo" : "foobar"; // OK, decay string literal to pointer
+
+  char arr[1];
+  const char *c = i ? arr : "bar";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not implicitly decay an 
array into a pointer
+  const char *d = i ? "foo" : arr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not implicitly decay an 
array into a pointer
+}
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -54,13 +54,17 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   implicitCastExpr(
-   unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   
unless(hasSourceExpression(ignoringParens(stringLiteral()
-   .bind("cast")),
+  traverse(
+  TK_AsIs,
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral(,
+  unless(hasSourceExpression(ignoringParens(conditionalOperator(
+  allOf(hasTrueExpression(stringLiteral()),
+hasFalseExpression(stringLiteral(
+  .bind("cast")),
   this);
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -49,3 +49,14 @@
   void *a[2];
   f2(static_cast(a)); // OK, explicit cast
 }
+
+void issue31155(int i) {
+  const char *a = i ? "foo" : "bar";// OK, decay string literal to pointer
+  const char *b = i ? "foo" : "foobar"; // OK, decay string literal to pointer
+
+  char arr[1];
+  const char *c = i ? arr : "bar";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not implicitly decay an array into a pointer
+  const char *d = i ? "foo" : arr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not implicitly decay an array into a pointer
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -54,13 +54,17 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D114622#3200678 , @NoQ wrote:

> These checks are almost 2000 lines of code each and it looks like all they do 
> is figure out if two statements are the same and we have a very flexible 
> reusable solution for this sort of stuff - `CloneDetector`, but none of them 
> use it. Your patch demonstrates that they all have the same bugs that need to 
> be fixed in each of them separately, so reusability seems really valuable. If 
> I was to work on these checks, the first thing I'd try would be to throw out 
> their machines and plug in a reusable solution.

There are already two way more sophisticated (forgive me my bias) 
implementations in Clang that are for checking if two statements or decls are 
the same.

1. ODRHash, used in modules to discover ODR violations
2. ASTStructuralEquivalenceContext, used in ASTImporter to discover if two AST 
nodes are the same or not (as a side effect we diagnose ODR violations as well).

It is not the first time, when such a similarity check is needed (see 
https://reviews.llvm.org/D75041). Of course reusing the before mentioned 
components would require some architectural changes, but it might be beneficial.


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

https://reviews.llvm.org/D114622

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

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

[Elvis Stansvik via Phabricator via cfe-commits]

estan commandeered this revision.
estan added a reviewer: fiesh.
estan added a comment.

I think we can close this one now that https://reviews.llvm.org/D116378 has 
landed though, since this revision was originally only about warnings in system 
macros case which is now solved (?)

I'll commandeer and close, but feel free to re-open if that's the wrong move 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88833

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

[PATCH] D115103: Leak Sanitizer port to Windows

[Clemens Wasser via Phabricator via cfe-commits]

clemenswasser added a comment.

@aganea Thanks for your help. I already did everything you wrote (I am however 
using the coreutils provided by Git).
`check-asan` works on my new computer. I still don't know why it didn't work on 
my older one...
I will try to get the lsan tests working (and hopefully passing 😉) on windows 
soon.


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

https://reviews.llvm.org/D115103

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

[PATCH] D116814: Accept string literal decay in conditional operator

[Elvis Stansvik via Phabricator via cfe-commits]

estan updated this revision to Diff 398177.
estan added a comment.

Add entry in clang-tools-extra/docs/ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116814

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -49,3 +49,14 @@
   void *a[2];
   f2(static_cast(a)); // OK, explicit cast
 }
+
+void issue31155(int i) {
+  const char *a = i ? "foo" : "bar";// OK, decay string literal to pointer
+  const char *b = i ? "foo" : "foobar"; // OK, decay string literal to pointer
+
+  char arr[1];
+  const char *c = i ? arr : "bar";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not implicitly decay an 
array into a pointer
+  const char *d = i ? "foo" : arr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not implicitly decay an 
array into a pointer
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,10 @@
 Improvements to clang-tidy
 --
 
+- Make the `cppcoreguidelines-pro-bounds-array-to-pointer-decay` check accept
+  string literal to pointer decay in conditional operator even if operands are
+  of the same length.
+
 - Ignore warnings from macros defined in system headers, if not using the
   `-system-headers` flag.
 
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -54,13 +54,17 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   implicitCastExpr(
-   unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   
unless(hasSourceExpression(ignoringParens(stringLiteral()
-   .bind("cast")),
+  traverse(
+  TK_AsIs,
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral(,
+  unless(hasSourceExpression(ignoringParens(conditionalOperator(
+  allOf(hasTrueExpression(stringLiteral()),
+hasFalseExpression(stringLiteral(
+  .bind("cast")),
   this);
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -49,3 +49,14 @@
   void *a[2];
   f2(static_cast(a)); // OK, explicit cast
 }
+
+void issue31155(int i) {
+  const char *a = i ? "foo" : "bar";// OK, decay string literal to pointer
+  const char *b = i ? "foo" : "foobar"; // OK, decay string literal to pointer
+
+  char arr[1];
+  const char *c = i ? arr : "bar";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not implicitly decay an array into a pointer
+  const char *d = i ? "foo" : arr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not implicitly decay an array into a pointer
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,10 @@
 Improvements to clang-tidy
 --
 
+- Make the `cppcoreguidelines-pro-bounds-array-to-pointer-decay` check accept
+  string literal to pointer decay in conditional operator even if operands are
+  of the same length.
+
 - Ignore warnings from macros defined in system headers, if not using the
   `-system-headers` flag.
 
Index: clang-tools-extra/clang-tidy/cppcoreguideline

[PATCH] D115183: [clang][HeaderSearch] Support framework includes in suggestPath...

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 398178.
dgoldman added a comment.

Don't suggest umbrella headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115183

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/double-quotes.m
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -47,6 +47,15 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void addSystemFrameworkSearchDir(llvm::StringRef Dir) {
+VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+ /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+auto DE = FileMgr.getOptionalDirectoryRef(Dir);
+assert(DE);
+auto DL = DirectoryLookup(*DE, SrcMgr::C_System, /*isFramework=*/true);
+Search.AddSystemSearchPath(DL);
+  }
+
   void addHeaderMap(llvm::StringRef Filename,
 std::unique_ptr Buf,
 bool isAngled = false) {
@@ -155,6 +164,29 @@
 "y/z/t.h");
 }
 
+TEST_F(HeaderSearchTest, SdkFramework) {
+  addSystemFrameworkSearchDir(
+  "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/Frameworks/");
+  bool IsSystem = false;
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
+"Frameworks/AppKit.framework/Headers/NSView.h",
+/*WorkingDir=*/"",
+/*MainFile=*/"", &IsSystem),
+"AppKit/NSView.h");
+  EXPECT_TRUE(IsSystem);
+}
+
+TEST_F(HeaderSearchTest, NestedFramework) {
+  addSystemFrameworkSearchDir("/Platforms/MacOSX/Frameworks");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+"/Platforms/MacOSX/Frameworks/AppKit.framework/Frameworks/"
+"Sub.framework/Headers/Sub.h",
+/*WorkingDir=*/"",
+/*MainFile=*/""),
+"Sub/Sub.h");
+}
+
 // Helper struct with null terminator character to make MemoryBuffer happy.
 template 
 struct NullTerminatedFile : public FileTy {
Index: clang/test/Modules/double-quotes.m
===
--- clang/test/Modules/double-quotes.m
+++ clang/test/Modules/double-quotes.m
@@ -24,8 +24,17 @@
 // because they only show up under the module A building context.
 // RUN: FileCheck --input-file=%t/stderr %s
 // CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "A0.h"
+// CHECK:  ^~
+// CHECK: 
 // CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "B.h"
+// CHECK:  ^
+// CHECK: 
 // CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #import "B.h" // Included from Z.h & A.h
+// CHECK: ^
+// CHECK: 
 
 #import "A.h"
 #import 
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -721,7 +721,8 @@
 }
 
 static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
- SmallVectorImpl &FrameworkName) {
+ SmallVectorImpl &FrameworkName,
+ SmallVectorImpl &IncludeSpelling) {
   using namespace llvm::sys;
   path::const_iterator I = path::begin(Path);
   path::const_iterator E = path::end(Path);
@@ -737,15 +738,22 @@
   // and some other variations among these lines.
   int FoundComp = 0;
   while (I != E) {
-if (*I == "Headers")
+if (*I == "Headers") {
   ++FoundComp;
-if (I->endswith(".framework")) {
-  FrameworkName.append(I->begin(), I->end());
-  ++FoundComp;
-}
-if (*I == "PrivateHeaders") {
+} else if (*I == "PrivateHeaders") {
   ++FoundComp;
   IsPrivateHeader = true;
+} else if (I->endswith(".framework")) {
+  StringRef Name = I->drop_back(10); // Drop .framework
+  // Need to reset the strings and counter to support nested frameworks.
+  FrameworkName.clear();
+  FrameworkName.append(Name.begin(), Name.end());
+  IncludeSpelling.clear();
+  IncludeSpelling.append(Name.begin(), Name.end());
+  FoundComp = 1;
+} else if (FoundComp >= 2) {
+  IncludeSpelling.push_back('/');
+  IncludeSpelling.append(I->begin(), I->end());
 }
 ++I;
   }
@@ -760,20 +768,24 @@
  bool FoundByHeaderMap = false) {
   bool IsIncluderPrivateHeader = false;
   SmallString<128> FromFramework, ToFramework;
-  if (!isFrameworkStylePath(Includer, IsIncluderPrivateHeader, FromFramework))
+  SmallString<128>

[PATCH] D116549: [OpenMP][Clang] Allow passing target features in ISA trait for metadirective clause

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 398179.
saiislam marked an inline comment as done.
saiislam added a comment.

Added diagnostic remarks for when ISA trait is not selected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116549

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_messages.c

Index: clang/test/OpenMP/metadirective_device_isa_messages.c
===
--- /dev/null
+++ clang/test/OpenMP/metadirective_device_isa_messages.c
@@ -0,0 +1,15 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux -emit-llvm-only -target-cpu znver1 %s -Rremark-backend-plugin
+
+#ifndef HEADER
+#define HEADER
+
+void bar();
+
+void foo() {
+#pragma omp metadirective when(device = {isa("some-unsupported-feature")} \
+   : parallel) default(single) // expected-remark {{isa trait 'some-unsupported-feature' is not a valid feature of the target 'x86_64'}}
+  bar();
+}
+
+#endif
Index: clang/test/OpenMP/metadirective_device_isa_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/metadirective_device_isa_codegen.cpp
@@ -0,0 +1,81 @@
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -target-cpu gfx906 -o - | FileCheck %s -check-prefix=AMDGPUISA
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope -target-cpu x86-64| FileCheck %s -check-prefixes=X86_64ISA
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int amdgcn_device_isa_selected() {
+  int threadCount = 0;
+
+#pragma omp target map(tofrom \
+   : threadCount)
+  {
+#pragma omp metadirective \
+when(device = {isa("flat-address-space")} \
+ : parallel) default(single)
+threadCount++;
+  }
+
+  return threadCount;
+}
+
+// AMDGPUISA: define weak amdgpu_kernel void @__omp_offloading_{{.*}}amdgcn_device_isa_selected
+// AMDGPUISA: user_code.entry:
+// AMDGPUISA: call void @__kmpc_parallel_51
+// AMDGPUISA-NOT: call i32 @__kmpc_single
+// AMDGPUISA: ret void
+
+int amdgcn_device_isa_not_selected() {
+  int threadCount = 0;
+
+#pragma omp target map(tofrom \
+   : threadCount)
+  {
+#pragma omp metadirective  \
+when(device = {isa("sse")} \
+ : parallel)   \
+when(device = {isa("another-unsupported-gpu-feature")} \
+ : parallel) default(single)
+threadCount++;
+  }
+
+  return threadCount;
+}
+// AMDGPUISA: define weak amdgpu_kernel void @__omp_offloading_{{.*}}amdgcn_device_isa_not_selected
+// AMDGPUISA: user_code.entry:
+// AMDGPUISA: call i32 @__kmpc_single
+// AMDGPUISA-NOT: call void @__kmpc_parallel_51
+// AMDGPUISA: ret void
+
+void bar();
+
+void x86_64_device_isa_selected() {
+#pragma omp metadirective when(device = {isa("sse2")} \
+   : parallel) default(single)
+  bar();
+}
+// X86_64ISA-LABEL: void @_Z26x86_64_device_isa_selectedv()
+// X86_64ISA: ...) @__kmpc_fork_call{{.*}}@.omp_outlined..1
+// X86_64ISA: ret void
+
+// X86_64ISA: define internal void @.omp_outlined..1(
+// X86_64ISA: @_Z3barv
+// X86_64ISA: ret void
+
+void x86_64_device_isa_not_selected() {
+#pragma omp metadirective when(device = {isa("some-unsupported-feature")} \
+   : parallel) default(single)
+  bar();
+}
+// X86_64ISA-LABEL: void @_Z30x86_64_device_isa_not_selectedv()
+// X86_64ISA: call i32 @__kmpc_single
+// X86_64ISA:  @_Z3barv
+// X86_64ISA: call void @__kmpc_end_single
+// X86_64ISA: ret void
+#endif
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2529,7 +2529,12 @@
 TPA.Revert();
 // End of the first iteration. Parser is reset to the start of metadirective
 
-TargetOMPContext OMPCtx(ASTContext, /* DiagUnknownTrait */ nullptr,
+std::function DiagUnknownTrait =
+[this, Loc](StringRef ISATrait) {
+  Diag(Loc, diag::remark_unknown_declare_variant_isa_trait)
+  << ISATrait << this->getTargetInfo().getTriple().getArchName();
+};
+Targ

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon updated this revision to Diff 398182.
MuAlphaOmegaEpsilon added a comment.

Rebased to more recent main branch, updated comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,17 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,17 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106518

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

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

> There are already two way more sophisticated (forgive me my bias) 
> implementations in Clang that are for checking if two statements or decls are 
> the same.
>
> 1. ODRHash, used in modules to discover ODR violations
> 2. ASTStructuralEquivalenceContext, used in ASTImporter to discover if two 
> AST nodes are the same or not (as a side effect we diagnose ODR violations as 
> well).
>
> It is not the first time, when such a similarity check is needed (see 
> https://reviews.llvm.org/D75041). Of course reusing the before mentioned 
> components would require some architectural changes, but it might be 
> beneficial.

I do not quite see the overlap.  This patch addresses the structural 
equivalence of DeclRefExprs: as the `std::is_same` (or any type trait example) 
demonstrates, two declarations may be the "same" (e.g. they are both 
`std::false_type::value`), but two DeclRefExprs referring to those declarations 
should not necessarily be considered the "same": the qualifier, specifying the 
path that was taken to look them up, can matter to a user.  It's not a matter 
of the sophistication of the similarity check, it's a matter of what we mean by 
similarity.

I do not see DeclRefExprs handled in ODRHash or ASTStructuralEquivalence.  I do 
see NestedNameSpecifiers handled in both, but I don't think the implementation 
quite matches what is needed here (e.g. in ASTStructuralEquivalence check, if 
one NNS is a NamespaceAlias, the other one is assumed to be a NamespaceAlias: 
not what we want).  It's probably not worth the trouble to factor something 
common out of those; though they should certainly be used as a guide to make 
sure no cases have been missed.


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

https://reviews.llvm.org/D114622

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

[PATCH] D116822: [Clang][Sema] Use VersionMap from SDKSettings for remapping tvOS and watchOS availability

[Egor Zhdan via Phabricator via cfe-commits]

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

This makes the mapping between iOS & tvOS/watchOS versions more accurate. For 
example, iOS 9.3 now gets correctly mapped into tvOS 9.2 and not tvOS 9.3.

Before this change, the incorrect mapping could cause excessive or missing 
warnings for code that specifies availability for iOS, but not for tvOS/watchOS.

rdar://81491680


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116822

Files:
  clang/include/clang/Basic/DarwinSDKInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/Inputs/AppleTVOS15.0.sdk/SDKSettings.json
  clang/test/Sema/Inputs/WatchOS7.0.sdk/SDKSettings.json
  clang/test/Sema/attr-availability-tvos.c
  clang/test/Sema/attr-availability-watchos-infer-from-ios.c
  clang/test/Sema/attr-availability-watchos.c

Index: clang/test/Sema/attr-availability-watchos-infer-from-ios.c
===
--- clang/test/Sema/attr-availability-watchos-infer-from-ios.c
+++ clang/test/Sema/attr-availability-watchos-infer-from-ios.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 "-triple" "arm64-apple-watchos3.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "arm64-apple-watchos4.0" -DUSE_VERSION_MAP -isysroot %S/Inputs/WatchOS7.0.sdk -fsyntax-only -verify %s
 
 void f0(int) __attribute__((availability(ios,introduced=2.0,deprecated=2.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
 void f1(int) __attribute__((availability(ios,introduced=2.1)));
@@ -58,3 +59,12 @@
 void test_ios_correctly_map_to_watchos() {
   deprecatedAfterIntroduced(); // expected-warning {{'deprecatedAfterIntroduced' is deprecated: first deprecated in watchOS 3}}
 }
+
+#ifdef USE_VERSION_MAP
+// iOS 10.3.1 corresponds to watchOS 3.2, as indicated in 'SDKSettings.json'.
+void f9(int) __attribute__((availability(ios,deprecated=10.3.1))); // expected-note {{'f9' has been explicitly marked deprecated here}}
+
+void testWithVersionMap() {
+  f9(0); // expected-warning {{'f9' is deprecated: first deprecated in watchOS 3.2}}
+}
+#endif
Index: clang/test/Sema/attr-availability-tvos.c
===
--- clang/test/Sema/attr-availability-tvos.c
+++ clang/test/Sema/attr-availability-tvos.c
@@ -1,63 +1,73 @@
-// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos3.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos13.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos13.0" -DUSE_VERSION_MAP -isysroot %S/Inputs/AppleTVOS15.0.sdk -fsyntax-only -verify %s
 
-void f0(int) __attribute__((availability(tvos,introduced=2.0,deprecated=2.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
-void f1(int) __attribute__((availability(tvos,introduced=2.1)));
-void f2(int) __attribute__((availability(tvos,introduced=2.0,deprecated=3.0))); // expected-note {{'f2' has been explicitly marked deprecated here}}
-void f3(int) __attribute__((availability(tvos,introduced=3.0)));
-void f4(int) __attribute__((availability(macosx,introduced=10.1,deprecated=10.3,obsoleted=10.5), availability(tvos,introduced=2.0,deprecated=2.1,obsoleted=3.0))); // expected-note{{explicitly marked unavailable}}
+void f0(int) __attribute__((availability(tvos,introduced=12.0,deprecated=12.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
+void f1(int) __attribute__((availability(tvos,introduced=12.1)));
+void f2(int) __attribute__((availability(tvos,introduced=12.0,deprecated=13.0))); // expected-note {{'f2' has been explicitly marked deprecated here}}
+void f3(int) __attribute__((availability(tvos,introduced=13.0)));
+void f4(int) __attribute__((availability(macosx,introduced=10.1,deprecated=10.3,obsoleted=10.5), availability(tvos,introduced=12.0,deprecated=12.1,obsoleted=13.0))); // expected-note{{explicitly marked unavailable}}
 
-void f5(int) __attribute__((availability(tvos,introduced=2.0))) __attribute__((availability(tvos,deprecated=3.0))); // expected-note {{'f5' has been explicitly marked deprecated here}}
-void f6(int) __attribute__((availability(tvos,deprecated=3.0))); // expected-note {{'f6' has been explicitly marked deprecated here}}
-void f6(int) __attribute__((availability(tvos,introduced=2.0)));
+void f5(int) __attribute__((availability(tvos,introduced=12.0))) __attribute__((availability(tvos,deprecated=13.0))); // expected-note {{'f5' has been explicitly marked deprecated here}}
+void f6(int) __attribute__((availability(tvos,deprecated=13.0))); // expected-note {{'f6' has been explicitly marked deprecated here}}
+void f6(int) __attribute__((availability(tvos,introduced=12.0)));
 
 void test() {
-  f0(0); // expected-warning{{'f0' is deprecated: first deprecated

[PATCH] D116722: [clang] Verify ssp buffer size is a valid integer

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3204
   if (StackProtectorLevel) {
-CmdArgs.push_back("-stack-protector-buffer-size");
-// FIXME: Verify the argument is a valid integer.
-CmdArgs.push_back(Args.MakeArgString(Str.drop_front(16)));
+auto BufferSize = Str.drop_front(16);
+if (IsInteger(BufferSize)) {

I really am not a fan of the `16` here.  Why not just use `split` and split on 
`=`?  Or use [constexpr] `strlen`?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3209
+} else
+  D.Diag(clang::diag::err_invalid_ssp_buffer_size);
   }

Please consistently use the braces (either applied to both or on neither).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116722

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

[PATCH] D116824: [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added reviewers: njames93, aaron.ballman.
logan-5 added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
logan-5 requested review of this revision.
Herald added a subscriber: cfe-commits.

This behavior was fixed for regular identifiers in 
9f3edc323a88c1a179a0a5a9dc9a87a2964c0d48 
, but the 
same fix was not applied to macro fixits.

This addresses https://github.com/llvm/llvm-project/issues/52895.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116824

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
@@ -171,6 +171,11 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', 
which is reserved in the global namespace; cannot be fixed automatically 
[bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// https://github.com/llvm/llvm-project/issues/52895
+#define _5_kmph_rpm 459
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier 
'_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed 
automatically [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
+
 // these should pass
 #define MACRO(m) int m = 0
 
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -486,6 +486,9 @@
   NamingCheckFailure &Failure = NamingCheckFailures[ID];
   SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
 
+  if (!isValidAsciiIdentifier(Info.Fixup))
+Failure.FixStatus = ShouldFixStatus::FixInvalidIdentifier;
+
   Failure.Info = std::move(Info);
   addUsage(ID, Range);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
@@ -171,6 +171,11 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
 // CHECK-FIXES: {{^}}int _;{{$}}
 
+// https://github.com/llvm/llvm-project/issues/52895
+#define _5_kmph_rpm 459
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_5_kmph_rpm', which is reserved in the global namespace; cannot be fixed automatically [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define _5_kmph_rpm 459{{$}}
+
 // these should pass
 #define MACRO(m) int m = 0
 
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -486,6 +486,9 @@
   NamingCheckFailure &Failure = NamingCheckFailures[ID];
   SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
 
+  if (!isValidAsciiIdentifier(Info.Fixup))
+Failure.FixStatus = ShouldFixStatus::FixInvalidIdentifier;
+
   Failure.Info = std::move(Info);
   addUsage(ID, Range);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I'm not aware of any of those places causing an actual problem though? The AST 
isn't a stable interface, and __builtin_dump_struct is for debugging purposes 
only. I would expect debug info consumers to be able to handle __va_list in the 
global namespace as this is the status quo for C.

So I'm somewhat inclined to do the simple thing here first, and then look at 
making things more conditional if a problem comes up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116774

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

[PATCH] D112718: Add intrinsics and builtins for PTX atomics with semantic orders

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsNVPTX.def:1057
+
+BUILTIN(__nvvm_atom_xchg_global_i, "iiD*i", "n")
+TARGET_BUILTIN(__nvvm_atom_cta_xchg_global_i, "iiD*i", "n", SM_60)

t4c1 wrote:
> tra wrote:
> > We need to figure out how address-space-specific builtins are supposed to 
> > work.
> > Right now two competing approaches.
> > 
> > This patch declares builtins with generic pointer as an argument, but, 
> > according to the test, expects to be used with the AS-specific pointer.
> > It probably does not catch a wrong-AS pointer passed as an argument, either.
> > It does happen to work, but I think it's mostly due to the fact that LLVM 
> > intrinsics are overloaded and we happen to end up addrspacecast'ing  things 
> > correctly if the builtin gets the right kind of pointer.
> > 
> > The other approach is to declare the pointer with the expected AS. E.g:
> > > TARGET_BUILTIN(__nvvm_mbarrier_init_shared, "vWi*3i", "", 
> > > AND(SM_80,PTX70))
> > 
> > IMO, this is the correct way to do it, but it is also rather inconvenient 
> > to use from CUDA code as it operates on generic pointers.
> > 
> > @jdoerfert - WDYT?
> >TARGET_BUILTIN(__nvvm_mbarrier_init_shared, "vWi*3i", "", AND(SM_80,PTX70))
> >IMO, this is the correct way to do it, but it is also rather inconvenient to 
> >use from CUDA code as it operates on generic pointers.
> 
> I tried doing this, however it is also completely unusable from OpenCL code 
> (which is our use case). Trying to use it gives you errors about how casting 
> pointers to different address spaces  - for example from local to AS3 is not 
> allowed.
Hmm. It should've worked. It would need the same explicit cast to a pointer in 
AS(3) as in your tests, but it would safeguard against attempts to pass it a 
generic pointer. E.g. https://godbolt.org/z/qE6oxzheM

Explicit casting to AS(X) for AS-specific variants is annoying, but it's 
probably unavoidable at the moment regardless of whether we declare the pointer 
argument to be AS-specific or not.  LLVM will not always be able to infer that 
a pointer is in particular AS.
Using specific AS in the declaration has a minor benefit of safeguarding at 
compile time against unintentional use of generic pointers.

Ideally we may want to convert generic variant of the builtin to AS-specific 
one, if LLVM does know the AS. We currently do this for loads/stores, but not 
for other instructions.




Comment at: clang/test/CodeGen/builtins-nvptx.c:557
+  // expected-error@+1 {{'__nvvm_atom_acquire_add_global_i' needs target 
feature sm_70}}
+  __nvvm_atom_acquire_add_global_i((__attribute__((address_space(1))) int 
*)ip, i);
+

t4c1 wrote:
> tra wrote:
> > What happens if I pass a wrong pointer kind? E.g. a generic or shared 
> > pointer?
> It will silently accept it. I can look into how to output appropriate error 
> message.
I guest the bast we can do here is to safeguard against unintentional use of 
generic pointer.
There's not much we can do if someone explicitly casts a pointer to a wrong AS.

I think declaring pointer arg to be in specific AS would be sufficient. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112718

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

[PATCH] D116822: [Clang][Sema] Use VersionMap from SDKSettings for remapping tvOS and watchOS availability

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan updated this revision to Diff 398192.
egorzhdan added a comment.

Remove accidental change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116822

Files:
  clang/include/clang/Basic/DarwinSDKInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/Inputs/AppleTVOS15.0.sdk/SDKSettings.json
  clang/test/Sema/Inputs/WatchOS7.0.sdk/SDKSettings.json
  clang/test/Sema/attr-availability-tvos.c
  clang/test/Sema/attr-availability-watchos.c

Index: clang/test/Sema/attr-availability-watchos.c
===
--- clang/test/Sema/attr-availability-watchos.c
+++ clang/test/Sema/attr-availability-watchos.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 "-triple" "arm64-apple-watchos3.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "arm64-apple-watchos4.0" -DUSE_VERSION_MAP -isysroot %S/Inputs/WatchOS7.0.sdk -fsyntax-only -verify %s
 
 void f0(int) __attribute__((availability(ios,introduced=2.0,deprecated=2.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
 void f1(int) __attribute__((availability(ios,introduced=2.1)));
@@ -58,3 +59,12 @@
 void test_ios_correctly_map_to_watchos() {
   deprecatedAfterIntroduced(); // expected-warning {{'deprecatedAfterIntroduced' is deprecated: first deprecated in watchOS 3}}
 }
+
+#ifdef USE_VERSION_MAP
+// iOS 10.3.1 corresponds to watchOS 3.2, as indicated in 'SDKSettings.json'.
+void f9(int) __attribute__((availability(ios,deprecated=10.3.1))); // expected-note {{'f9' has been explicitly marked deprecated here}}
+
+void testWithVersionMap() {
+  f9(0); // expected-warning {{'f9' is deprecated: first deprecated in watchOS 3.2}}
+}
+#endif
Index: clang/test/Sema/attr-availability-tvos.c
===
--- clang/test/Sema/attr-availability-tvos.c
+++ clang/test/Sema/attr-availability-tvos.c
@@ -1,63 +1,73 @@
-// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos3.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos13.0" -fsyntax-only -verify %s
+// RUN: %clang_cc1 "-triple" "x86_64-apple-tvos13.0" -DUSE_VERSION_MAP -isysroot %S/Inputs/AppleTVOS15.0.sdk -fsyntax-only -verify %s
 
-void f0(int) __attribute__((availability(tvos,introduced=2.0,deprecated=2.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
-void f1(int) __attribute__((availability(tvos,introduced=2.1)));
-void f2(int) __attribute__((availability(tvos,introduced=2.0,deprecated=3.0))); // expected-note {{'f2' has been explicitly marked deprecated here}}
-void f3(int) __attribute__((availability(tvos,introduced=3.0)));
-void f4(int) __attribute__((availability(macosx,introduced=10.1,deprecated=10.3,obsoleted=10.5), availability(tvos,introduced=2.0,deprecated=2.1,obsoleted=3.0))); // expected-note{{explicitly marked unavailable}}
+void f0(int) __attribute__((availability(tvos,introduced=12.0,deprecated=12.1))); // expected-note {{'f0' has been explicitly marked deprecated here}}
+void f1(int) __attribute__((availability(tvos,introduced=12.1)));
+void f2(int) __attribute__((availability(tvos,introduced=12.0,deprecated=13.0))); // expected-note {{'f2' has been explicitly marked deprecated here}}
+void f3(int) __attribute__((availability(tvos,introduced=13.0)));
+void f4(int) __attribute__((availability(macosx,introduced=10.1,deprecated=10.3,obsoleted=10.5), availability(tvos,introduced=12.0,deprecated=12.1,obsoleted=13.0))); // expected-note{{explicitly marked unavailable}}
 
-void f5(int) __attribute__((availability(tvos,introduced=2.0))) __attribute__((availability(tvos,deprecated=3.0))); // expected-note {{'f5' has been explicitly marked deprecated here}}
-void f6(int) __attribute__((availability(tvos,deprecated=3.0))); // expected-note {{'f6' has been explicitly marked deprecated here}}
-void f6(int) __attribute__((availability(tvos,introduced=2.0)));
+void f5(int) __attribute__((availability(tvos,introduced=12.0))) __attribute__((availability(tvos,deprecated=13.0))); // expected-note {{'f5' has been explicitly marked deprecated here}}
+void f6(int) __attribute__((availability(tvos,deprecated=13.0))); // expected-note {{'f6' has been explicitly marked deprecated here}}
+void f6(int) __attribute__((availability(tvos,introduced=12.0)));
 
 void test() {
-  f0(0); // expected-warning{{'f0' is deprecated: first deprecated in tvOS 2.1}}
+  f0(0); // expected-warning{{'f0' is deprecated: first deprecated in tvOS 12.1}}
   f1(0);
-  f2(0); // expected-warning{{'f2' is deprecated: first deprecated in tvOS 3.0}}
+  f2(0); // expected-warning{{'f2' is deprecated: first deprecated in tvOS 13.0}}
   f3(0);
-  f4(0); // expected-error{{f4' is unavailable: obsoleted in tvOS 3.0}}
-  f5(0); // expected-warning{{'f5' is deprecated: first deprecated in tvOS 3.0}}
-  f6(0); // expected-warning{{'f6' is deprecated: first d

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Masoud Ataei via Phabricator via cfe-commits]

masoud.ataei added inline comments.



Comment at: llvm/include/llvm/Analysis/ScalarFuncs.def:19
+TLI_DEFINE_SCALAR_MASS_FUNC("acosf", "__xl_acosf")
+TLI_DEFINE_SCALAR_MASS_FUNC("__acosf_finite", "__xl_acosf")
+TLI_DEFINE_SCALAR_MASS_FUNC("acos", "__xl_acos")

efriedma wrote:
> Do "__acosf_finite" etc. actually exist on AIX?  I thought they only existed 
> on glibc, and the glibc functions are all deprecated.
> 
> I think I'd prefer to track this information in TargetLibraryInfo, like we do 
> for the vector functions, so we can more easily generalize this mechanism in 
> the future.
Some machines still have the old glibc, so I kept them for compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

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

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 398195.
serge-sans-paille added a comment.

rebased


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

https://reviews.llvm.org/D112913

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp

Index: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - misc-misleading-bidirectional
+
+misc-misleading-bidirectional
+=
+
+Warn about unterminated bidirectional unicode sequence, detecting potential attack
+as described in the `Trojan Source `_ attack.
+
+Example:
+
+.. code-block:: c++
+
+#include 
+
+int main() {
+bool isAdmin = false;
+/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
+std::cout << "You are an admin.\n";
+/* end admins only ‮ { ⁦*/
+return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,7 +212,8 @@
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
-   `misc-misleading-identifier `_,
+   `misc-misleading-bidirectional `_,
+   `misc-misleading-identifier `_,
`misc-misplaced-const `_,
`misc-new-delete-overloads `_,
`misc-no-recursion `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,6 +123,10 @@
   Reports identifiers whose names are too short. Currently checks local
   variables and function parameters only.
 
+- New :doc:`misc-misleading-bidirectional ` check.
+
+  Inspects string literal and comments for unterminated bidirectional Unicode
+  characters.
 
 New check aliases
 ^
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
@@ -0,0 +1,38 @@
+//===--- MisleadingBidirectionalCheck.h - clang-tidy *- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class MisleadingBidirectionalCheck : public ClangTidyCheck {
+public:
+  MisleadingBidirectionalCheck(StringRef Name, ClangTidyContext *Context);
+  ~MisleadingBidirectionalCheck();
+
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  class MisleadingBidirectionalHandler;
+  std::unique_ptr Handler;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -0,0 +1,142 @@
+//===--- MisleadingBidirectional.cpp - clang-tidy -===//
+//
+// 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
+//
+//===--===//
+
+#include "MisleadingBidirectional.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/ConvertUTF.h"
+
+using nam

[PATCH] D116827: Don't pass uninitialized QueryKind

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
vitalybuka added reviewers: kda, eugenis.
Herald added subscribers: usaxena95, kadircet, arphaman.
vitalybuka requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Even if findImplementors does not use
uninitialized parameter it's still UB and
it's going to be detected by msan with:
-Xclang -enable-noundef-analysis -mllvm -msan-eager-checks=1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116827

Files:
  clang-tools-extra/clangd/XRefs.cpp


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1310,6 +1310,8 @@
   QueryKind = RelationKind::BaseOf;
 }
   }
+  if (IDs.empty())
+return {};
   return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1310,6 +1310,8 @@
   QueryKind = RelationKind::BaseOf;
 }
   }
+  if (IDs.empty())
+return {};
   return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116828: Initialize HighlightingsBuilder::Resolver

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
vitalybuka added reviewers: kda, eugenis.
Herald added subscribers: usaxena95, kadircet, arphaman.
vitalybuka requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Detected by msan with:
-Xclang -enable-noundef-analysis -mllvm -msan-eager-checks=1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116828

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -468,7 +468,7 @@
   const LangOptions &LangOpts;
   std::vector Tokens;
   std::map> ExtraModifiers;
-  const HeuristicResolver *Resolver;
+  const HeuristicResolver *Resolver = nullptr;
   // returned from addToken(InvalidLoc)
   HighlightingToken InvalidHighlightingToken;
 };


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -468,7 +468,7 @@
   const LangOptions &LangOpts;
   std::vector Tokens;
   std::map> ExtraModifiers;
-  const HeuristicResolver *Resolver;
+  const HeuristicResolver *Resolver = nullptr;
   // returned from addToken(InvalidLoc)
   HighlightingToken InvalidHighlightingToken;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116833: [clang][Sema] Disable -Wc++20-designator in system macros

[Carlos Galvez via Phabricator via cfe-commits]

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

To be consistent with GCC.

Fixes #52944.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116833

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-sysheader-macro.cpp


Index: clang/test/SemaCXX/warn-sysheader-macro.cpp
===
--- clang/test/SemaCXX/warn-sysheader-macro.cpp
+++ clang/test/SemaCXX/warn-sysheader-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast 
-Wc++20-designator %s
 
 // Test that macro expansions from system headers don't trigger 'syntactic'
 // warnings that are not actionable.
@@ -12,6 +12,11 @@
 
 #define OLD_STYLE_CAST(a) ((int) (a))
 
+struct Foo {
+  int x;
+};
+#define DESIGNATED_INITIALIZERS (Foo{.x = 123})
+
 #else
 
 #define IS_SYSHEADER
@@ -32,4 +37,9 @@
   int i = OLD_STYLE_CAST(0);
 }
 
+void PR52944() {
+  // no -Wc++20-designator in system macro expansion
+  auto i = DESIGNATED_INITIALIZERS;
+}
+
 #endif
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7157,7 +7157,8 @@
 // Only diagnose designated initiaization as a C++20 extension if we didn't
 // already diagnose use of (non-C++20) C99 designator syntax.
 if (getLangOpts().CPlusPlus && !DiagnosedArrayDesignator &&
-!DiagnosedNestedDesignator && !DiagnosedMixedDesignator) {
+!DiagnosedNestedDesignator && !DiagnosedMixedDesignator &&
+!getSourceManager().isInSystemMacro(FirstDesignator)) {
   Diag(FirstDesignator, getLangOpts().CPlusPlus20
 ? diag::warn_cxx17_compat_designated_init
 : diag::ext_cxx_designated_init);


Index: clang/test/SemaCXX/warn-sysheader-macro.cpp
===
--- clang/test/SemaCXX/warn-sysheader-macro.cpp
+++ clang/test/SemaCXX/warn-sysheader-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast -Wc++20-designator %s
 
 // Test that macro expansions from system headers don't trigger 'syntactic'
 // warnings that are not actionable.
@@ -12,6 +12,11 @@
 
 #define OLD_STYLE_CAST(a) ((int) (a))
 
+struct Foo {
+  int x;
+};
+#define DESIGNATED_INITIALIZERS (Foo{.x = 123})
+
 #else
 
 #define IS_SYSHEADER
@@ -32,4 +37,9 @@
   int i = OLD_STYLE_CAST(0);
 }
 
+void PR52944() {
+  // no -Wc++20-designator in system macro expansion
+  auto i = DESIGNATED_INITIALIZERS;
+}
+
 #endif
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7157,7 +7157,8 @@
 // Only diagnose designated initiaization as a C++20 extension if we didn't
 // already diagnose use of (non-C++20) C99 designator syntax.
 if (getLangOpts().CPlusPlus && !DiagnosedArrayDesignator &&
-!DiagnosedNestedDesignator && !DiagnosedMixedDesignator) {
+!DiagnosedNestedDesignator && !DiagnosedMixedDesignator &&
+!getSourceManager().isInSystemMacro(FirstDesignator)) {
   Diag(FirstDesignator, getLangOpts().CPlusPlus20
 ? diag::warn_cxx17_compat_designated_init
 : diag::ext_cxx_designated_init);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits