[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

How does this handle cases where a macro is




Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:106
+
+bool hasBlankLines(const std::string &Text) {
+  enum class WhiteSpaceState {





Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:116
+  for (char c : Text) {
+if (c == '\r') {
+  if (State == WhiteSpaceState::CR)

Make this a switch?



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:134
+
+bool MacroToEnumCallbacks::isConsecutiveMacro(const MacroDirective *MD) const {
+  if (LastMacroLocation.isInvalid())

Is there any practical reason for these to be defined outline?



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:147-148
+  Lexer::makeFileCharRange(BetweenMacros, SM, LangOptions);
+  std::string BetweenText =
+  Lexer::getSourceText(CharRange, SM, LangOptions).str();
+  return !hasBlankLines(BetweenText);

No need to create a `std::string` here, just keep it as a StringRef.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:182
+  MD->getMacroInfo()->isUsedForHeaderGuard() ||
+  MD->getMacroInfo()->isBuiltinMacro() || ConditionScope > 0)
+return;

This `ConditionScope` checks looks like it would prevent warning in header 
files that use a header guard(instead of pragma once) to prevent multiple 
inclusion.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:192-194
+  if (LastFile != CurrentFile) {
+LastFile = CurrentFile;
+newEnum();

This seems a strange way to decect changes in file when you can just override 
the FileChanged callback.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:212-218
+  Enums.erase(std::remove_if(Enums.begin(), Enums.end(),
+ [MatchesToken](const MacroList &MacroList) {
+   return std::find_if(
+  MacroList.begin(), MacroList.end(),
+  MatchesToken) != MacroList.end();
+ }),
+  Enums.end());

Probably a syntax error there, but something like that would be much more 
readable.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h:13
+#include "../ClangTidyCheck.h"
+#include 
+

IWYU Violation



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h:19
+
+/// FIXME: Write a short description.
+///

FIXME



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp:7-15
+// CHECK-MESSAGES: :[[@LINE-4]]:1: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-5]]:12: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-6]]:21: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-6]]:1: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-7]]:14: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-8]]:23: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: :[[@LINE-8]]:1: note: FIX-IT applied suggested code changes

These checks aren't needed as notes are implicitly ignored by the 
check_clang_tidy script.
Same goes for below



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp:37-40
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin 
*/
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous 
point */
+// CHECK-FIXES-NEXT: };

Not essential for this to go in, but it would be nice if we could detect the 
longest common prefix for all items in a group and potentially use that to name 
the enum. This would only be valid if the generated name is not currently a 
recognised token,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117522

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

[PATCH] D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext

[Nathan James via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/IncludeInserter.h:20
 namespace tidy {
-namespace utils {
 

LegalizeAdulthood wrote:
> njames93 wrote:
> > LegalizeAdulthood wrote:
> > > What's the guidance on what belongs in `clang::tidy`
> > > and what belongs in `clang::tidy::utils`?
> > Well as the file moved out of utils, its no longer in the utils namespace.
> Yes, if you move the file out of utils then it shouldn't be in the
> utils namespace.  But why move the file in the first place?
> 
> IOW, what is the guideline for what files belong under `utils`
> and what files belong under `clang-tidy`?
Utils is for utilities that checks can make use of, but aren't required for the 
base clang-tidy infra to work.
By having the Inserter as a dependancy for ClangTidyDiagnosticConsumer it is 
now required for clang-tidy infrastructure, hence it was moved out of utils.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117409

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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h:19
+
+/// FIXME: Write a short description.
+///

njames93 wrote:
> FIXME
Please do this :-)



Comment at: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp:63
 CheckFactories.registerCheck("modernize-loop-convert");
+CheckFactories.registerCheck(
+"modernize-macro-to-enum");

Please address this.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:6
+
+This check performs basic analysis of macros and replaces them
+with an anonymous unscoped enum.  Using an unscoped anonymous enum

Please make first statement same as in Release Notes. `This check` should be 
omitted.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:45
+  enum {
+  RED = 0xFF,
+  GREEN = 0x00FF00,

It'll be reasonable to support indentation. Option could be used to specify 
string (tab or several spaces).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117522

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

[PATCH] D117529: [clangd][NFC] Cache ClangTidy check globs to speed up createChecks

[Nathan James via Phabricator via cfe-commits]

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

Build a fast factory for creating clang-tidy check lists based off a glob 
string.

Compile Globs into a BitVector representation that can be used to build a 
vector of checks without having to consult the glob string. Cache these 
representations as most invocations will be using the same glob string.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117529

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


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -45,6 +46,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -282,6 +284,59 @@
   }
 }
 
+// Turns a clang-tidy glob list into a BitVector that can be used to build the
+// necessary checks efficiently.
+class CachedFactories {
+  std::vector(
+llvm::StringRef, tidy::ClangTidyContext *)>>>
+  AllChecks;
+  std::mutex Guard;
+  llvm::StringMap CachedGlobs;
+
+  llvm::BitVector &getCachedValue(StringRef CheckGlobString) {
+std::lock_guard LG(Guard);
+auto It = CachedGlobs.find(CheckGlobString);
+if (It != CachedGlobs.end())
+  return It->getValue();
+// On the first time we see a glob, we construct the representation and
+// cache it for the next time. In most projects there would only ever be 
one
+// unique Glob.
+tidy::GlobList Glob(CheckGlobString);
+llvm::BitVector IsEnabled(AllChecks.size(), false);
+for (size_t I = 0, E = AllChecks.size(); I != E; ++I) {
+  if (Glob.contains(AllChecks[I].first))
+IsEnabled.set(I);
+}
+return CachedGlobs.insert_or_assign(CheckGlobString, std::move(IsEnabled))
+.first->getValue();
+  }
+
+public:
+  CachedFactories() {
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+for (const auto &Item : CTFactories) {
+  AllChecks.emplace_back(Item.getKey(), Item.getValue());
+}
+  }
+
+  std::vector>
+  createChecks(tidy::ClangTidyContext *Context) {
+if (!Context->getOptions().Checks)
+  return {};
+llvm::BitVector &IsEnabled = getCachedValue(*Context->getOptions().Checks);
+std::vector> Result;
+Result.reserve(IsEnabled.size());
+for (unsigned CheckID : IsEnabled.set_bits()) {
+  Result.push_back(
+  AllChecks[CheckID].second(AllChecks[CheckID].first, Context));
+}
+return Result;
+  }
+};
+
 } // namespace
 
 llvm::Optional
@@ -410,15 +465,13 @@
   // diagnostics.
   if (PreserveDiags) {
 trace::Span Tracer("ClangTidyInit");
-tidy::ClangTidyCheckFactories CTFactories;
-for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
-  E.instantiate()->addCheckFactories(CTFactories);
+static CachedFactories Factory;
 CTContext.emplace(std::make_unique(
 tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
 CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
 CTContext->setASTContext(&Clang->getASTContext());
 CTContext->setCurrentFile(Filename);
-CTChecks = CTFactories.createChecks(CTContext.getPointer());
+CTChecks = Factory.createChecks(CTContext.getPointer());
 llvm::erase_if(CTChecks, [&](const auto &Check) {
   return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
 });


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -45,6 +46,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -282,6 +284,59 @@
   }
 }
 
+// Turns a clang-tidy g

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:45
+  enum {
+  RED = 0xFF,
+  GREEN = 0x00FF00,

Eugene.Zelenko wrote:
> It'll be reasonable to support indentation. Option could be used to specify 
> string (tab or several spaces).
That's not actually necessary as clang-tidy runs clang-format on the fixes it 
generates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117522

___
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 inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+const auto &NextLine = *I[1];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)

HazardyKnusperkeks wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > owenpan wrote:
> > > > I would move this line to just before handling empty record blocks 
> > > > below.
> > > I'd rather keep the definitions close together.
> > If it were just a simple initialization, it wouldn't matter much. However, 
> > it would be a bit wasteful as `PreviousLine` always gets computed here even 
> > if the function may return before the pointer would get chance to be used. 
> > If you really want to keep all related definitions together, wouldn't you 
> > want to move lines 214-215 up to right after line 211?
> In a release build I'm betting that the compiler is smart enough to never 
> calculate `PreviousLine` and that performance doesn't really matter was shown 
> in D116318.
> 
> But yeah moving it up to `TheLine` is consistent and will do.
> In a release build I'm betting that the compiler is smart enough to never 
> calculate `PreviousLine`

No compiler will (or should) skip generating code that calculates 
`PreviousLine`, which may or may not get used at runtime. I'm not aware of any 
compiler that is smart enough to move the initialization code to just before 
where the variable is first used.

To illustrate the difference, I compiled the code below with `clang++ -Wall 
-std=c++11 -O3 -S foo.cpp`:
```
#include 

using namespace std;
using Type = vector;

int f(const Type &V, Type::const_iterator I) {
  const auto *P = I != V.begin() ? I[-1] : nullptr;
  if (I == V.end())
return 0;
  return *P;
}
```
The generated code in foo.s includes the following:
```
.cfi_startproc
; %bb.0:
ldr x8, [x0]
cmp x8, x1
b.eqLBB0_3
; %bb.1:
ldurx8, [x1, #-8]
ldr x9, [x0, #8]
cmp x9, x1
b.eqLBB0_4
LBB0_2:
ldr w0, [x8]
ret
LBB0_3:
mov x8, #0
ldr x9, [x0, #8]
cmp x9, x1
b.neLBB0_2
LBB0_4:
mov w0, #0
ret
.cfi_endproc
```
As you can see, the initialization code was neither optimized away nor moved.

When I changed the function body to:
```
  if (I == V.end())
return 0;
  const auto *P = I != V.begin() ? I[-1] : nullptr;
  return *P;
```
The generated code became much simpler:
```
.cfi_startproc
; %bb.0:
ldr x8, [x0, #8]
cmp x8, x1
b.eqLBB0_2
; %bb.1:
ldurx8, [x1, #-8]
ldr w0, [x8]
ret
LBB0_2:
mov w0, #0
ret
.cfi_endproc
```


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 inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+const auto &NextLine = *I[1];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)

owenpan wrote:
> HazardyKnusperkeks wrote:
> > owenpan wrote:
> > > HazardyKnusperkeks wrote:
> > > > owenpan wrote:
> > > > > I would move this line to just before handling empty record blocks 
> > > > > below.
> > > > I'd rather keep the definitions close together.
> > > If it were just a simple initialization, it wouldn't matter much. 
> > > However, it would be a bit wasteful as `PreviousLine` always gets 
> > > computed here even if the function may return before the pointer would 
> > > get chance to be used. If you really want to keep all related definitions 
> > > together, wouldn't you want to move lines 214-215 up to right after line 
> > > 211?
> > In a release build I'm betting that the compiler is smart enough to never 
> > calculate `PreviousLine` and that performance doesn't really matter was 
> > shown in D116318.
> > 
> > But yeah moving it up to `TheLine` is consistent and will do.
> > In a release build I'm betting that the compiler is smart enough to never 
> > calculate `PreviousLine`
> 
> No compiler will (or should) skip generating code that calculates 
> `PreviousLine`, which may or may not get used at runtime. I'm not aware of 
> any compiler that is smart enough to move the initialization code to just 
> before where the variable is first used.
> 
> To illustrate the difference, I compiled the code below with `clang++ -Wall 
> -std=c++11 -O3 -S foo.cpp`:
> ```
> #include 
> 
> using namespace std;
> using Type = vector;
> 
> int f(const Type &V, Type::const_iterator I) {
>   const auto *P = I != V.begin() ? I[-1] : nullptr;
>   if (I == V.end())
> return 0;
>   return *P;
> }
> ```
> The generated code in foo.s includes the following:
> ```
>   .cfi_startproc
> ; %bb.0:
>   ldr x8, [x0]
>   cmp x8, x1
>   b.eqLBB0_3
> ; %bb.1:
>   ldurx8, [x1, #-8]
>   ldr x9, [x0, #8]
>   cmp x9, x1
>   b.eqLBB0_4
> LBB0_2:
>   ldr w0, [x8]
>   ret
> LBB0_3:
>   mov x8, #0
>   ldr x9, [x0, #8]
>   cmp x9, x1
>   b.neLBB0_2
> LBB0_4:
>   mov w0, #0
>   ret
>   .cfi_endproc
> ```
> As you can see, the initialization code was neither optimized away nor moved.
> 
> When I changed the function body to:
> ```
>   if (I == V.end())
> return 0;
>   const auto *P = I != V.begin() ? I[-1] : nullptr;
>   return *P;
> ```
> The generated code became much simpler:
> ```
>   .cfi_startproc
> ; %bb.0:
>   ldr x8, [x0, #8]
>   cmp x8, x1
>   b.eqLBB0_2
> ; %bb.1:
>   ldurx8, [x1, #-8]
>   ldr w0, [x8]
>   ret
> LBB0_2:
>   mov w0, #0
>   ret
>   .cfi_endproc
> ```
> and that performance doesn't really matter was shown in D116318.

It wasn't quite the same there as we must pay either in the caller or in the 
callee. That being said, I agree that the performance hit here would be 
negligible.


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] D70401: [RISCV] Complete RV32E/ilp32e implementation

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.
Herald added subscribers: alextsao1999, eopXD.

Gentle ping.

We are testing this patch and I'd like to get some nice advice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400696.
glotchimo marked an inline comment as done.
glotchimo added a comment.

Simplify by removing look-ahead (unnecessary b/c of difference between ident 
and tokens).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D70401: [RISCV] Complete RV32E/ilp32e implementation

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

1. please add a check here 

 and a clang cc1 test for it.
2. Have you try to run llvm-test-suite with rv32e config on qemu?




Comment at: llvm/lib/Support/TargetParser.cpp:339
+if (ISAInfo.hasExtension("d"))
+  return "ilp32d";
 return "ilp32";

why do we need to change the order?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood marked 13 inline comments as done.
LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:134
+
+bool MacroToEnumCallbacks::isConsecutiveMacro(const MacroDirective *MD) const {
+  if (LastMacroLocation.isInvalid())

njames93 wrote:
> Is there any practical reason for these to be defined outline?
Is there any practical reason for it to be inline?

I don't like making large inline functions.  It's my understanding that
the compiler may inline small "outlined" functions anyway, where it
has a better idea of what "small" means.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:182
+  MD->getMacroInfo()->isUsedForHeaderGuard() ||
+  MD->getMacroInfo()->isBuiltinMacro() || ConditionScope > 0)
+return;

njames93 wrote:
> This `ConditionScope` checks looks like it would prevent warning in header 
> files that use a header guard(instead of pragma once) to prevent multiple 
> inclusion.
Oh, good catch, you're probably right.  I'll test that manually.

(Gee, another case where we need `check_clang_tidy.py` to validate
changes to header files!  This keeps coming up!  My implementation
of this from several years ago died in review hell and was never born.)



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:192-194
+  if (LastFile != CurrentFile) {
+LastFile = CurrentFile;
+newEnum();

njames93 wrote:
> This seems a strange way to decect changes in file when you can just override 
> the FileChanged callback.
I'll try that.  Does it fire once at the beginning?



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:212-218
+  Enums.erase(std::remove_if(Enums.begin(), Enums.end(),
+ [MatchesToken](const MacroList &MacroList) {
+   return std::find_if(
+  MacroList.begin(), MacroList.end(),
+  MatchesToken) != MacroList.end();
+ }),
+  Enums.end());

njames93 wrote:
> Probably a syntax error there, but something like that would be much more 
> readable.
I was unaware of `llvm::erase`, presumably it's a range-like algo that
does the boiler plate of `begin()`, `end()`, `std::remove_if` and
`std::vector::erase`?

Looks like I need to switch to `llvm::SmallVector` for that to work.
I'll try that.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h:13
+#include "../ClangTidyCheck.h"
+#include 
+

njames93 wrote:
> IWYU Violation
Good catch.  This is a leftover when I had more methods declared
on the `Check` class.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h:19
+
+/// FIXME: Write a short description.
+///

Eugene.Zelenko wrote:
> njames93 wrote:
> > FIXME
> Please do this :-)
Oops `:)`



Comment at: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp:63
 CheckFactories.registerCheck("modernize-loop-convert");
+CheckFactories.registerCheck(
+"modernize-macro-to-enum");

Eugene.Zelenko wrote:
> Please address this.
Yep, I didn't `clang-format` this file after `add_new_check` ran.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:45
+  enum {
+  RED = 0xFF,
+  GREEN = 0x00FF00,

njames93 wrote:
> Eugene.Zelenko wrote:
> > It'll be reasonable to support indentation. Option could be used to specify 
> > string (tab or several spaces).
> That's not actually necessary as clang-tidy runs clang-format on the fixes it 
> generates
It can optionally do this, but it is off by default.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp:37-40
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin 
*/
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous 
point */
+// CHECK-FIXES-NEXT: };

njames93 wrote:
> Not essential for this to go in, but it would be nice if we could detect the 
> longest common prefix for all items in a group and potentially use that to 
> name the enum. This would only be valid if the generated name is not 
> currently a recognised token,
Yeah, for this example that would yield a reasonable enum
name of `CoordMode`, but many macros have acronym prefixes
and they wouldn't yield useful names, e.g. `WM_COMMAND`
would get a name like `WM` which isn't particularly useful and
may cause other problems.

IMO, introducing names should be a step that's invoked
manually by the developer.

I intend to write another check that migrates a named, unscoped
enum to a scoped enum.  Once everything h

[PATCH] D117398: [clang-format] Fix bug in parsing `operator<` with template

[Jino Park via Phabricator via cfe-commits]

pjessesco updated this revision to Diff 400697.
pjessesco marked 2 inline comments as done.
pjessesco added a comment.

Fix to accept feedbacks.

1. `Forth` -> `Fourth`
2. Add unit test `verifyFormat("< <>");`
3. Fix to avoid undefined behavior


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

https://reviews.llvm.org/D117398

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,9 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  verifyFormat("< <>");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,9 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
+Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,11 +429,18 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto First = Tokens.end() - 3;
   bool FourthTokenIsLess = false;
-  if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
 
-  auto First = Tokens.end() - 3;
+  if (Tokens.size() > 3) {
+auto Fourth = (Tokens.end() - 4)[0];
+FourthTokenIsLess = Fourth->is(tok::less);
+
+// Do not remove a whitespace between the two "<" e.g. "operator< <>".
+if (First[2]->is(tok::greater) && Fourth->is(tok::kw_operator))
+  return false;
+  }
+
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
   First[0]->isNot(tok::less) || FourthTokenIsLess)
 return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,9 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  verifyFormat("< <>");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,9 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
+Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,11 +429,18 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto First = Tokens.end() - 3;
   bool FourthTokenIsLess = false;
-  if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
 
-  auto First = Tokens.end() - 3;
+  if (Tokens.size() > 3) {
+auto Fourth = (Tokens.end() - 4)[0];
+FourthTokenIsLess = Fourth->is(tok::less);
+
+// Do not remove a whitespace between the two "<" e.g. "operator< <>".
+if (First[2]->is(tok::greater) && Fourth->is(tok::kw_operator))
+  return false;
+  }
+
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
   First[0]->isNot(tok::less) || FourthTokenIsLess)
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:192-194
+  if (LastFile != CurrentFile) {
+LastFile = CurrentFile;
+newEnum();

LegalizeAdulthood wrote:
> njames93 wrote:
> > This seems a strange way to decect changes in file when you can just 
> > override the FileChanged callback.
> I'll try that.  Does it fire once at the beginning?
Tests continue to pass after switching to `FileChanged`, so looks good.
I'll double check in the debugger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117522

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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 400700.
LegalizeAdulthood added a comment.

Addresses most of the review comments, still need to:

- Verify that include guards don't interfere with analysis of headers


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' is disguised as an enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' is disguised as an enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' is disguised as an enum
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' is disguised as an enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' is disguised as an enum
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' is disguised as an enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' is disguised as an enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' is disguised as an enum
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: BadAlloc =11  /* insufficient resources */
+// CHECK-FIXES-NEXT: };
+
+// Undefining a macro invalidates adjacent macros
+// from being considered as an enum.
+#define REMOVED1 1
+#define REMOVED2 2
+#define REMOVED3 3
+#undef REMOVED2
+#define VALID1 1
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: Macro 'VALID1' is disguised as an enum
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: VALID1 = 1
+// CHECK-FIXES-NEXT: };
+
+// Regular conditional compilation blocks should leave previous
+// macro enums alone.
+#if 0
+#include 
+#endif
+
+// Conditional compilation blocks invalidate adjacent macros
+// from being considered as an enum.  Conditionally compiled
+// blocks coul

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:182
+  MD->getMacroInfo()->isUsedForHeaderGuard() ||
+  MD->getMacroInfo()->isBuiltinMacro() || ConditionScope > 0)
+return;

LegalizeAdulthood wrote:
> njames93 wrote:
> > This `ConditionScope` checks looks like it would prevent warning in header 
> > files that use a header guard(instead of pragma once) to prevent multiple 
> > inclusion.
> Oh, good catch, you're probably right.  I'll test that manually.
> 
> (Gee, another case where we need `check_clang_tidy.py` to validate
> changes to header files!  This keeps coming up!  My implementation
> of this from several years ago died in review hell and was never born.)
Any ideas for an algorithm that detects a header guard condition
from some other condition?  I don't see anything obvious.


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

https://reviews.llvm.org/D117522

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400704.
psigillito added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Removed old approach and started initial changes to add C language


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400706.
psigillito added a comment.

revert changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400708.
psigillito added a comment.

Hopefully this revision works


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language

[PATCH] D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/IncludeInserter.h:20
 namespace tidy {
-namespace utils {
 

njames93 wrote:
> LegalizeAdulthood wrote:
> > njames93 wrote:
> > > LegalizeAdulthood wrote:
> > > > What's the guidance on what belongs in `clang::tidy`
> > > > and what belongs in `clang::tidy::utils`?
> > > Well as the file moved out of utils, its no longer in the utils namespace.
> > Yes, if you move the file out of utils then it shouldn't be in the
> > utils namespace.  But why move the file in the first place?
> > 
> > IOW, what is the guideline for what files belong under `utils`
> > and what files belong under `clang-tidy`?
> Utils is for utilities that checks can make use of, but aren't required for 
> the base clang-tidy infra to work.
> By having the Inserter as a dependancy for ClangTidyDiagnosticConsumer it is 
> now required for clang-tidy infrastructure, hence it was moved out of utils.
OK yeah, that's a good explanation, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117409

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

[PATCH] D70401: [RISCV] Complete RV32E/ilp32e implementation

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added inline comments.



Comment at: llvm/lib/Support/TargetParser.cpp:339
+if (ISAInfo.hasExtension("d"))
+  return "ilp32d";
 return "ilp32";

khchen wrote:
> why do we need to change the order?
IMO, when `e` is combined with `d`, `e` should have higher priority so that the 
default ABI will be `ilp32e` and then this error will be reported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D117535: [clang-tidy] Force LF newlines when writing files

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood created this revision.
LegalizeAdulthood added reviewers: alexfh, Quuxplusone, aaron.ballman.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
LegalizeAdulthood requested review of this revision.

The recommendation on Windows is to checkout from git with
core.autolf=false in order to preserve LF line endings on
test files.  However, when creating a new check this results
in modified files as having switched all the line endings on
Windows.  Write all files with explicit LF line endings to
prevent this.

Fixes #52968


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117535

Files:
  clang-tools-extra/clang-tidy/add_new_check.py


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -37,7 +37,7 @@
   return False
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 cpp_found = False
 file_added = False
 for line in lines:
@@ -57,7 +57,7 @@
   check_name_dashes = module + '-' + check_name
   filename = os.path.join(module_path, check_name_camel) + '.h'
   print('Creating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 header_guard = ('LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_' + module.upper() + '_'
 + check_name_camel.upper() + '_H')
 f.write('//===--- ')
@@ -110,7 +110,7 @@
 def write_implementation(module_path, module, namespace, check_name_camel):
   filename = os.path.join(module_path, check_name_camel) + '.cpp'
   print('Creating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 f.write('//===--- ')
 f.write(os.path.basename(filename))
 f.write(' - clang-tidy ')
@@ -168,7 +168,7 @@
 lines = f.readlines()
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 header_added = False
 header_found = False
 check_added = False
@@ -231,7 +231,7 @@
   checkMatcher = re.compile('- New :doc:`(.*)')
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 note_added = False
 header_found = False
 add_note_here = False
@@ -277,7 +277,7 @@
   filename = os.path.normpath(os.path.join(module_path, 
'../../test/clang-tidy/checkers',
check_name_dashes + '.' + 
test_extension))
   print('Creating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 f.write("""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t
 
 // FIXME: Add something that triggers the check here.
@@ -386,7 +386,7 @@
   checks_alias = map(format_link_alias, doc_files)
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 for line in lines:
   f.write(line)
   if line.strip() == ".. csv-table::":
@@ -407,7 +407,7 @@
   filename = os.path.normpath(os.path.join(
   module_path, '../../docs/clang-tidy/checks/', check_name_dashes + 
'.rst'))
   print('Creating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 f.write(""".. title:: clang-tidy - %(check_name_dashes)s
 
 %(check_name_dashes)s


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -37,7 +37,7 @@
   return False
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 cpp_found = False
 file_added = False
 for line in lines:
@@ -57,7 +57,7 @@
   check_name_dashes = module + '-' + check_name
   filename = os.path.join(module_path, check_name_camel) + '.h'
   print('Creating %s...' % filename)
-  with io.open(filename, 'w', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
 header_guard = ('LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_' + module.upper() + '_'
 + check_name_camel.upper() + '_H')
 f.write('//===--- ')
@@ -110,7 +110,7 @@
 def write_implementation(module_path, module, namespace, check_name_camel):
   filename = os.path.join(module_path, check_name_c

[PATCH] D117536: Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

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

revert old changes, breakout c into own language initial changes

revert arclint changes

revert bad arclint change


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117536

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLi

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400715.
psigillito added a comment.

Include multiple commits in review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Lan

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400717.
psigillito added a comment.

- undo delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language == FormatStyle:

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400718.
psigillito added a comment.

- annoying arc changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language == For

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

These are just initial changes, there is still a lot of work and test cases to 
write. I figured I would put this out there to see if this is the direction we 
were thinking.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2504
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;

MyDeveloperDay wrote:
> 
> I get what your trying to do but what happens in this scenario
> 
> `private::mynamespace::g_value -1;`
> 
> 
Yea, it doesnt parse correctly and is split to a new line after private:. I am 
going to revert all the changes in this code review and start looking at 
implementing C as its own language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Hi!

Overall the check looks good to me, I have only one problem. We usually try to 
emit fixes if some parts of the replaced text is a macro, e.g.:

  #define MYMACRO(X) count(X)
  
  if (myCont.MYMACRO(X))
   ...

In the above code snippet, we want to avoid modifying the source inside the 
definition of `MYMACRO`, because that could render the code incorrect if 
`MYMACRO` is used in another context, e.g. with a custom container as well. I 
think most tidy checks are explicitly guarding the replacements to avoid these 
situations.

Once this is fixed and a test is added, the rest looks good to me.




Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:112
+  bool Negated = NegativeComparison != nullptr;
+  const auto *Comparison = Negated ? NegativeComparison : PositiveComparison;
+

Should we have an assert to express that only one of `PositiveComparison` and 
`NegativeComparison` expected to be not-null?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood abandoned this revision.
LegalizeAdulthood added a comment.

Based on discussion, I'm going to move this as a private matcher
in the check where I intend to use it.

Therefore, I'm abandoning this review.

I look forward to seeing Yitzhak's generalized matcher `:)`


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

https://reviews.llvm.org/D116518

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

[PATCH] D116521: [CMake] Factor out config prefix finding logic

[John Ericson via Phabricator via cfe-commits]

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



Comment at: llvm/CMakeLists.txt:209
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )

Ericson2314 wrote:
> sebastian-ne wrote:
> > Hi, adding this module path overwrites the `llvm_check_linker_flag` from 
> > `llvm/cmake/modules/LLVMCheckLinkerFlag.cmake` with the one defined in 
> > `cmake/Modules/CheckLinkerFlag.cmake`, which does not check the linker but 
> > the compiler.
> > This breaks our gcc+ld build because ld does not support 
> > `--color-diagnostics`.
> > 
> > Our downstream issue is here: 
> > https://github.com/GPUOpen-Drivers/llpc/issues/1645
> Thanks for letting me know. Let's just rename the `/cmake` one then; nothing 
> there is installed, on purpose, so we can change it freely without worrying 
> breaking an interface used downstream.
> 
> I will make a patch for that in the next 2 days if no one beats me to it.
D117537 fixes this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116521

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

[libunwind] f16a4a0 - [libcxx][libcxxabi][libunwind][cmake] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via cfe-commits]

Author: John Ericson
Date: 2022-01-18T06:44:57Z
New Revision: f16a4a034a279f52c33f41cafb7d9751ee8a01dd

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

LOG: [libcxx][libcxxabi][libunwind][cmake] Use `GNUInstallDirs` to support 
custom installation dirs

I am breaking apart D99484 so the cause of build failures is easier to
understand.

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

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxx/cmake/Modules/HandleLibCXXABI.cmake
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt

Removed: 




diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index f78fb77ceb378..b44b16088effe 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -10,6 +10,8 @@ endif()
 
#===
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
 # Add path for custom modules
@@ -412,9 +414,9 @@ endif ()
 # TODO: Projects that depend on libc++ should use LIBCXX_GENERATED_INCLUDE_DIR
 # instead of hard-coding include/c++/v1.
 
-set(LIBCXX_INSTALL_INCLUDE_DIR "include/c++/v1" CACHE PATH
+set(LIBCXX_INSTALL_INCLUDE_DIR "${CMAKE_INSTALL_INCLUDEDIR}/c++/v1" CACHE PATH
 "Path where target-agnostic libc++ headers should be installed.")
-set(LIBCXX_INSTALL_RUNTIME_DIR bin CACHE PATH
+set(LIBCXX_INSTALL_RUNTIME_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
 "Path where built libc++ runtime libraries should be installed.")
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
@@ -423,7 +425,7 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR 
"${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
   set(LIBCXX_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE} CACHE PATH
   "Path where built libc++ libraries should be installed.")
-  set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR 
"include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1" CACHE PATH
+  set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1" CACHE PATH
   "Path where target-specific libc++ headers should be installed.")
   if(LIBCXX_LIBDIR_SUBDIR)
 string(APPEND LIBCXX_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})

diff  --git a/libcxx/cmake/Modules/HandleLibCXXABI.cmake 
b/libcxx/cmake/Modules/HandleLibCXXABI.cmake
index 5a8a4a270a1a1..d69405ddeeacf 100644
--- a/libcxx/cmake/Modules/HandleLibCXXABI.cmake
+++ b/libcxx/cmake/Modules/HandleLibCXXABI.cmake
@@ -1,8 +1,9 @@
-
 
#===
 # Add an ABI library if appropriate
 
#===
 
+include(GNUInstallDirs)
+
 #
 # _setup_abi: Set up the build to use an ABI library
 #
@@ -63,7 +64,7 @@ macro(setup_abi_lib abidefines abishared abistatic abifiles 
abidirs)
 
 if (LIBCXX_INSTALL_HEADERS)
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION include/c++/v1/${dstdir}
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}"
 COMPONENT cxx-headers
 PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
 )

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index b73804390ba20..6b3bcef2851da 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -10,6 +10,8 @@ endif()
 
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
 # Add path for custom modules
@@ -210,7 +212,7 @@ set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
   )
 
-set(LIBCXXABI_INSTALL_RUNTIME_DIR bin CACHE PATH
+set(LIBCXXABI_INSTALL_RUNTIME_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
 "Path where built libc++abi runtime libraries should be installed.")
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)

diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 33741aea8f77a..340a103a4abe8 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -8,6 +8,8 @@ endif()
 
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
 
 # Add path for custom modules
@@ -137,9 +139,9 @@ set(CMAKE_MODULE_PATH
 "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
 ${CMAKE_MODULE_PATH})
 
-set(LIBUNWIND_INSTALL_INCLUDE_DIR include CACHE PATH
+set(LIBUNWIND_INSTALL_INCLUDE_DIR "${CMAKE_INSTALL_INCLUDEDIR}" CACHE PATH
 "Path where built libunwind headers should be installed.")
-set(LIBUNWIND_INSTALL_RUNTIME_DIR bin CACHE PATH
+set(LIBUNWIND_INSTA

[PATCH] D117496: [clang][dataflow] Add transfer function for addrof

[Gábor Horváth via Phabricator via cfe-commits]

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

The address-of part looks good to me. However, just realized that I'm not 
convinced whether the dereference operator, or references, in general, are 
correctly handled.




Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:190
   Env.setValue(Loc, Env.takeOwnership(std::make_unique(
 SubExprVal->getPointeeLoc(;
+  break;

I know this is not strictly related to this patch, but why do we actually need 
do `getPointeeLoc` here? I'd expect `UO_Deref` to be a noop for all intents and 
purposes.

E.g. for a snippet like: 
```
int f(int *p) {
return *p;
}
```

The AST looks like:
```
  `-CompoundStmt 0x5565d151d038 
`-ReturnStmt 0x5565d151d028 
  `-ImplicitCastExpr 0x5565d151d010  'int' 
`-UnaryOperator 0x5565d151cff8  'int' lvalue prefix '*' 
cannot overflow
  `-ImplicitCastExpr 0x5565d151cfe0  'int *' 
`-DeclRefExpr 0x5565d151cfc0  'int *' lvalue ParmVar 
0x5565d151ce00 'p' 'int *'
```

I'd expect any actual dereference to happen in the `LValueToRValue` cast.
The reason is, this is how references can be handled. E.g.:
```
void f(int &p) {
int &q = p;
int r = p;
}
```

Has the AST:
```
|-DeclStmt 0x55d49a1f00b0 
| `-VarDecl 0x55d49a1effd0  col:10 q 'int &' cinit
|   `-DeclRefExpr 0x55d49a1f0038  'int' lvalue ParmVar 
0x55d49a1efe00 'p' 'int &'
`-DeclStmt 0x55d49a1f0180 
  `-VarDecl 0x55d49a1f00e0  col:9 r 'int' cinit
`-ImplicitCastExpr 0x55d49a1f0168  'int' 
  `-DeclRefExpr 0x55d49a1f0148  'int' lvalue ParmVar 
0x55d49a1efe00 'p' 'int &'
```

The reason why you know when should you propagate the reference or the pointee 
of the reference from the subexpression is because of the `LValueToRValue` 
cast. So you already need to do the "dereference" operator there to correctly 
handle references. And if you already do that, I see no reason to duplicate 
this logic here for pointers. Do I miss something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117496

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence updated this revision to Diff 400733.
achieveartificialintelligence added a comment.
Herald added subscribers: alextsao1999, eopXD.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoD.td
  llvm/lib/Target/RISCV/RISCVInstrInfoF.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32i-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-valid.s
  llvm/test/MC/RISCV/rv32zfinx-invalid.s
  llvm/test/MC/RISCV/rv32zfinx-valid.s
  llvm/test/MC/RISCV/rv32zhinx-invalid.s
  llvm/test/MC/RISCV/rv32zhinx-valid.s
  llvm/test/MC/RISCV/rv32zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv32zhinxmin-valid.s
  llvm/test/MC/RISCV/rv64zdinx-invalid.s
  llvm/test/MC/RISCV/rv64zdinx-valid.s
  llvm/test/MC/RISCV/rv64zfinx-invalid.s
  llvm/test/MC/RISCV/rv64zfinx-valid.s
  llvm/test/MC/RISCV/rv64zhinx-invalid.s
  llvm/test/MC/RISCV/rv64zhinx-valid.s
  llvm/test/MC/RISCV/rv64zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv64zhinxmin-valid.s
  llvm/test/MC/RISCV/rvzdinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzhinx-aliases-valid.s

Index: llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
@@ -0,0 +1,82 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+
+##===--===##
+## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20)
+##===--===##
+
+# CHECK-INST: fsgnjx.h s1, s2, s2
+# CHECK-ALIAS: fabs.h s1, s2
+fabs.h s1, s2
+# CHECK-INST: fsgnjn.h s2, s3, s3
+# CHECK-ALIAS: fneg.h s2, s3
+fneg.h s2, s3
+
+# CHECK-INST: flt.h tp, s6, s5
+# CHECK-ALIAS: flt.h tp, s6, s5
+fgt.h x4, s5, s6
+# CHECK-INST: fle.h t2, s1, s0
+# CHECK-ALIAS: fle.h t2, s1, s0
+fge.h x7, x8, x9
+
+##===--===##
+## Aliases which omit the rounding mode.
+##===--===##
+
+# CHECK-INST: fmadd.h a0, a1, a2, a3, dyn
+# CHECK-ALIAS: fmadd.h a0, a1, a2, a3
+fmadd.h x10, x11, x12, x13
+# CHECK-INST: fmsub.h a4, a5, a6, a7, dyn
+# CHECK-ALIAS: fmsub.h a4, a5, a6, a7
+fmsub.h x14, x15, x16, x17
+# CHECK-INST: fnmsub.h s2, s3, s4, s5, dyn
+# CHECK-ALIAS: fnmsub.h s2, s3, s4, s5
+fnmsub.h x18, x19, x20, x21
+# CHECK-INST: fnmadd.h s6, s7, s8, s9, dyn
+# CHECK-ALIAS: fnmadd.h s6, s7, s8, s9
+fnmadd.h x22, x23, x24, x25
+# CHECK-INST: fadd.h s10, s11, t3, dyn
+# CHECK-ALIAS: fadd.h s10, s11, t3
+fadd.h x26, x27, x28
+# CHECK-INST: fsub.h t4, t5, t6, dyn
+# CHECK-ALIAS: fsub.h t4, t5, t6
+fsub.h x29, x30, x31
+# CHECK-INST: fmul.h s0, s1, s2, dyn
+# CHECK-ALIAS: fmul.h s0, s1, s2
+fmul.h s0, s1, s2
+# CHECK-INST: fdiv.h s3, s4, s5, dyn
+# CHECK-ALIAS: fdiv.h s3, s4, s5
+fdiv.h s3, s4, s5
+# CHECK-INST: fsqrt.h s6, s7, dyn
+# CHECK-ALIAS: fsqrt.h s6, s7
+fsqrt.h s6, s7
+# CHECK-INST: fcvt.w.h a0, s5, dyn
+# CHECK-ALIAS: fcvt.w.h a0, s5
+fcvt.w.h a0, s5
+# CHECK-INST: fcvt.wu.h a1, s6, dyn
+# CHECK-ALIAS: fcvt.wu.h a1, s6
+fcvt.wu.h a1, s6
+# CHECK-INST: fcvt.h.w t6, a4, dyn
+# CHECK-ALIAS: fcvt.h.w t6, a4
+fcvt.h.w t6, a4
+# CHECK-INST: fcvt.h.wu s0, a5, dyn
+# CHECK-ALIAS: fcvt.h.wu s0, a5
+fcvt.h.wu s0, a5
Index: llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
=

[clang] 782eced - [clang][dataflow] Replace initValueInStorageLocation with createValue

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-18T07:09:35Z
New Revision: 782eced561492c74f7b4409d6ee7eee84a1647c7

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

LOG: [clang][dataflow] Replace initValueInStorageLocation with createValue

Since Environment's setValue method already does part of the work that
initValueInStorageLocation does, we can factor out a new createValue
method to reduce the duplication.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed-by: ymandel, xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 9613b2921c3f2..5082181ecf3e6 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -116,15 +116,15 @@ class Environment {
   /// in the environment.
   StorageLocation *getThisPointeeStorageLocation() const;
 
-  /// Creates a value appropriate for `Type`, assigns it to `Loc`, and returns
-  /// it, if `Type` is supported, otherwise return null. If `Type` is a pointer
-  /// or reference type, creates all the necessary storage locations and values
-  /// for indirections until it finds a non-pointer/non-reference type.
+  /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
+  /// return null. If `Type` is a pointer or reference type, creates all the
+  /// necessary storage locations and values for indirections until it finds a
+  /// non-pointer/non-reference type.
   ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
-  Value *initValueInStorageLocation(const StorageLocation &Loc, QualType Type);
+  Value *createValue(QualType Type);
 
   /// Assigns `Val` as the value of `Loc` in the environment.
   void setValue(const StorageLocation &Loc, Value &Val);
@@ -150,8 +150,8 @@ class Environment {
   Value &takeOwnership(std::unique_ptr Val);
 
 private:
-  /// Returns the value assigned to `Loc` in the environment or null if `Type`
-  /// isn't supported.
+  /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
+  /// return null.
   ///
   /// Recursively initializes storage locations and values until it sees a
   /// self-referential pointer or reference type. `Visited` is used to track
@@ -161,9 +161,8 @@ class Environment {
   /// Requirements:
   ///
   ///  `Type` must not be null.
-  Value *initValueInStorageLocationUnlessSelfReferential(
-  const StorageLocation &Loc, QualType Type,
-  llvm::DenseSet &Visited);
+  Value *createValueUnlessSelfReferential(QualType Type,
+  llvm::DenseSet &Visited);
 
   StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 7d0cbf3f656bd..e1d420fb55b82 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -49,7 +49,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
   assert(ParamDecl != nullptr);
   auto &ParamLoc = createStorageLocation(*ParamDecl);
   setStorageLocation(*ParamDecl, ParamLoc);
-  initValueInStorageLocation(ParamLoc, ParamDecl->getType());
+  if (Value *ParamVal = createValue(ParamDecl->getType()))
+setValue(ParamLoc, *ParamVal);
 }
   }
 
@@ -60,7 +61,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
   if (!ThisPointeeType->isUnionType()) {
 auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
 DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
-initValueInStorageLocation(ThisPointeeLoc, ThisPointeeType);
+if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+  setValue(ThisPointeeLoc, *ThisPointeeVal);
   }
 }
   }
@@ -189,21 +191,17 @@ Value *Environment::getValue(const Expr &E, SkipPast SP) 
const {
   return getValue(*Loc);
 }
 
-Value *Environment::initValueInStorageLocation(const StorageLocation &Loc,
-   QualType Type) {
+Value *Environment::createValue(QualType Type) {
   llvm::DenseSet Visited;
-  return initValueInStorageLocationUnlessSelfReferential(Loc,

[PATCH] D117493: [clang][dataflow] Replace initValueInStorageLocation with createValue

[Stanislav Gatev 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 rG782eced56149: [clang][dataflow] Replace 
initValueInStorageLocation with createValue (authored by sgatev).
Herald added a subscriber: steakhal.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117493

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp

Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -96,7 +96,8 @@
 const Expr *InitExpr = D.getInit();
 if (InitExpr == nullptr) {
   // No initializer expression - associate `Loc` with a new value.
-  Env.initValueInStorageLocation(Loc, D.getType());
+  if (Value *Val = Env.createValue(D.getType()))
+Env.setValue(Loc, *Val);
   return;
 }
 
@@ -117,7 +118,8 @@
 // FIXME: The initializer expression must always be assigned a value.
 // Replace this with an assert when we have sufficient coverage of
 // language features.
-Env.initValueInStorageLocation(Loc, D.getType());
+if (Value *Val = Env.createValue(D.getType()))
+  Env.setValue(Loc, *Val);
   }
   return;
 }
@@ -128,7 +130,8 @@
   // FIXME: The initializer expression must always be assigned a value.
   // Replace this with an assert when we have sufficient coverage of
   // language features.
-  Env.initValueInStorageLocation(Loc, D.getType());
+  if (Value *Val = Env.createValue(D.getType()))
+Env.setValue(Loc, *Val);
 } else {
   llvm_unreachable("structs and classes must always be assigned values");
 }
@@ -269,7 +272,8 @@
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
-Env.initValueInStorageLocation(Loc, S->getType());
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
   }
 
   void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -319,7 +323,8 @@
   void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) {
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
-Env.initValueInStorageLocation(Loc, S->getType());
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
   }
 
   void VisitCallExpr(const CallExpr *S) {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -49,7 +49,8 @@
   assert(ParamDecl != nullptr);
   auto &ParamLoc = createStorageLocation(*ParamDecl);
   setStorageLocation(*ParamDecl, ParamLoc);
-  initValueInStorageLocation(ParamLoc, ParamDecl->getType());
+  if (Value *ParamVal = createValue(ParamDecl->getType()))
+setValue(ParamLoc, *ParamVal);
 }
   }
 
@@ -60,7 +61,8 @@
   if (!ThisPointeeType->isUnionType()) {
 auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
 DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
-initValueInStorageLocation(ThisPointeeLoc, ThisPointeeType);
+if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+  setValue(ThisPointeeLoc, *ThisPointeeVal);
   }
 }
   }
@@ -189,21 +191,17 @@
   return getValue(*Loc);
 }
 
-Value *Environment::initValueInStorageLocation(const StorageLocation &Loc,
-   QualType Type) {
+Value *Environment::createValue(QualType Type) {
   llvm::DenseSet Visited;
-  return initValueInStorageLocationUnlessSelfReferential(Loc, Type, Visited);
+  return createValueUnlessSelfReferential(Type, Visited);
 }
 
-Value *Environment::initValueInStorageLocationUnlessSelfReferential(
-const StorageLocation &Loc, QualType Type,
-llvm::DenseSet &Visited) {
+Value *Environment::createValueUnlessSelfReferential(
+QualType Type, llvm::DenseSet &Visited) {
   assert(!Type.isNull());
 
   if (Type->isIntegerType()) {
-auto &Val = takeOwnership(std::make_unique());
-setValue(Loc, Val);
-return &Val;
+return &takeOwnership(std::make_unique());
   }
 
   if (Type->isReferenceType()) {
@@ -212,14 +210,15 @@
 
 if (!Visited.contains(PointeeType.getCanonicalType())) {
   Visited.insert(PointeeType.getCanonicalType());
-  initValueInStorageLocationUnlessSelfReferential(PointeeLoc, PointeeType,
-  Visited);
+  Value *PointeeVal =
+  createValueUnlessSelfReferential(PointeeType, Visited);
 

[PATCH] D115124: [clang-tidy] Fix `readability-container-size-empty` check for smart pointers

[gehry via Phabricator via cfe-commits]

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

I think it looks great and safe enough.  It would be better to turn 
`!(*ptr).empty()` into `!ptr->empty()`, but I have no particular opinions at 
this point.  Let's see if @aaron.ballman has any comments.


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

https://reviews.llvm.org/D115124

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[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.
Thanks for contributing!

Let's wait a day or two before landing to let other reviewers chime in.

Do you need help landing?
If so please provide your name and email address that you'd like to be used for 
the commit.
Otherwise, you can request a commit access if you continue contributing (cf. 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[MyDeveloperDay via Phabricator via cfe-commits]

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

I'm not a fan of this approach of adding a "C" language, mainly because of the 
`.h` problem so ultimately it doesn't solve your problem.

I think this is overkill for what is basically the subtle handling of 
"public/private/protected/try/delete/new" being used as variables (which 
frankly is at least in my view not a bright idea in the first place!)

I think ultimately we can try to catch those corner cases but IMHO it doesn't 
need the addition of a whole new language for that, (we are not at that much on 
an impasse)

This review contains .arclint that needs removing, this review contains no tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[clang] 966f24e - [clang-format] Add a BlockIndent option to AlignAfterOpenBracket

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

Author: Cameron Mulhern
Date: 2022-01-17T09:03:23+01:00
New Revision: 966f24e5a62a9f5df518357c2d4b0e361244a624

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

LOG: [clang-format] Add a BlockIndent option to AlignAfterOpenBracket

This style is similar to AlwaysBreak, but places closing brackets on new lines.

For example, if you have a multiline parameter list, clang-format currently 
only supports breaking per-parameter, but places the closing bracket on the 
line of the last parameter.

Function(
param1,
param2,
param3);

A style supported by other code styling tools (e.g. rustfmt) is to allow the 
closing brackets to be placed on their own line, aiding the user in being able 
to quickly infer the bounds of the block of code.

Function(
param1,
param2,
param3
);

For prior work on a similar feature, see: https://reviews.llvm.org/D33029.

Note: This currently only supports block indentation for closing parentheses.

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/ContinuationIndenter.h
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index b752df864c56a..8e2a7924063cd 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -214,6 +214,22 @@ the configuration (without a prefix: ``Auto``).
   someLongFunction(
   argument1, argument2);
 
+  * ``BAS_BlockIndent`` (in configuration: ``BlockIndent``)
+Always break after an open bracket, if the parameters don't fit
+on a single line. Closing brackets will be placed on a new line.
+E.g.:
+
+.. code-block:: c++
+
+  someLongFunction(
+  argument1, argument2
+  )
+
+
+.. warning:: 
+
+ Note: This currently only applies to parentheses.
+
 
 
 **AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) 
:versionbadge:`clang-format 13`

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b01f3035fc00c..9fd22297ae423 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -300,6 +300,10 @@ clang-format
 - Option ``AllowShortEnumsOnASingleLine: false`` has been improved, it now
   correctly places the opening brace according to ``BraceWrapping.AfterEnum``.
 
+- Option ``AlignAfterOpenBracket: BlockIndent`` has been added. If set, it will
+  always break after an open bracket, if the parameters don't fit on a single
+  line. Closing brackets will be placed on a new line.
+
 - Option ``QualifierAlignment`` has been added in order to auto-arrange the
   positioning of specifiers/qualifiers
   `const` `volatile` `static` `inline` `constexpr` `restrict`

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 7841a7a9949d0..0b5f74503d807 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -82,6 +82,19 @@ struct FormatStyle {
 ///   argument1, argument2);
 /// \endcode
 BAS_AlwaysBreak,
+/// Always break after an open bracket, if the parameters don't fit
+/// on a single line. Closing brackets will be placed on a new line.
+/// E.g.:
+/// \code
+///   someLongFunction(
+///   argument1, argument2
+///   )
+/// \endcode
+///
+/// \warning
+///  Note: This currently only applies to parentheses.
+/// \endwarning
+BAS_BlockIndent,
   };
 
   /// If ``true``, horizontally aligns arguments after an open bracket.

diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 9b1d4ecf3..28f13c06e3088 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -341,6 +341,8 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   if (State.Stack.back().BreakBeforeClosingBrace &&
   Current.closesBlockOrBlockTypeList(Style))
 return true;
+  if (State.Stack.back().BreakBeforeClosingParen && Current.is(tok::r_paren))
+return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
@@ -639,10 +641,12 @@ void 
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
   State.Stack.back().ColonPos = FirstColonPos;
   }
 
-  // In "AlwaysBreak" mode, enforce wrapping directly after the parenthesis by
-  // disallowing any further line 

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers 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 rG966f24e5a62a: [clang-format] Add a BlockIndent option to 
AlignAfterOpenBracket (authored by csmulhern, committed by HazardyKnusperkeks).

Changed prior to commit:
  https://reviews.llvm.org/D109557?vs=400435&id=400452#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19167,6 +19167,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -23696,6 +23698,224 @@
Style);
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n

[PATCH] D117456: [clangd] Avoid a code completion crash

[Haojian Wu via Phabricator via cfe-commits]

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

This is a workaround (adding a newline to the eof) in clangd to avoid the code
completion crash, see https://github.com/clangd/clangd/issues/332.

In principle, this is a clang bug, we should fix it in clang, but it is not
trivial.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117456

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2474,6 +2474,26 @@
   )cpp");
 }
 
+TEST(CompletionTest, NoCrashOnMissingNewLineAtEOF) {
+  auto FooCpp = testPath("foo.cpp");
+
+  MockCompilationDatabase CDB;
+  MockFS FS;
+  Annotations F("#pragma ^ // no new line");
+  FS.Files[FooCpp] = F.code().str();
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+  runAddDocument(Server, FooCpp, F.code());
+  // Run completion outside the file range.
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, F.point(),
+   clangd::CodeCompleteOptions()))
+  .Completions,
+  IsEmpty());
+  EXPECT_THAT(cantFail(runSignatureHelp(Server, FooCpp, F.point(),
+MarkupKind::PlainText))
+  .signatures,
+  IsEmpty());
+}
+
 TEST(GuessCompletionPrefix, Filters) {
   for (llvm::StringRef Case : {
"[[scope::]][[ident]]^",
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -369,6 +369,10 @@
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -417,6 +421,10 @@
   return CB(error("Failed to parse includes"));
 
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  DocumentationFormat));


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2474,6 +2474,26 @@
   )cpp");
 }
 
+TEST(CompletionTest, NoCrashOnMissingNewLineAtEOF) {
+  auto FooCpp = testPath("foo.cpp");
+
+  MockCompilationDatabase CDB;
+  MockFS FS;
+  Annotations F("#pragma ^ // no new line");
+  FS.Files[FooCpp] = F.code().str();
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+  runAddDocument(Server, FooCpp, F.code());
+  // Run completion outside the file range.
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, F.point(),
+   clangd::CodeCompleteOptions()))
+  .Completions,
+  IsEmpty());
+  EXPECT_THAT(cantFail(runSignatureHelp(Server, FooCpp, F.point(),
+MarkupKind::PlainText))
+  .signatures,
+  IsEmpty());
+}
+
 TEST(GuessCompletionPrefix, Filters) {
   for (llvm::StringRef Case : {
"[[scope::]][[ident]]^",
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -369,6 +369,10 @@
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -417,6 +421,10 @@
   return CB(error("Failed to parse includes"));
 
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a cras

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400455.
glotchimo marked an inline comment as done.
glotchimo added a comment.

Add equality operator tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,26 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,26 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM.


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] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Could you check if your patch fixes 
https://github.com/llvm/llvm-project/issues/33044 as well?
If so, please add tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[clang-tools-extra] 64c108c - [clangd] Better handling `\n` in the synthesized diagnostic message.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-17T09:27:58+01:00
New Revision: 64c108c9e4e0525328ceb4da55e4ab4b765d4c27

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

LOG: [clangd] Better handling `\n` in the synthesized diagnostic message.

The newline-eof fix was rendered as "insert '...'", this patch
special-case it.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 044ef1934426a..40eab3f647424 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -631,7 +631,10 @@ void StoreDiags::EndSourceFile() {
 /// the result is not too large and does not contain newlines.
 static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) 
{
   constexpr unsigned MaxLen = 50;
-
+  if (Code == "\n") {
+OS << "\\n";
+return;
+  }
   // Only show the first line if there are many.
   llvm::StringRef R = Code.split('\n').first;
   // Shorten the message if it's too long.

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 5303917402afd..76dd20177c47c 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -623,6 +623,15 @@ n]] = 10; // error-ok
   Fix(Source.range(), "ident", "change 'ide\\…' to 
'ident'";
 }
 
+TEST(DiagnosticTest, NewLineFixMessage) {
+  Annotations Source("int a;[[]]");
+  TestTU TU = TestTU::withCode(Source.code());
+  TU.ExtraArgs = {"-Wnewline-eof"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(WithFix((Fix(Source.range(), "\n", "insert '\\n'");
+}
+
 TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
   Annotations Main(R"cpp(
 int main() {



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

[PATCH] D117294: [clangd] Better handling `\n` in the synthesized diagnostic message.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64c108c9e4e0: [clangd] Better handling `\n` in the 
synthesized diagnostic message. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117294

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


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -623,6 +623,15 @@
   Fix(Source.range(), "ident", "change 'ide\\…' to 
'ident'";
 }
 
+TEST(DiagnosticTest, NewLineFixMessage) {
+  Annotations Source("int a;[[]]");
+  TestTU TU = TestTU::withCode(Source.code());
+  TU.ExtraArgs = {"-Wnewline-eof"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(WithFix((Fix(Source.range(), "\n", "insert '\\n'");
+}
+
 TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
   Annotations Main(R"cpp(
 int main() {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -631,7 +631,10 @@
 /// the result is not too large and does not contain newlines.
 static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) 
{
   constexpr unsigned MaxLen = 50;
-
+  if (Code == "\n") {
+OS << "\\n";
+return;
+  }
   // Only show the first line if there are many.
   llvm::StringRef R = Code.split('\n').first;
   // Shorten the message if it's too long.


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -623,6 +623,15 @@
   Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'";
 }
 
+TEST(DiagnosticTest, NewLineFixMessage) {
+  Annotations Source("int a;[[]]");
+  TestTU TU = TestTU::withCode(Source.code());
+  TU.ExtraArgs = {"-Wnewline-eof"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(WithFix((Fix(Source.range(), "\n", "insert '\\n'");
+}
+
 TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
   Annotations Main(R"cpp(
 int main() {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -631,7 +631,10 @@
 /// the result is not too large and does not contain newlines.
 static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) {
   constexpr unsigned MaxLen = 50;
-
+  if (Code == "\n") {
+OS << "\\n";
+return;
+  }
   // Only show the first line if there are many.
   llvm::StringRef R = Code.split('\n').first;
   // Shorten the message if it's too long.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117428: [docs] Clarify & update JSONCompilationDatabase docs

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/docs/JSONCompilationDatabase.rst:95
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.

sammccall wrote:
> Hopefully I'm not too far out on a limb here.
> 
> Today, `command` is generated by CMake & others. The shell unescaping 
> performed is underspecified and platform-specific (`#if _WIN32 ... #else`). 
> There are non-llvm consumers of this format (e.g. vscode-cpptools).
> 
> I think we're in danger of interop problems and gently encouraging migration 
> to `arguments` reduces the danger.
yeah, the `arguments` was added 7 years ago, and most of tools are still 
generating `command`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117428

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D117421#3246627 , @glotchimo wrote:

> I don't know why, but `clang-format` reformatted most if not all of the 
> long/block comments in `WhitespaceManager.cpp`. Will it be necessary for me 
> to revert the changes to those comments and skip formatting when updating the 
> diff so as to keep it minimal?

If when editing you save (and clang-format) when you've missed a `}` then the 
file likely reindented, this, in turn, reflowed the comments, which means when 
you add the '}' back in, those comments have newlines in different places

once reflowed the comments don't reflow back.

This is commonly how I mess up, I then `git difftool` with something like 
winmerge and revert the comments one by one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:16172
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);

can you add a test showing what happens when you don't have the `{` in there 
definition?



Comment at: clang/unittests/Format/FormatTest.cpp:16189
+   "int &operator!=() {",
+   Alignment);
 

can you mi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117456: [clangd] Avoid a code completion crash

[Sam McCall via Phabricator via cfe-commits]

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

This isn't an ideal fix but it's simple, we know the real fix is invasive, and 
14 is upon us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117456

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

[clang] 1694976 - [docs] Clarify & update JSONCompilationDatabase docs

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-17T09:51:55+01:00
New Revision: 16949762dc6a670d2f3a1f5043262ec31e09c556

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

LOG: [docs] Clarify & update JSONCompilationDatabase docs

- prefer `arguments` over `command`, and add example
- clarify that there's no shell-unescaping of `arguments`

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

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

Added: 


Modified: 
clang/docs/JSONCompilationDatabase.rst

Removed: 




diff  --git a/clang/docs/JSONCompilationDatabase.rst 
b/clang/docs/JSONCompilationDatabase.rst
index fd822c78d1fe..3595cf452f4c 100644
--- a/clang/docs/JSONCompilationDatabase.rst
+++ b/clang/docs/JSONCompilationDatabase.rst
@@ -63,8 +63,13 @@ Example:
 
 [
   { "directory": "/home/user/llvm/build",
-"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"arguments": ["/usr/bin/clang++", "-Irelative", "-DSOMEDEF=With 
spaces, quotes and \\-es.", "-c", "-o", "file.o", "file.cc"],
 "file": "file.cc" },
+
+  { "directory": "/home/user/llvm/build",
+"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"file": "file2.cc" },
+
   ...
 ]
 
@@ -78,14 +83,17 @@ The contracts for each field in the command object are:
compilation database. There can be multiple command objects for the
same file, for example if the same source file is compiled with

diff erent configurations.
--  **command:** The compile command executed. After JSON unescaping,
-   this must be a valid command to rerun the exact compilation step for
-   the translation unit in the environment the build system uses.
-   Parameters use shell quoting and shell escaping of quotes, with '``"``'
-   and '``\``' being the only special characters. Shell expansion is not
-   supported.
--  **arguments:** The compile command executed as list of strings.
-   Either **arguments** or **command** is required.
+-  **arguments:** The compile command argv as list of strings.
+   This should run the compilation step for the translation unit ``file``.
+   ``arguments[0]`` should be the executable name, such as ``clang++``.
+   Arguments should not be escaped, but ready to pass to ``execvp()``.
+-  **command:** The compile command as a single shell-escaped string.
+   Arguments may be shell quoted and escaped following platform conventions,
+   with '``"``' and '``\``' being the only special characters. Shell expansion
+   is not supported.
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.
 -  **output:** The name of the output created by this compilation step.
This field is optional. It can be used to distinguish 
diff erent processing
modes of the same input file.



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

[PATCH] D117428: [docs] Clarify & update JSONCompilationDatabase docs

[Sam McCall 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 rG16949762dc6a: [docs] Clarify & update 
JSONCompilationDatabase docs (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117428

Files:
  clang/docs/JSONCompilationDatabase.rst


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -63,8 +63,13 @@
 
 [
   { "directory": "/home/user/llvm/build",
-"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"arguments": ["/usr/bin/clang++", "-Irelative", "-DSOMEDEF=With 
spaces, quotes and \\-es.", "-c", "-o", "file.o", "file.cc"],
 "file": "file.cc" },
+
+  { "directory": "/home/user/llvm/build",
+"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"file": "file2.cc" },
+
   ...
 ]
 
@@ -78,14 +83,17 @@
compilation database. There can be multiple command objects for the
same file, for example if the same source file is compiled with
different configurations.
--  **command:** The compile command executed. After JSON unescaping,
-   this must be a valid command to rerun the exact compilation step for
-   the translation unit in the environment the build system uses.
-   Parameters use shell quoting and shell escaping of quotes, with '``"``'
-   and '``\``' being the only special characters. Shell expansion is not
-   supported.
--  **arguments:** The compile command executed as list of strings.
-   Either **arguments** or **command** is required.
+-  **arguments:** The compile command argv as list of strings.
+   This should run the compilation step for the translation unit ``file``.
+   ``arguments[0]`` should be the executable name, such as ``clang++``.
+   Arguments should not be escaped, but ready to pass to ``execvp()``.
+-  **command:** The compile command as a single shell-escaped string.
+   Arguments may be shell quoted and escaped following platform conventions,
+   with '``"``' and '``\``' being the only special characters. Shell expansion
+   is not supported.
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.
 -  **output:** The name of the output created by this compilation step.
This field is optional. It can be used to distinguish different processing
modes of the same input file.


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -63,8 +63,13 @@
 
 [
   { "directory": "/home/user/llvm/build",
-"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, quotes and \\-es.\" -c -o file.o file.cc",
+"arguments": ["/usr/bin/clang++", "-Irelative", "-DSOMEDEF=With spaces, quotes and \\-es.", "-c", "-o", "file.o", "file.cc"],
 "file": "file.cc" },
+
+  { "directory": "/home/user/llvm/build",
+"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, quotes and \\-es.\" -c -o file.o file.cc",
+"file": "file2.cc" },
+
   ...
 ]
 
@@ -78,14 +83,17 @@
compilation database. There can be multiple command objects for the
same file, for example if the same source file is compiled with
different configurations.
--  **command:** The compile command executed. After JSON unescaping,
-   this must be a valid command to rerun the exact compilation step for
-   the translation unit in the environment the build system uses.
-   Parameters use shell quoting and shell escaping of quotes, with '``"``'
-   and '``\``' being the only special characters. Shell expansion is not
-   supported.
--  **arguments:** The compile command executed as list of strings.
-   Either **arguments** or **command** is required.
+-  **arguments:** The compile command argv as list of strings.
+   This should run the compilation step for the translation unit ``file``.
+   ``arguments[0]`` should be the executable name, such as ``clang++``.
+   Arguments should not be escaped, but ready to pass to ``execvp()``.
+-  **command:** The compile command as a single shell-escaped string.
+   Arguments may be shell quoted and escaped following platform conventions,
+   with '``"``' and '``\``' being the only special characters. Shell expansion
+   is not supported.
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.
 -  **output:** The name of the output created by this compilation step.
This field is option

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

With your patch I still see the bug in this case:

  struct S {
constexpr S(const S &) = default;
void f()   = default;
S &operator/**/=(S) {}
  };

Mind the `operator  =`.
It happens every time that `operator=` has only declaration (no definition 
body).

Another failure:

  struct S {
/*comment*/ S(const S &) = default;
void f() = default;
S &operator/**/  =(S) {}
  };

As mentioned yesterday, comment between `operator` and `=`.

Note, `/*comment*/` is there just to make the spacing more visible. It's 
unrelated to the bug otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:735
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;

This should fix the `operator/**/ =` problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:735-742
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }

curdeius wrote:
> This should fix the `operator/**/ =` problem.
Can't we simplify this?



Comment at: clang/lib/Format/WhitespaceManager.cpp:738
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;

And this should fix the declaration-only problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 400464.
gamesh411 marked 7 inline comments as done.
gamesh411 added a comment.

Fixes round two


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116025

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


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -52,7 +52,7 @@
 "Untrusted data is passed to a system call "
 "(CERT/STR02-C. Sanitize data passed to complex subsystems)";
 
-/// Check if tainted data is used as a buffer size ins strn.. functions,
+/// Check if tainted data is used as a buffer size in strn.. functions,
 /// and allocators.
 constexpr llvm::StringLiteral MsgTaintedBufferSize =
 "Untrusted data is used to specify the buffer size "
@@ -160,7 +160,8 @@
 public:
   ArgSet() = default;
   ArgSet(ArgVecTy &&DiscreteArgs, Optional VariadicIndex = None)
-  : DiscreteArgs(DiscreteArgs), VariadicIndex(VariadicIndex) {}
+  : DiscreteArgs(std::move(DiscreteArgs)),
+VariadicIndex(std::move(VariadicIndex)) {}
 
   bool contains(ArgIdxTy ArgIdx) const {
 if (llvm::is_contained(DiscreteArgs, ArgIdx))
@@ -308,13 +309,13 @@
  TaintConfiguration &&Config) const;
 
 private:
-  using NamePartTy = llvm::SmallVector, 2>;
+  using NamePartsTy = llvm::SmallVector, 2>;
 
   /// Validate part of the configuration, which contains a list of argument
   /// indexes.
   void validateArgVector(const std::string &Option, const ArgVecTy &Args) 
const;
 
-  template  static NamePartTy parseNameParts(const Config &C);
+  template  static NamePartsTy parseNameParts(const Config 
&C);
 
   // Takes the config and creates a CallDescription for it and associates a 
Rule
   // with that.
@@ -445,9 +446,9 @@
 }
 
 template 
-GenericTaintRuleParser::NamePartTy
+GenericTaintRuleParser::NamePartsTy
 GenericTaintRuleParser::parseNameParts(const Config &C) {
-  NamePartTy NameParts;
+  NamePartsTy NameParts;
   if (!C.Scope.empty()) {
 // If the Scope argument contains multiple "::" parts, those are considered
 // namespace identifiers.
@@ -464,7 +465,7 @@
 void GenericTaintRuleParser::consumeRulesFromConfig(const Config &C,
 GenericTaintRule &&Rule,
 RulesContTy &Rules) {
-  NamePartTy NameParts = parseNameParts(C);
+  NamePartsTy NameParts = parseNameParts(C);
   llvm::SmallVector CallDescParts{NameParts.size()};
   llvm::transform(NameParts, CallDescParts.begin(),
   [](SmallString<32> &S) { return S.c_str(); });
@@ -642,6 +643,7 @@
   llvm::Optional Config =
   getConfiguration(*Mgr, this, Option, ConfigFile);
   if (!Config) {
+// We don't have external taint config, no parsing required.
 DynamicTaintRules = RuleLookupTy{};
 return;
   }
@@ -746,8 +748,8 @@
   bool IsMatching = PropSrcArgs.isEmpty();
   ForEachCallArg(
   [this, &C, &IsMatching, &State](ArgIdxTy I, const Expr *E, SVal) {
-IsMatching |=
-PropSrcArgs.contains(I) && isTaintedOrPointsToTainted(E, State, C);
+IsMatching = IsMatching || (PropSrcArgs.contains(I) &&
+isTaintedOrPointsToTainted(E, State, C));
   });
 
   if (!IsMatching)
@@ -860,7 +862,7 @@
 
   SourceLocation DomLoc = Call.getArgExpr(0)->getExprLoc();
   StringRef DomName = C.getMacroNameOrSpelling(DomLoc);
-  // White list the internal communication protocols.
+  // Allow internal communication protocols.
   bool SafeProtocol = DomName.equals("AF_SYSTEM") ||
   DomName.equals("AF_LOCAL") || DomName.equals("AF_UNIX") 
||
   DomName.equals("AF_RESERVED_36");


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -52,7 +52,7 @@
 "Untrusted data is passed to a system call "
 "(CERT/STR02-C. Sanitize data passed to complex subsystems)";
 
-/// Check if tainted data is used as a buffer size ins strn.. functions,
+/// Check if tainted data is used as a buffer size in strn.. functions,
 /// and allocators.
 constexpr llvm::StringLiteral MsgTaintedBufferSize =
 "Untrusted data is used to specify the buffer size "
@@ -160,7 +160,8 @@
 public:
   ArgSet() = default;
   ArgSet(ArgVecTy &&DiscreteArgs, Optional VariadicIndex = None)
-  : DiscreteArgs(DiscreteArgs), VariadicIndex(VariadicIndex) {}
+  : DiscreteArgs(std::move(DiscreteArgs)),
+VariadicIndex(std::move(VariadicIndex)) {}
 
   bool contains(ArgIdxTy ArgIdx) const 

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

[Nathan James via Phabricator via cfe-commits]

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

Because StringMapEntrys have stable addresses, The IncludeBuckets can be 
changed to store pointers to entrys instead of the key used for accessing 
IncludeLocations.
This saves having to lookup the IncludeLocations map as well as creating 
unnecessary strings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117460

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -10,7 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_INCLUDESORTER_H
 
 #include "../ClangTidyCheck.h"
-#include 
 
 namespace clang {
 namespace tidy {
@@ -61,7 +60,8 @@
   /// Mapping from file name to #include locations.
   llvm::StringMap IncludeLocations;
   /// Includes sorted into buckets.
-  SmallVector IncludeBucket[IK_InvalidInclude];
+  SmallVector *, 1>
+  IncludeBucket[IK_InvalidInclude];
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -134,20 +134,21 @@
SourceLocation EndLocation) {
   int Offset = findNextLine(SourceMgr->getCharacterData(EndLocation));
 
+  auto &MapEntry = *IncludeLocations.try_emplace(FileName).first;
   // Record the relevant location information for this inclusion directive.
-  IncludeLocations[FileName].push_back(
+  MapEntry.getValue().push_back(
   SourceRange(HashLocation, EndLocation.getLocWithOffset(Offset)));
-  SourceLocations.push_back(IncludeLocations[FileName].back());
+  SourceLocations.push_back(MapEntry.getValue().back());
 
   // Stop if this inclusion is a duplicate.
-  if (IncludeLocations[FileName].size() > 1)
+  if (MapEntry.getValue().size() > 1)
 return;
 
   // Add the included file's name to the appropriate bucket.
   IncludeKinds Kind =
   determineIncludeKind(CanonicalFile, FileName, IsAngled, Style);
   if (Kind != IK_InvalidInclude)
-IncludeBucket[Kind].push_back(FileName.str());
+IncludeBucket[Kind].push_back(&MapEntry);
 }
 
 Optional IncludeSorter::createIncludeInsertion(StringRef FileName,
@@ -174,9 +175,11 @@
   determineIncludeKind(CanonicalFile, FileName, IsAngled, Style);
 
   if (!IncludeBucket[IncludeKind].empty()) {
-for (const std::string &IncludeEntry : IncludeBucket[IncludeKind]) {
+for (auto *IncludeMapEntry : IncludeBucket[IncludeKind]) {
+  StringRef IncludeEntry = IncludeMapEntry->getKey();
   if (compareHeaders(FileName, IncludeEntry, Style) < 0) {
-const auto &Location = IncludeLocations[IncludeEntry][0];
+
+const auto &Location = IncludeMapEntry->getValue().front();
 return FixItHint::CreateInsertion(Location.getBegin(), IncludeStmt);
   }
   if (FileName == IncludeEntry) {
@@ -185,8 +188,8 @@
 }
 // FileName comes after all include entries in bucket, insert it after
 // last.
-const std::string &LastInclude = IncludeBucket[IncludeKind].back();
-SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+SourceRange LastIncludeLocation =
+IncludeBucket[IncludeKind].back()->getValue().back();
 return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
   IncludeStmt);
   }
@@ -209,15 +212,15 @@
 
   if (NonEmptyKind < IncludeKind) {
 // Create a block after.
-const std::string &LastInclude = IncludeBucket[NonEmptyKind].back();
-SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+SourceRange LastIncludeLocation =
+IncludeBucket[NonEmptyKind].back()->getValue().back();
 IncludeStmt = '\n' + IncludeStmt;
 return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
   IncludeStmt);
   }
   // Create a block before.
-  const std::string &FirstInclude = IncludeBucket[NonEmptyKind][0];
-  SourceRange FirstIncludeLocation = IncludeLocations[FirstInclude].back();
+  SourceRange FirstIncludeLocation =
+  IncludeBucket[NonEmptyKind][0]->getValue().back();
   IncludeStmt.append("\n");
   return FixItHint::CreateInsertion(FirstIncludeLocation.getBegin(),
 IncludeStmt);


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

Applied typo and naming fixes, introduced 2 move operations, and re-introduced 
short circuiting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116025

___
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

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 abandoned this revision.
gamesh411 added a comment.

This is superseded by D116025 .


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] D117405: [AArch64] CodeGen for Armv8.8/9.3 MOPS

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

This looks like a large patch. It may be better split up into a base/isel part, 
a clang part and a global isel part (which may require different reviewers).




Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:836
+  // inside a bundle to prevent other passes to moving things in between.
+  MIBundleBuilder Bundler(MBB, MBBI);
+  auto &MF = *MBB.getParent();

It is probably better to expand the instruction later than to create a bundle.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:940
+  if (Subtarget->hasMOPS()) {
+// If we have MOPS, always use them
+MaxStoresPerMemsetOptSize = 0;

Are you sure we should _always_ be using them? I would expect a `ldr+str` to be 
better than a `mov #4; cpyfp; cpyfm; cpyfe`, for example.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11875
+Info.opc = ISD::INTRINSIC_W_CHAIN;
+Info.memVT = MVT::getVT(Val->getType());
+Info.ptrVal = Dst;

Can this safely set memVT to the size of a single element? That would need to 
be multiplied by the size of the memory being set, and if this is being used 
for aliasing info will be too small.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8350
+  let mayLoad = 1 in {
+def MOPSMemoryCopy : Pseudo<(outs GPR64common:$Rd_wb, GPR64common:$Rs_wb, 
GPR64:$Rn_wb),
+(ins GPR64common:$Rd, GPR64common:$Rs, 
GPR64:$Rn),

These need to be marked as 96bit pseudos. They should also clobber NZCV (as 
should the real instructions above, but that doesn't look like it was ever 
changed after D116157)



Comment at: llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp:28
+  uint64_t ConstSize = 0;
+  if (auto *C = dyn_cast(Size)) {
+ConstSize = cast(Size)->getZExtValue();

LLVM doesn't need brackets around a single statement.



Comment at: llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp:48
+}
+llvm_unreachable_internal("Unhandled MOPS ISD Opcode");
+return AArch64::INSTRUCTION_LIST_END;

llvm_unreachable is far more common than llvm_unreachable_internal. It can be 
moved into the default case too, then it doesn't need a return after it.



Comment at: llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp:53-54
+  MachineMemOperand::Flags Flags = MachineMemOperand::MOStore;
+  // if (!Temporal)
+  //   Flags |= MachineMemOperand::MONonTemporal;
+  if (isVolatile)

There is commented out code here.



Comment at: llvm/test/CodeGen/AArch64/aarch64-mops-mte.ll:3
+
+; RUN: llc %s -o - -mtriple=aarch64-arm-none-eabi -O2 -mattr=+mops,+mte  | 
FileCheck %s --check-prefix=SDAG
+; RUN: llc %s -o - -mtriple=aarch64-arm-none-eabi -O0 -global-isel=1 
-global-isel-abort=1 -mattr=+mops,+mte  | FileCheck %s --check-prefix=GISel

You likely don't need the -O2 and -O0 for llc run commands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117405

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

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

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

balazske added a comment.

I am not sure if it is correct to have replacement (fixit) for a part of the 
cases only. It is not possible to make a fixit for every use. In a case like 
`shared_ptr p1{new int}, p2{new int[10]};` the type can not be changed 
without other changes. For this reason the replacement is offered only for 
single-variable declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

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

balazske added a comment.

Is there a fast way to find single-variable field declaration?

  class A {
shared_ptr F1{new int}, F2{new int[10]};
  };

The `FieldDecl` does not contain information like `DeclStmt::isSingleDecl`. The 
only way looks like to check the source locations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Sam McCall via Phabricator via cfe-commits]

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

Sorry about letting this sit. I think this is good to go, with a naming tweak.

For the 14 cycle, it should be hidden: we may want to tweak the interactions 
with `didSave` reparsing etc, and there may be other unexpected wrinkles.

Regarding moving to config: we should do this, but there's some dumb 
bikeshedding of names (mostly: does this go at the top level or do we want a 
`Parsing` block or something, will other things fit in there, etc). So a hidden 
flag seems like the easiest thing for now.




Comment at: clang-tools-extra/clangd/ClangdServer.h:169
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyPreambles = false;
 

Naming (here and throughout). On reflection the use of "preamble" isn't ideal:

 - it mostly affects the preamble, but not exclusively: `#includes` in the 
non-preamble region also use this FS.
 - it's jargony, and if we want to put this in config we should use a 
friendlier name. Might as well find one now.

I'd suggest a boolean `UseDirtyHeaders` for now, and if we move it to config 
then something like `HeaderContents: Saved|Dirty`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

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

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

a drive by concern around possible flakiness of the interaction. have you 
checked how does it look like when the user is still forming the initializer? 
it might be annoying if their cursor kept jumping around while they're editing 
the (possibly half-formed) initializer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116786

___
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 inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+const auto &NextLine = *I[1];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)

HazardyKnusperkeks wrote:
> owenpan wrote:
> > I would move this line to just before handling empty record blocks below.
> I'd rather keep the definitions close together.
If it were just a simple initialization, it wouldn't matter much. However, it 
would be a bit wasteful as `PreviousLine` always gets computed here even if the 
function may return before the pointer would get chance to be used. If you 
really want to keep all related definitions together, wouldn't you want to move 
lines 214-215 up to right after line 211?



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:268-289
+[this, B = AnnotatedLines.begin(), &NextLine,
+ TheLine](SmallVectorImpl::const_iterator I) {
   if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
 return true;
   if (Style.AllowShortFunctionsOnASingleLine >=
   FormatStyle::SFS_Empty &&
+  NextLine.First->is(tok::r_brace))

HazardyKnusperkeks wrote:
> owenpan wrote:
> > I'd either leave this lambda alone or further simplify it as suggested here.
> I'm no fan of capturing everything, but some things I've applied.
`[&]` doesn't capture everything. It only captures the variables that are used 
in the lambda body. That being said, I'm okay with explicitly capturing each 
one.


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] D117456: [clangd] Avoid a code completion crash

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG884832407e47: [clangd] Avoid a code completion crash 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117456

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2474,6 +2474,26 @@
   )cpp");
 }
 
+TEST(CompletionTest, NoCrashOnMissingNewLineAtEOF) {
+  auto FooCpp = testPath("foo.cpp");
+
+  MockCompilationDatabase CDB;
+  MockFS FS;
+  Annotations F("#pragma ^ // no new line");
+  FS.Files[FooCpp] = F.code().str();
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+  runAddDocument(Server, FooCpp, F.code());
+  // Run completion outside the file range.
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, F.point(),
+   clangd::CodeCompleteOptions()))
+  .Completions,
+  IsEmpty());
+  EXPECT_THAT(cantFail(runSignatureHelp(Server, FooCpp, F.point(),
+MarkupKind::PlainText))
+  .signatures,
+  IsEmpty());
+}
+
 TEST(GuessCompletionPrefix, Filters) {
   for (llvm::StringRef Case : {
"[[scope::]][[ident]]^",
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -369,6 +369,10 @@
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -417,6 +421,10 @@
   return CB(error("Failed to parse includes"));
 
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  DocumentationFormat));


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2474,6 +2474,26 @@
   )cpp");
 }
 
+TEST(CompletionTest, NoCrashOnMissingNewLineAtEOF) {
+  auto FooCpp = testPath("foo.cpp");
+
+  MockCompilationDatabase CDB;
+  MockFS FS;
+  Annotations F("#pragma ^ // no new line");
+  FS.Files[FooCpp] = F.code().str();
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+  runAddDocument(Server, FooCpp, F.code());
+  // Run completion outside the file range.
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, F.point(),
+   clangd::CodeCompleteOptions()))
+  .Completions,
+  IsEmpty());
+  EXPECT_THAT(cantFail(runSignatureHelp(Server, FooCpp, F.point(),
+MarkupKind::PlainText))
+  .signatures,
+  IsEmpty());
+}
+
 TEST(GuessCompletionPrefix, Filters) {
   for (llvm::StringRef Case : {
"[[scope::]][[ident]]^",
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -369,6 +369,10 @@
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -417,6 +421,10 @@
   return CB(error("Failed to parse includes"));
 
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  

[clang-tools-extra] 8848324 - [clangd] Avoid a code completion crash

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-17T10:35:55+01:00
New Revision: 884832407e47579a28e8c363492a620901f2eab4

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

LOG: [clangd] Avoid a code completion crash

This is a workaround (adding a newline to the eof) in clangd to avoid the code
completion crash, see https://github.com/clangd/clangd/issues/332.

In principle, this is a clang bug, we should fix it in clang, but it is not
trivial.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 29d5d9dfe1d1..34fc71f70bb8 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -369,6 +369,10 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -417,6 +421,10 @@ void ClangdServer::signatureHelp(PathRef File, Position 
Pos,
   return CB(error("Failed to parse includes"));
 
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+// FIXME: Add traling new line if there is none at eof, workaround a crash,
+// see https://github.com/clangd/clangd/issues/332
+if (!IP->Contents.endswith("\n"))
+  ParseInput.Contents.append("\n");
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  DocumentationFormat));

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e3bf5ebdb0a7..c602627c3595 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2474,6 +2474,26 @@ TEST(CompletionTest, OverridesNonIdentName) {
   )cpp");
 }
 
+TEST(CompletionTest, NoCrashOnMissingNewLineAtEOF) {
+  auto FooCpp = testPath("foo.cpp");
+
+  MockCompilationDatabase CDB;
+  MockFS FS;
+  Annotations F("#pragma ^ // no new line");
+  FS.Files[FooCpp] = F.code().str();
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+  runAddDocument(Server, FooCpp, F.code());
+  // Run completion outside the file range.
+  EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, F.point(),
+   clangd::CodeCompleteOptions()))
+  .Completions,
+  IsEmpty());
+  EXPECT_THAT(cantFail(runSignatureHelp(Server, FooCpp, F.point(),
+MarkupKind::PlainText))
+  .signatures,
+  IsEmpty());
+}
+
 TEST(GuessCompletionPrefix, Filters) {
   for (llvm::StringRef Case : {
"[[scope::]][[ident]]^",



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

[Dmitri Gribenko via Phabricator via cfe-commits]

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



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

asoffer wrote:
> I think std::optional satisfies these requirements but should not be 
> considered a smart-pointer.
Are you objecting to naming, or to this code handling optionals and smart 
pointers in the same way at all?

An optional is not a pointer, however it has few semantic differences from 
`std::unique_ptr` that are relevant here (whether the object is heap-allocated 
or not is not relevant for building an access expression). So I think this code 
should handle optionals and other value wrappers like `llvm::Expected` and 
`absl::StatusOr`.



Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:221-222
+  isSmartPointerType(E->getType(), Context)) {
+// Strip off any operator->. This can only occur inside an actual arrow
+// member access, so we treat it as equivalent to an actual object
+// expression.





Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:234
+  if (const auto *Operand = isSmartDereference(*E, Context)) {
+// `buildDot` already handles the built-in dereference operator, so we
+// only need to catch overloaded `operator*`.





Comment at: clang/unittests/Tooling/SourceCodeBuildersTest.cpp:141
+  std::string Snippet = "std::unique_ptr P; P;";
+  auto StmtMatch = matchStmt(Snippet, expr(hasType(qualType().bind("ty";
+  ASSERT_TRUE(StmtMatch) << "Snippet: " << Snippet;

There are actually two expressions in the snippet above, one is the 
`DeclRefExpr` in `P;`, but the other is the `CXXConstructExpr` in 
`std::unique_ptr P;`. Both have the same type, so it makes no difference 
which one we find, but the snippet deliberately having `P;` makes it look like 
we specifically want the `DeclRefExpr`. I'd suggest changing the matcher to 
`declRefExpr()` for clarity. Same in tests below.



Comment at: clang/unittests/Tooling/SourceCodeBuildersTest.cpp:373
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointer) {
+  testBuilder(buildAccess, "Smart x; x;", "x->");
+}

This is a case where the old APIs provided the user with a choice, but the new 
API does not. If the user wanted to call a method on the smart pointer itself 
(e.g., `reset()`), they could have used `buildDot` to get `x.`.

IDK if it is an important use case, but I thought I'd mention it, since the new 
API is not 100% equivalent to the ones that it replaces.


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] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D95046#3247793 , @sammccall wrote:

> Sorry about letting this sit. I think this is good to go, with a naming tweak.
>
> For the 14 cycle, it should be hidden: we may want to tweak the interactions 
> with `didSave` reparsing etc, and there may be other unexpected wrinkles.
>
> Regarding moving to config: we should do this, but there's some dumb 
> bikeshedding of names (mostly: does this go at the top level or do we want a 
> `Parsing` block or something, will other things fit in there, etc). So a 
> hidden flag seems like the easiest thing for now.

I've been using this locally for months. The one nit i have is if you edit a 
header file then switch to a different file, diagnostics aren't updated until 
you force clangd to reparse the file (by editing it for example).
However I'm not sure of a nice way around this that doesn't involve trying to 
rebuild the world when you make an edit to a header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

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

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 400473.
njames93 added a comment.

Update name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -501,6 +501,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when building "
+   "preambles instead of reading from the disk"),
+  Hidden,
+  init(ClangdServer::Options().UseDirtyHeaders)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -938,6 +944,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.UseDirtyHeaders = UseDirtyHeaders;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {
 if (T.hidden() && !HiddenFeatures)
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -165,6 +165,8 @@
 bool FoldingRanges = false;
 
 FeatureModuleSet *FeatureModules = nullptr;
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyHeaders = false;
 
 explicit operator TUScheduler::Options() const;
   };
@@ -390,6 +392,9 @@
 private:
   FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
+  const ThreadsafeFS &getHeaderFS() const {
+return UseDirtyHeaders ? *DirtyFS : TFS;
+  }
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
@@ -409,6 +414,8 @@
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
+  bool UseDirtyHeaders = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap>
   CachedCompletionFuzzyFindRequestByFile;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -156,7 +156,7 @@
 : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot),
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
@@ -228,7 +228,7 @@
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.TFS = &TFS;
+  Inputs.TFS = &getHeaderFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
@@ -368,7 +368,7 @@
 SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -416,7 +416,7 @@
 if (!PreambleData)
   return CB(error("Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  DocumentationFormat));


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -501,6 +501,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when building "
+   "preambles instead of reading from the disk"),
+  Hidden,
+  init(ClangdServer::Options().UseDirtyHeaders)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -938,6 +944,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.UseDirt

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D117306#3247417 , 
@LegalizeAdulthood wrote:

> In D117306#3245915 , @njames93 
> wrote:
>
>> How does this check play with the `modernize-make-shared` check?
>
> Wouldn't it be orthogonal?
>
> This check looks for existing `make_shared` usages, whereas
> modernize-make-shared adds new `make_shared` usages from
> `new shared_ptr` usages.  I wouldn't expect `modernize-make-shared`
> to generate mismatched scalar/array `shared_ptr` instances.

This check looks for constructions of shared_ptr types from an array new 
expression. modernize-make-shared looks for constructions of shared_ptr types 
using the new expression, However I'm not sure how it handles the array version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D117461: [clangd] IncludeCleaner: Attach "insert prgama keep" fix-it to diagnostic

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Now that IWYU pragma: keep supresses the warning for unused header, it should
be possible to suggest the user both removing the header and adding a pragma to
keep it instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117461

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


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1613,7 +1613,7 @@
 
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
-$fix[[  $diag[[#include "unused.h"]]
+$remove[[  $diag[[#include "unused.h"]]$insert[[]]
 ]]
   #include "used.h"
 
@@ -1645,7 +1645,9 @@
   UnorderedElementsAre(AllOf(
   Diag(Test.range("diag"), "included header unused.h is not used"),
   WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
-  WithFix(Fix(Test.range("fix"), "", "remove #include directive");
+  WithFix(Fix(Test.range("remove"), "", "remove #include directive"),
+  Fix(Test.range("insert"), " // IWYU pragma: keep",
+  "insert IWYU pragma: keep");
   Cfg.Diagnostics.SuppressAll = true;
   WithContextValue SuppressAllWithCfg(Config::Key, std::move(Cfg));
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -205,6 +205,28 @@
   return Result;
 }
 
+// Returns the range starting right after the included header name and ending 
at
+// EOL.
+clangd::Range getIWYUPragmaRange(llvm::StringRef Code, unsigned HashOffset) {
+  clangd::Range Result;
+  Result.end = Result.start = offsetToPosition(Code, HashOffset);
+
+  unsigned QuotesCount = 0;
+  Result.start.character +=
+  lspLength(Code.drop_front(HashOffset).take_until([&QuotesCount](char C) {
+if (C == '"')
+  if (++QuotesCount == 2)
+return true;
+return C == '>';
+  })) +
+  1;
+  Result.end.character +=
+  lspLength(Code.drop_front(HashOffset).take_until([](char C) {
+return C == '\n' || C == '\r';
+  }));
+  return Result;
+}
+
 // Finds locations of macros referenced from within the main file. That 
includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
 void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
@@ -422,14 +444,23 @@
 // FIXME(kirillbobyrev): Removing inclusion might break the code if the
 // used headers are only reachable transitively through this one. Suggest
 // including them directly instead.
-// FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
-// (keep/export) remove the warning once we support IWYU pragmas.
 D.Fixes.emplace_back();
 D.Fixes.back().Message = "remove #include directive";
 D.Fixes.back().Edits.emplace_back();
 D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
 D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
 D.InsideMainFile = true;
+// Allow suppressing the warning through adding keep pragma.
+// NOTE: This would also erase the comment after the header inclusion if
+// there is one.
+D.Fixes.emplace_back();
+D.Fixes.back().Message = "insert IWYU pragma: keep";
+D.Fixes.back().Edits.emplace_back();
+D.Fixes.back().Edits.back().range =
+getIWYUPragmaRange(Code, Inc->HashOffset);
+D.InsideMainFile = true;
+D.Fixes.back().Edits.back().newText = " // IWYU pragma: keep";
+D.InsideMainFile = true;
 Result.push_back(std::move(D));
   }
   return Result;


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1613,7 +1613,7 @@
 
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
-$fix[[  $diag[[#include "unused.h"]]
+$remove[[  $diag[[#include "unused.h"]]$insert[[]]
 ]]
   #include "used.h"
 
@@ -1645,7 +1645,9 @@
   UnorderedElementsAre(AllOf(
   Diag(Test.range("diag"), "included header unused.h is not used"),
   WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
-  WithFix(Fix(Test.range("fix"), "", "remove #include directive");
+  WithFix(Fix(Test.range("remove"), "", "remove #include 

[clang-tools-extra] ab3f100 - Reland (2) "[AST] Add RParen loc for decltype AutoTypeloc.""

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-17T11:33:11+01:00
New Revision: ab3f100bec03d72ecee947a323c51698d4b95207

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

LOG: Reland (2) "[AST] Add RParen loc for decltype AutoTypeloc.""

The patch was reverted because it caused a crash during PCH build -- we
missed to update the RParenLoc in TreeTransform::TransformAutoType.

This relands 55d96ac and 37ec65e with a test and fix.

Added: 
clang/test/PCH/cxx14-decltype-auto.cpp

Modified: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
clang/include/clang/AST/TypeLoc.h
clang/lib/AST/TypeLoc.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaType.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/AST/ast-dump-template-decls-json.cpp
clang/test/AST/ast-dump-template-decls.cpp
clang/unittests/AST/SourceLocationTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
index d5c9fa7de811e..2b9907d162664 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
@@ -285,15 +285,12 @@ SourceRange 
UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange(
 return {};
   }
 
-  // If the return type is a constrained 'auto' or 'decltype(auto)', we need to
-  // include the tokens after the concept. Unfortunately, the source range of 
an
-  // AutoTypeLoc, if it is constrained, does not include the 'auto' or
-  // 'decltype(auto)'. If the return type is a plain 'decltype(...)', the
-  // source range only contains the first 'decltype' token.
+  // If the return type is a constrained 'auto', we need to include the token
+  // after the concept. Unfortunately, the source range of an AutoTypeLoc, if 
it
+  // is constrained, does not include the 'auto'.
+  // FIXME: fix the AutoTypeLoc location in clang.
   auto ATL = ReturnLoc.getAs();
-  if ((ATL && (ATL.isConstrained() ||
-   ATL.getAutoKeyword() == AutoTypeKeyword::DecltypeAuto)) ||
-  ReturnLoc.getAs()) {
+  if (ATL && ATL.isConstrained() && !ATL.isDecltypeAuto()) {
 SourceLocation End =
 expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
 SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
index 3776e1c3505d1..914564e9ae218 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -96,9 +96,7 @@ bool ExpandAutoType::prepare(const Selection& Inputs) {
   if (auto *Node = Inputs.ASTSelection.commonAncestor()) {
 if (auto *TypeNode = Node->ASTNode.get()) {
   if (const AutoTypeLoc Result = TypeNode->getAs()) {
-// Code in apply() does handle 'decltype(auto)' yet.
-if (!Result.getTypePtr()->isDecltypeAuto() &&
-!isStructuredBindingType(Node) &&
+if (!isStructuredBindingType(Node) &&
 !isDeducedAsLambda(Node, Result.getBeginLoc()) &&
 !isTemplateParam(Node))
   CachedLocation = Result;

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index b3a0847c2a4a8..a2b7071540405 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -399,6 +399,8 @@ TEST(SelectionTest, CommonAncestor) {
 )cpp",
   "DeclRefExpr"},
   {"[[decltype^(1)]] b;", "DecltypeTypeLoc"}, // Not the VarDecl.
+  // decltype(auto) is an AutoTypeLoc!
+  {"[[de^cltype(a^uto)]] a = 1;", "AutoTypeLoc"},
 
   // Objective-C nullability attributes.
   {

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
index 96574a67b5a46..6d9d4362be7af 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -71,7 +71,8 @@ TEST_F(ExpandAutoTypeTest, Test) {
   apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
   "void ns::Func() { ns::Class::Nested * x = new ns::Class::Nested{}; }");
 
-  EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, everyone!

I manage to figure out the cause, the crash was caused by an arbitrary RPareLoc 
-- we missed to set the RPareLoc in `TreeTransform::TransformAutoType`. I 
reland the patch in ab3f100bec03d72ecee947a323c51698d4b95207 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116919

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

[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117463

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -82,6 +82,11 @@
 
   ExtraArgs.push_back("-std=c++17");
   EXPECT_UNAVAILABLE("template  class Y;");
+  ExtraArgs.back() = "-std=c++20";
+  EXPECT_UNAVAILABLE(
+  R"cpp(template  concept C = true;
+^C a^uto abc();
+)cpp");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -96,7 +96,7 @@
   if (auto *Node = Inputs.ASTSelection.commonAncestor()) {
 if (auto *TypeNode = Node->ASTNode.get()) {
   if (const AutoTypeLoc Result = TypeNode->getAs()) {
-if (!isStructuredBindingType(Node) &&
+if (!isStructuredBindingType(Node) && !Result.isConstrained() &&
 !isDeducedAsLambda(Node, Result.getBeginLoc()) &&
 !isTemplateParam(Node))
   CachedLocation = Result;


Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -82,6 +82,11 @@
 
   ExtraArgs.push_back("-std=c++17");
   EXPECT_UNAVAILABLE("template  class Y;");
+  ExtraArgs.back() = "-std=c++20";
+  EXPECT_UNAVAILABLE(
+  R"cpp(template  concept C = true;
+^C a^uto abc();
+)cpp");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -96,7 +96,7 @@
   if (auto *Node = Inputs.ASTSelection.commonAncestor()) {
 if (auto *TypeNode = Node->ASTNode.get()) {
   if (const AutoTypeLoc Result = TypeNode->getAs()) {
-if (!isStructuredBindingType(Node) &&
+if (!isStructuredBindingType(Node) && !Result.isConstrained() &&
 !isDeducedAsLambda(Node, Result.getBeginLoc()) &&
 !isTemplateParam(Node))
   CachedLocation = Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D95046#3247844 , @njames93 wrote:

> In D95046#3247793 , @sammccall wrote:
>
>> Sorry about letting this sit. I think this is good to go, with a naming 
>> tweak.
>>
>> For the 14 cycle, it should be hidden: we may want to tweak the interactions 
>> with `didSave` reparsing etc, and there may be other unexpected wrinkles.
>>
>> Regarding moving to config: we should do this, but there's some dumb 
>> bikeshedding of names (mostly: does this go at the top level or do we want a 
>> `Parsing` block or something, will other things fit in there, etc). So a 
>> hidden flag seems like the easiest thing for now.
>
> I've been using this locally for months. The one nit i have is if you edit a 
> header file then switch to a different file, diagnostics aren't updated until 
> you force clangd to reparse the file (by editing it for example).

That's the only real interaction I can think of too.
The didSave trigger for rebuild will still trigger the rebuild too I think. It 
doesn't make a great deal of sense in this mode, but it may still be a helpful 
workflow.

> However I'm not sure of a nice way around this that doesn't involve trying to 
> rebuild the world when you make an edit to a header.

Agree, this is tricky.

Maybe one idea is a large global debounce: we reparse all files when a header 
is saved *or* e.g. 30 seconds have passed with no edits to any file.

Exact details could vary, I think important aspects are: rebuilds should not 
scale with rate of edits, prefer the extra work to happen when relatively idle, 
clangd should still eventually reach a steady state where it's doing no work 
until it receives requests.

In any case I'd prefer to land in its current simple form, and land any further 
bits after the branch cut (soon).




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:506
+  desc("Use files open in the editor when building "
+   "preambles instead of reading from the disk"),
+  Hidden,

Nit: when parsing headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

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

[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I'm missing some context on this patch. My intuition is that constrained auto 
is unlikely to be used in deducible contexts, but maybe some people will like 
`Iterator auto I = foo.begin()` or so...




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp:99
   if (const AutoTypeLoc Result = TypeNode->getAs()) {
-if (!isStructuredBindingType(Node) &&
+if (!isStructuredBindingType(Node) && !Result.isConstrained() &&
 !isDeducedAsLambda(Node, Result.getBeginLoc()) &&

Why? Is there a mechanical limitation (bad typeloc or so) or is this a policy 
decision?

If it's technical, we should link to a bug



Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+  R"cpp(template  concept C = true;
+^C a^uto abc();
+)cpp");

This shouldn't be unavailable because it's constrained, it should be 
unavailable because it's not deduced.

Does this case pass or fail today?

If the issue with constrained auto is syntactic, can you add a deductible test 
case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117463

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

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 400479.
njames93 added a comment.

Nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -492,6 +492,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when parsing "
+   "headers instead of reading from the disk"),
+  Hidden,
+  init(ClangdServer::Options().UseDirtyHeaders)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -928,6 +934,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.UseDirtyHeaders = UseDirtyHeaders;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {
 if (T.hidden() && !HiddenFeatures)
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -165,6 +165,8 @@
 bool FoldingRanges = false;
 
 FeatureModuleSet *FeatureModules = nullptr;
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyHeaders = false;
 
 explicit operator TUScheduler::Options() const;
   };
@@ -394,6 +396,9 @@
 private:
   FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
+  const ThreadsafeFS &getHeaderFS() const {
+return UseDirtyHeaders ? *DirtyFS : TFS;
+  }
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
@@ -413,6 +418,8 @@
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
+  bool UseDirtyHeaders = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap>
   CachedCompletionFuzzyFindRequestByFile;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -156,7 +156,7 @@
 : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot),
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
@@ -228,7 +228,7 @@
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.TFS = &TFS;
+  Inputs.TFS = &getHeaderFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
@@ -368,7 +368,7 @@
 SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))
@@ -420,7 +420,7 @@
 if (!PreambleData)
   return CB(error("Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -492,6 +492,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when parsing "
+   "headers instead of reading from the disk"),
+  Hidden,
+  init(ClangdServer::Options().UseDirtyHeaders)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -928,6 +934,7 @@
 ClangTidyOptProvider = com

[clang-tools-extra] 8b88ff0 - [clangd] Add option to use dirty file contents when building preambles.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-01-17T10:55:35Z
New Revision: 8b88ff08038c5525bdb5d22b050550ca3b34ad77

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

LOG: [clangd] Add option to use dirty file contents when building preambles.

Adds a option `use-dirty-preambles` to enable using unsaved in editor contents 
when building pre-ambles.
This enables a more seamless user experience when switching between header and 
implementation files and forgetting to save inbetween.
It's also in line with the LSP spec that states open files in the editor should 
be used instead of on the contents on disk - 
https://microsoft.github.io/language-server-protocol/overviews/lsp/overview/
For now the option is defaulted to off and hidden, Though I have a feeling it 
should be moved into the `.clangd` config and possibly defaulted to true.

Addresses https://github.com/clangd/clangd/issues/488

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 34fc71f70bb81..0760ed5317be8 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -156,7 +156,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase 
&CDB,
 : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot),
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
@@ -228,7 +228,7 @@ void ClangdServer::addDocument(PathRef File, 
llvm::StringRef Contents,
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.TFS = &TFS;
+  Inputs.TFS = &getHeaderFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
@@ -368,7 +368,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
 SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))
@@ -420,7 +420,7 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
 if (!PreambleData)
   return CB(error("Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 4bef653cbf82d..b6325fa567ac4 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -165,6 +165,8 @@ class ClangdServer {
 bool FoldingRanges = false;
 
 FeatureModuleSet *FeatureModules = nullptr;
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyHeaders = false;
 
 explicit operator TUScheduler::Options() const;
   };
@@ -394,6 +396,9 @@ class ClangdServer {
 private:
   FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
+  const ThreadsafeFS &getHeaderFS() const {
+return UseDirtyHeaders ? *DirtyFS : TFS;
+  }
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
@@ -413,6 +418,8 @@ class ClangdServer {
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
+  bool UseDirtyHeaders = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap>
   CachedCompletionFuzzyFindRequestByFile;

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index fc448c2c235ce..fa3c59de20fa6 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -492,6 +492,12 @@ opt EnableConfig{
 init(true),
 };
 
+opt Use

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

[Nathan James 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 rG8b88ff08038c: [clangd] Add option to use dirty file contents 
when building preambles. (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -492,6 +492,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when parsing "
+   "headers instead of reading from the disk"),
+  Hidden,
+  init(ClangdServer::Options().UseDirtyHeaders)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -928,6 +934,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.UseDirtyHeaders = UseDirtyHeaders;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {
 if (T.hidden() && !HiddenFeatures)
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -165,6 +165,8 @@
 bool FoldingRanges = false;
 
 FeatureModuleSet *FeatureModules = nullptr;
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyHeaders = false;
 
 explicit operator TUScheduler::Options() const;
   };
@@ -394,6 +396,9 @@
 private:
   FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
+  const ThreadsafeFS &getHeaderFS() const {
+return UseDirtyHeaders ? *DirtyFS : TFS;
+  }
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
@@ -413,6 +418,8 @@
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
+  bool UseDirtyHeaders = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap>
   CachedCompletionFuzzyFindRequestByFile;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -156,7 +156,7 @@
 : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot),
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
@@ -228,7 +228,7 @@
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.TFS = &TFS;
+  Inputs.TFS = &getHeaderFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
@@ -368,7 +368,7 @@
 SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))
@@ -420,7 +420,7 @@
 if (!PreambleData)
   return CB(error("Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
 // FIXME: Add traling new line if there is none at eof, workaround a crash,
 // see https://github.com/clangd/clangd/issues/332
 if (!IP->Contents.endswith("\n"))


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -492,6 +492,12 @@
 init(true),
 };
 
+opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
+  desc("Use files open in the editor when parsing "
+   "headers instead of reading from the disk"),
+  Hidden,
+  init(Clan

[PATCH] D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman, 
kbarton, xazax.hun, mgorny, nemanjai.
Eugene.Zelenko added reviewers: aaron.ballman, LegalizeAdulthood.
Eugene.Zelenko added a project: clang-tools-extra.
njames93 updated this revision to Diff 400365.
njames93 added a comment.
njames93 marked 10 inline comments as done.
njames93 updated this revision to Diff 400472.
njames93 marked 3 inline comments as done.
njames93 published this revision for review.
Herald added a subscriber: cfe-commits.

Update some includes


njames93 added a comment.

Address comments




Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:489
+  if (Context.hasIncludeInserter()) {
+auto Mapping = IncludeSorter::getStyleMapping();
+auto Index = 
static_cast(Context.getIncludeInserter().getStyle());

Please do not use `auto`, because type is not spelled explicitly in statement.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:434
+  if (Context.hasIncludeInserter())
+Context.getIncludeInserter().registerPreprocessor(PP);
+

Why not just `Context.registerIncludeInserter(PP)` and follow the Law of 
Demeter?

Let `Context` do the `hasIncludeInserter()` check itself.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:220-238
+llvm::Optional
+ClangTidyCheck::createMainFileIncludeInsertion(StringRef Include) {
+  if (!Context->hasIncludeInserter()) {
+// Only crash on debug builds
+assert(false && "No IncludeInserter registered");
+return llvm::None;
+  }

Law of Demeter.  Move this to `ClangTidyContext` and let it handle the details.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:220-238
+llvm::Optional
+ClangTidyCheck::createMainFileIncludeInsertion(StringRef Include) {
+  if (!Context->hasIncludeInserter()) {
+// Only crash on debug builds
+assert(false && "No IncludeInserter registered");
+return llvm::None;
+  }

LegalizeAdulthood wrote:
> Law of Demeter.  Move this to `ClangTidyContext` and let it handle the 
> details.
Are you suggesting I create methods for createIncludeInsertion in the 
ClangTidyContext?



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:420
+
+  llvm::Optional createMainFileIncludeInsertion(StringRef Include);
+  llvm::Optional createIncludeInsertion(FileID File,

`diag` has visibility public.  Any reason why these are visibility `protected`?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:219
   std::make_unique(*getOptions().WarningsAsErrors);
+  this->Inserter.reset();
 }

is `this->` necessary here?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:569
+  StringRef Closest;
+  auto Mapping = IncludeSorter::getStyleMapping();
+  for (unsigned I = 0, E = Mapping.size(); I < E; ++I) {

Ditto.



Comment at: clang-tools-extra/clang-tidy/IncludeInserter.h:20
 namespace tidy {
-namespace utils {
 

What's the guidance on what belongs in `clang::tidy`
and what belongs in `clang::tidy::utils`?



Comment at: clang-tools-extra/clang-tidy/IncludeInserter.h:20
 namespace tidy {
-namespace utils {
 

LegalizeAdulthood wrote:
> What's the guidance on what belongs in `clang::tidy`
> and what belongs in `clang::tidy::utils`?
Well as the file moved out of utils, its no longer in the utils namespace.



Comment at: clang-tools-extra/clang-tidy/IncludeSorter.h:14
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/StringMap.h"
 #include 

Since you introduced `ArrayRef` as a data type, I was expecting
the includes for those to be included instead of `StringMap`; are we really
using `StringMap` in this header?

"Include what you use" would seem to imply that we should be including:
- StringRef
- ArrayRef
- Optional
- FixItHint



Comment at: clang-tools-extra/clang-tidy/IncludeSorter.h:14
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/StringMap.h"
 #include 

LegalizeAdulthood wrote:
> Since you introduced `ArrayRef` as a data type, I was expecting
> the includes for those to be included instead of `StringMap`; are we really
> using `StringMap` in this header?
> 
> "Include what you use" would seem to imply that we should be including:
> - StringRef
> - ArrayRef
> - Optional
> - FixItHint
IWYU should strictly be followed but in practice throughout clang/llvm it never 
really is.
We are actually using StringMap in this header.
All I really did here was using clangd's include information fix-its to make it 
compile.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:432-433
 Preprocessor *PP = &Clang->getPreprocessor();
+if (CTContext->hasI

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

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

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

In D117306#3247859 , @njames93 wrote:

> In D117306#3247417 , 
> @LegalizeAdulthood wrote:
>
>> In D117306#3245915 , @njames93 
>> wrote:
>>
>>> How does this check play with the `modernize-make-shared` check?
>>
>> Wouldn't it be orthogonal?
>>
>> This check looks for existing `make_shared` usages, whereas
>> modernize-make-shared adds new `make_shared` usages from
>> `new shared_ptr` usages.  I wouldn't expect `modernize-make-shared`
>> to generate mismatched scalar/array `shared_ptr` instances.
>
> This check looks for constructions of shared_ptr types from an array new 
> expression. modernize-make-shared looks for constructions of shared_ptr types 
> using the new expression, However I'm not sure how it handles the array 
> version.

I found out that `make_shared` can not handle array at all (until C++20, but I 
do not plan to handle such new features). `modernize-make-shared` should not 
transform into `make_shared` if an array is created (if it works correct) but I 
do not see test cases for this.




Comment at: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:39
+  cxxConstructExpr(
+  hasDeclaration(UsedConstructor), argumentCountIs(1),
+  hasArgument(0, cxxNewExpr(isArray(), hasType(pointerType(pointee(

LegalizeAdulthood wrote:
> What about the other constructor overloads?
> Creating a `shared_ptr` with a deleter is quite common.
The intent is to find cases where no deleter is specified. If a deleter is used 
we can not tell in reliable way if it is correct for an array. The check 
assumes that a deleter is correct and needs no check.



Comment at: 
clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:104
+.getLocalSourceRange();
+D << TemplateArgumentRange;
+

LegalizeAdulthood wrote:
> I'm not sure what this is doing?
I think that this adds the additional range to the bug report. It is shown in 
the output (but I did not find a way how to check it in the test).



Comment at: 
clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:106
+
+if (isInSingleDeclStmt(VarOrField)) {
+  const SourceManager &SM = Ctx.getSourceManager();

LegalizeAdulthood wrote:
> Is this to guard against multiple variables being declared at once?
Yes, this is to check for "single variable declaration". But works only for 
variables (not fields).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D86546: [compiler-rt][builtins] Use explicitly-sized integer types for LibCalls

[Ayke via Phabricator via cfe-commits]

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

I've looked through the code again. It looks correct and (in past testing) it 
fixed a bug I had, so I'm going to accept this.

@atrosinenko do you have commit access? If not, I can commit it for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86546

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

[PATCH] D117468: [RISCV] Add intrinsic for Zbt extension

[Chenbing.Zheng via Phabricator via cfe-commits]

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

RV32: cmov, cmix, fsl, fsr, fsri

RV64: fsl, fsr, fsri, fslw, fsrw, fsriw


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117468

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbt.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbt.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/include/llvm/Target/TargetSelectionDAG.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
  llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll
@@ -0,0 +1,65 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zbt -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBT
+
+declare i32 @llvm.riscv.fsl.i32(i32, i32, i32)
+
+define i32 @fsl_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fslw a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsl.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.fsr.i32(i32, i32, i32)
+
+define i32 @fsr_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsrw a0, a1, a0, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+define i32 @fsri_i32(i32 %a, i32 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsriw a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 5)
+  ret i32 %1
+}
+
+declare i64 @llvm.riscv.fsl.i64(i64, i64, i64)
+
+define i64 @fsl_i64(i64 %a, i64 %b, i64 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsl a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsl.i64(i64 %a, i64 %b, i64 %c)
+  ret i64 %1
+}
+
+declare i64 @llvm.riscv.fsr.i64(i64, i64, i64)
+
+define i64 @fsr_i64(i64 %a, i64 %b, i64 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsr a0, a1, a0, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsr.i64(i64 %a, i64 %b, i64 %c)
+  ret i64 %1
+}
+
+define i64 @fsri_i64(i64 %a, i64 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsri a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsr.i64(i64 %a, i64 %b, i64 5)
+  ret i64 %1
+}
Index: llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zbt -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBT
+
+declare i32 @llvm.riscv.cmov.i32(i32 %a, i32 %b, i32 %c)
+
+define i32 @cmov(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: cmov:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:cmov a0, a2, a0, a1
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.cmov.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.cmix.i32(i32 %a, i32 %b, i32 %c)
+
+define i32 @cmix(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: cmix:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:cmix a0, a2, a0, a1
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.cmix.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.fsl.i32(i32, i32, i32)
+
+define i32 @fsl_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsl a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsl.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.fsr.i32(i32, i32, i32)
+
+define i32 @fsr_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

[Ayke via Phabricator via cfe-commits]

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

Looks good to me.




Comment at: compiler-rt/lib/builtins/fp_extend.h:32-39
 #if defined __LP64__
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
+return 32 + clzsi(a & REP_C(0x));

Perhaps more reliable would be the following:

```
#if ULONG_MAX == 0x
  return __builtin_clzl(a);
#elif ULLONG_MAX == 0x
  return __builtin_clzll(a);
#else
  #error Unsupported platform
#endif
```

This is what I've also used in int_types.h to detect clzsi etc. It probably 
would need to be tested on some more architectures though (32 and 64 bit).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86547

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

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

[Ayke via Phabricator via cfe-commits]

aykevl added inline comments.



Comment at: compiler-rt/lib/builtins/fp_extend.h:32-39
 #if defined __LP64__
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
+return 32 + clzsi(a & REP_C(0x));

aykevl wrote:
> Perhaps more reliable would be the following:
> 
> ```
> #if ULONG_MAX == 0x
>   return __builtin_clzl(a);
> #elif ULLONG_MAX == 0x
>   return __builtin_clzll(a);
> #else
>   #error Unsupported platform
> #endif
> ```
> 
> This is what I've also used in int_types.h to detect clzsi etc. It probably 
> would need to be tested on some more architectures though (32 and 64 bit).
In fact, it might be possible to use `__builtin_clzll` for all platforms (it 
maps to 'long long', which I think is 64-bit practically everywhere).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86547

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

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 400498.
gamesh411 added a comment.

All commits were exluded in the previous patch upload


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116025

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -22,15 +22,14 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "llvm/Support/YAMLTraits.h"
 
-#include 
 #include 
 #include 
-#include 
 #include 
 
 using namespace clang;
@@ -38,577 +37,651 @@
 using namespace taint;
 
 namespace {
-class GenericTaintChecker : public Checker {
-public:
-  static void *getTag() {
-static int Tag;
-return &Tag;
+
+class GenericTaintChecker;
+
+/// Check for CWE-134: Uncontrolled Format String.
+constexpr llvm::StringLiteral MsgUncontrolledFormatString =
+"Untrusted data is used as a format string "
+"(CWE-134: Uncontrolled Format String)";
+
+/// Check for:
+/// CERT/STR02-C. "Sanitize data passed to complex subsystems"
+/// CWE-78, "Failure to Sanitize Data into an OS Command"
+constexpr llvm::StringLiteral MsgSanitizeSystemArgs =
+"Untrusted data is passed to a system call "
+"(CERT/STR02-C. Sanitize data passed to complex subsystems)";
+
+/// Check if tainted data is used as a buffer size in strn.. functions,
+/// and allocators.
+constexpr llvm::StringLiteral MsgTaintedBufferSize =
+"Untrusted data is used to specify the buffer size "
+"(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
+"for character data and the null terminator)";
+
+/// Check if tainted data is used as a custom sink's parameter.
+constexpr llvm::StringLiteral MsgCustomSink =
+"Untrusted data is passed to a user-defined sink";
+
+using ArgIdxTy = int;
+using ArgVecTy = llvm::SmallVector;
+
+/// Denotes the return value.
+constexpr ArgIdxTy ReturnValueIndex{-1};
+
+static ArgIdxTy fromArgumentCount(unsigned Count) {
+  assert(Count <=
+ static_cast(std::numeric_limits::max()) &&
+ "ArgIdxTy is not large enough to represent the number of arguments.");
+  return Count;
+}
+
+/// Check if the region the expression evaluates to is the standard input,
+/// and thus, is tainted.
+/// FIXME: Move this to Taint.cpp.
+bool isStdin(SVal Val, const ASTContext &ACtx) {
+  // FIXME: What if Val is NonParamVarRegion?
+
+  // The region should be symbolic, we do not know it's value.
+  const auto *SymReg = dyn_cast_or_null(Val.getAsRegion());
+  if (!SymReg)
+return false;
+
+  // Get it's symbol and find the declaration region it's pointing to.
+  const auto *Sm = dyn_cast(SymReg->getSymbol());
+  if (!Sm)
+return false;
+  const auto *DeclReg = dyn_cast(Sm->getRegion());
+  if (!DeclReg)
+return false;
+
+  // This region corresponds to a declaration, find out if it's a global/extern
+  // variable named stdin with the proper type.
+  if (const auto *D = dyn_cast_or_null(DeclReg->getDecl())) {
+D = D->getCanonicalDecl();
+// FIXME: This should look for an exact match.
+if (D->getName().contains("stdin") && D->isExternC()) {
+  const QualType FILETy = ACtx.getFILEType().getCanonicalType();
+  const QualType Ty = D->getType().getCanonicalType();
+
+  if (Ty->isPointerType())
+return Ty->getPointeeType() == FILETy;
+}
   }
+  return false;
+}
 
-  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
-  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+SVal getPointeeOf(const CheckerContext &C, Loc LValue) {
+  const QualType ArgTy = LValue.getType(C.getASTContext());
+  if (!ArgTy->isPointerType() || !ArgTy->getPointeeType()->isVoidType())
+return C.getState()->getSVal(LValue);
 
-  void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
-  const char *Sep) const override;
+  // Do not dereference void pointers. Treat them as byte pointers instead.
+  // FIXME: we might want to consider more than just the first byte.
+  return C.getState()->getSVal(LValue, C.getASTContext().CharTy);
+}
+
+/// Given a pointer/reference argument, return the value it refers to.
+Optional getPointeeOf(const CheckerContext &C, SVal Arg) {
+  if (auto LValue = Arg.getAs())
+return ge

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D116919#3247887 , @hokein wrote:

> Thanks, everyone!
>
> I manage to figure out the cause, the crash was caused by an arbitrary 
> RPareLoc -- we missed to set the RPareLoc in 
> `TreeTransform::TransformAutoType`. I reland the patch in 
> ab3f100bec03d72ecee947a323c51698d4b95207 
> .

I rebuilt my full, non-reduced testcase now, and it seems like it built fine 
this time. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116919

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

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

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

steakhal accepted this revision.
steakhal added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116025

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

[PATCH] D116521: [CMake] Factor out config prefix finding logic

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added inline comments.



Comment at: llvm/CMakeLists.txt:209
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )

Hi, adding this module path overwrites the `llvm_check_linker_flag` from 
`llvm/cmake/modules/LLVMCheckLinkerFlag.cmake` with the one defined in 
`cmake/Modules/CheckLinkerFlag.cmake`, which does not check the linker but the 
compiler.
This breaks our gcc+ld build because ld does not support `--color-diagnostics`.

Our downstream issue is here: 
https://github.com/GPUOpen-Drivers/llpc/issues/1645


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116521

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This appears to have triggered some buildbot failures:

https://lab.llvm.org/buildbot/#/builders/5/builds/17461/steps/10/logs/stdio

e.g. in path-mappings.test:

  "message": "Unknown argument: '-enable-noundef-analysis'",

This flag isn't being added by clangd code, I'm not sure where it's coming 
from... maybe somewhere in driver?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: vitalybuka.
sammccall added a comment.

Ah, this seems to be coming from the buildbot configuration.

https://github.com/llvm/llvm-zorg/blob/38ab06456f7302b4eab53e5b6f1e2eaf4f127132/zorg/buildbot/builders/sanitizers/buildbot_functions.sh#L140-L149

@vitalybuka looks like sanitizer-buildbot7 might need a config update for this 
change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D71026: Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

[Alexander Richardson via Phabricator via cfe-commits]

arichardson abandoned this revision.
arichardson added a comment.

I believe this is no longer necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71026

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

[PATCH] D116919: [AST] Add RParen loc for decltype AutoTypeloc.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D116919#3248113 , @mstorsjo wrote:

> In D116919#3247887 , @hokein wrote:
>
>> Thanks, everyone!
>>
>> I manage to figure out the cause, the crash was caused by an arbitrary 
>> RPareLoc -- we missed to set the RPareLoc in 
>> `TreeTransform::TransformAutoType`. I reland the patch in 
>> ab3f100bec03d72ecee947a323c51698d4b95207 
>> .
>
> I rebuilt my full, non-reduced testcase now, and it seems like it built fine 
> this time. Thanks!

Glad to hear, and thanks for the verification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116919

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

[PATCH] D117472: [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Haojian Wu via Phabricator via cfe-commits]

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

The early-claim hack was removed in 96f5cc1ee417f863f85756d1e56b1bed1bd76a7e 
,
we see a regression about captured var-decl in lambda, see 
https://github.com/clangd/clangd/issues/990.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117472

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -509,6 +509,13 @@
 enum Bar : [[Fo^o]];
   )cpp",
"TypedefTypeLoc"},
+
+  // lambda captured var-decl
+  {R"cpp(
+void test(int bar) {
+  auto l = [^[[foo = bar]]] { };
+})cpp",
+   "VarDecl"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -716,13 +716,16 @@
   }
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
+  // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
+SourceRange Early = earlySourceRange(Node);
 dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
 Nodes.back().Selected = NoTokens;
 Stack.push(&Nodes.back());
+claimRange(Early, Nodes.back().Selected);
   }
 
   // Pops a node off the ancestor stack, and finalizes it. Pairs with push().
@@ -744,6 +747,25 @@
 Stack.pop();
   }
 
+  // Returns the range of tokens that this node will claim directly, and
+  // is not available to the node's children.
+  // Usually empty, but sometimes children cover tokens but shouldn't own them.
+  SourceRange earlySourceRange(const DynTypedNode &N) {
+if (const Decl *D = N.get()) {
+  // We want the name in the var-decl to be claimed by the decl itself and
+  // not by any children. Ususally, we don't need this, because source
+  // ranges of children are not overlapped with their parent's.
+  // But there are some AST-weird cases, e.g.
+  //auto fun = [bar = foo]() { ... }
+  //~   VarDecl
+  //~~~ |- AutoTypeLoc
+  if (const auto *DD = llvm::dyn_cast(D))
+return DD->getLocation();
+}
+
+return SourceRange();
+  }
+
   // Claim tokens for N, after processing its children.
   // By default this claims all unclaimed tokens in getSourceRange().
   // We override this if we want to claim fewer tokens (e.g. there are gaps).


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -509,6 +509,13 @@
 enum Bar : [[Fo^o]];
   )cpp",
"TypedefTypeLoc"},
+
+  // lambda captured var-decl
+  {R"cpp(
+void test(int bar) {
+  auto l = [^[[foo = bar]]] { };
+})cpp",
+   "VarDecl"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -716,13 +716,16 @@
   }
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
+  // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
+SourceRange Early = earlySourceRange(Node);
 dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
 Nodes.back().Selected = NoTokens;
 Stack.push(&Nodes.back());
+claimRange(Early, Nodes.back().Selected);
   }
 
   // Pops a node off the ancestor stack, and finalizes it. Pairs with push().
@@ -744,6 +747,25 @@
 Stack.pop();
   }
 
+  // Returns the range of tokens that this node will claim directly, and
+  // is not available to the node's children.
+  // Usually empty, but sometimes children cover tokens but shouldn't own them.
+  SourceRange earlySourceRange(const DynTypedNode &N) {
+if (const Decl *D = N.get()) {
+  // We want the name in the var-decl to be claimed by the decl itself and
+  // not by any child

[clang-tools-extra] 192f8d9 - [clangd] Don't rename on symbols from system headers.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-17T15:08:53+01:00
New Revision: 192f8d97002f13ab5a74ee11c4d5559b5ca693a8

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

LOG: [clangd] Don't rename on symbols from system headers.

Fixes https://github.com/clangd/clangd/issues/963.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index e092c677c57cf..a00d2f51b56c0 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -159,13 +159,17 @@ llvm::DenseSet locateDeclAt(ParsedAST 
&AST,
   return Result;
 }
 
-// By default, we exclude C++ standard symbols and protobuf symbols as rename
-// these symbols would change system/generated files which are unlikely to be
-// modified.
+// By default, we exclude symbols from system headers and protobuf symbols as
+// renaming these symbols would change system/generated files which are 
unlikely
+// to be good candidates for modification.
 bool isExcluded(const NamedDecl &RenameDecl) {
-  if (isProtoFile(RenameDecl.getLocation(),
-  RenameDecl.getASTContext().getSourceManager()))
+  const auto &SM = RenameDecl.getASTContext().getSourceManager();
+  if (SM.isInSystemHeader(RenameDecl.getLocation()))
 return true;
+  if (isProtoFile(RenameDecl.getLocation(), SM))
+return true;
+  // FIXME: Remove this std symbol list, as they should be covered by the
+  // above isInSystemHeader check.
   static const auto *StdSymbols = new llvm::DenseSet({
 #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
 #include "StdSymbolMap.inc"

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp 
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 26b3d7ad1c6c0..e3de3e7411c78 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1198,6 +1198,29 @@ TEST(RenameTest, MainFileReferencesOnly) {
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+  R"cpp(
+#include 
+SystemSym^bol abc;
+)cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["system"] = R"cpp(
+class SystemSymbol {};
+)cpp";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+  llvm::StringRef NewName = "abcde";
+
+  auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results) << "expected rename returned an error: " << 
Code.code();
+  auto ActualMessage = llvm::toString(Results.takeError());
+  EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind"));
+}
+
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());



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

[PATCH] D116643: [clangd] Don't rename on symbols from system headers.

[Haojian Wu 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 rG192f8d97002f: [clangd] Don't rename on symbols from 
system headers. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D116643?vs=398589&id=400510#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116643

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1198,6 +1198,29 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+  R"cpp(
+#include 
+SystemSym^bol abc;
+)cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["system"] = R"cpp(
+class SystemSymbol {};
+)cpp";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+  llvm::StringRef NewName = "abcde";
+
+  auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results) << "expected rename returned an error: " << 
Code.code();
+  auto ActualMessage = llvm::toString(Results.takeError());
+  EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind"));
+}
+
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -159,13 +159,17 @@
   return Result;
 }
 
-// By default, we exclude C++ standard symbols and protobuf symbols as rename
-// these symbols would change system/generated files which are unlikely to be
-// modified.
+// By default, we exclude symbols from system headers and protobuf symbols as
+// renaming these symbols would change system/generated files which are 
unlikely
+// to be good candidates for modification.
 bool isExcluded(const NamedDecl &RenameDecl) {
-  if (isProtoFile(RenameDecl.getLocation(),
-  RenameDecl.getASTContext().getSourceManager()))
+  const auto &SM = RenameDecl.getASTContext().getSourceManager();
+  if (SM.isInSystemHeader(RenameDecl.getLocation()))
 return true;
+  if (isProtoFile(RenameDecl.getLocation(), SM))
+return true;
+  // FIXME: Remove this std symbol list, as they should be covered by the
+  // above isInSystemHeader check.
   static const auto *StdSymbols = new llvm::DenseSet({
 #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
 #include "StdSymbolMap.inc"


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1198,6 +1198,29 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+  R"cpp(
+#include 
+SystemSym^bol abc;
+)cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["system"] = R"cpp(
+class SystemSymbol {};
+)cpp";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+  llvm::StringRef NewName = "abcde";
+
+  auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results) << "expected rename returned an error: " << Code.code();
+  auto ActualMessage = llvm::toString(Results.takeError());
+  EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind"));
+}
+
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -159,13 +159,17 @@
   return Result;
 }
 
-// By default, we exclude C++ standard symbols and protobuf symbols as rename
-// these symbols would change system/generated files which are unlikely to be
-// modified.
+// By default, we exclude symbols from system headers and protobuf symbols as
+// renaming these symbols would change system/generated files which are unlikely
+// to be good candidates for modification.
 bool isExcluded(const NamedDecl &RenameDecl) {
-  if (isProtoFile(RenameDecl.getLocation(),
-

[PATCH] D117472: [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Selection.cpp:758
+  // ranges of children are not overlapped with their parent's.
+  // But there are some AST-weird cases, e.g.
+  //auto fun = [bar = foo]() { ... }

I don't think this is an e.g., but rather is suitable for specifically this 
case.

If there are other cases, we likely should be testing DeclaratorDecl here, and 
excluding Constructor/Destructor as before.



Comment at: clang-tools-extra/clangd/Selection.cpp:773
   void claimTokensFor(const DynTypedNode &N, SelectionTree::Selection &Result) 
{
 // CXXConstructExpr often shows implicit construction, like `string s;`.
 // Don't associate any tokens with it unless there's some syntax like {}.

I wonder whether we want to keep the CXXConstructExpr hack now, up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117472

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

[PATCH] D117475: [clangd] NFC, emit source ranges in selection debug messages.

[Haojian Wu via Phabricator via cfe-commits]

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

It will make the output more versbose, but I found that these are useful
information when debugging selection tree.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117475

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


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -465,11 +465,12 @@
 }
 
 #ifndef NDEBUG
-std::string printNodeToString(const DynTypedNode &N, const PrintingPolicy &PP) 
{
+std::string printNodeToString(const DynTypedNode &N, const PrintingPolicy &PP,
+  const SourceManager &SM) {
   std::string S;
   llvm::raw_string_ostream OS(S);
   printNodeKind(OS, N);
-  OS << " ";
+  OS << " " << N.getSourceRange().printToString(SM) << " ";
   return std::move(OS.str());
 }
 #endif
@@ -699,7 +700,7 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
+  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy, SM), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
   return true;
 }
@@ -717,7 +718,7 @@
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
   void push(DynTypedNode Node) {
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy, SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
@@ -729,7 +730,8 @@
   // Performs primary hit detection.
   void pop() {
 Node &N = *Stack.top();
-dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
+dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy, SM),
+ indent(-1));
 claimTokensFor(N.ASTNode, N.Selected);
 if (N.Selected == NoTokens)
   N.Selected = SelectionTree::Unselected;


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -465,11 +465,12 @@
 }
 
 #ifndef NDEBUG
-std::string printNodeToString(const DynTypedNode &N, const PrintingPolicy &PP) {
+std::string printNodeToString(const DynTypedNode &N, const PrintingPolicy &PP,
+  const SourceManager &SM) {
   std::string S;
   llvm::raw_string_ostream OS(S);
   printNodeKind(OS, N);
-  OS << " ";
+  OS << " " << N.getSourceRange().printToString(SM) << " ";
   return std::move(OS.str());
 }
 #endif
@@ -699,7 +700,7 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
+  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy, SM), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
   return true;
 }
@@ -717,7 +718,7 @@
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
   void push(DynTypedNode Node) {
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy, SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
@@ -729,7 +730,8 @@
   // Performs primary hit detection.
   void pop() {
 Node &N = *Stack.top();
-dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
+dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy, SM),
+ indent(-1));
 claimTokensFor(N.ASTNode, N.Selected);
 if (N.Selected == NoTokens)
   N.Selected = SelectionTree::Unselected;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116748: [AArch64][ARM][Clang] PerfMon Extension Added

[Mubashar Ahmad via Phabricator via cfe-commits]

mubashar_ updated this revision to Diff 400518.
mubashar_ marked an inline comment as done.

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

https://reviews.llvm.org/D116748

Files:
  clang/test/Driver/aarch64-perfmon.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1428,17 +1428,14 @@
 
 TEST(TargetParserTest, AArch64ExtensionFeatures) {
   std::vector Extensions = {
-AArch64::AEK_CRC,  AArch64::AEK_CRYPTO,
-AArch64::AEK_FP,   AArch64::AEK_SIMD,
-AArch64::AEK_FP16, AArch64::AEK_PROFILE,
-AArch64::AEK_RAS,  AArch64::AEK_LSE,
-AArch64::AEK_RDM,  AArch64::AEK_DOTPROD,
-AArch64::AEK_SVE,  AArch64::AEK_SVE2,
-AArch64::AEK_SVE2AES,  AArch64::AEK_SVE2SM4,
-AArch64::AEK_SVE2SHA3, AArch64::AEK_SVE2BITPERM,
-AArch64::AEK_RCPC, AArch64::AEK_FP16FML,
-AArch64::AEK_SME,  AArch64::AEK_SMEF64,
-AArch64::AEK_SMEI64 };
+  AArch64::AEK_CRC, AArch64::AEK_CRYPTO,  AArch64::AEK_FP,
+  AArch64::AEK_SIMD,AArch64::AEK_FP16,AArch64::AEK_PROFILE,
+  AArch64::AEK_RAS, AArch64::AEK_LSE, AArch64::AEK_RDM,
+  AArch64::AEK_DOTPROD, AArch64::AEK_SVE, AArch64::AEK_SVE2,
+  AArch64::AEK_SVE2AES, AArch64::AEK_SVE2SM4, AArch64::AEK_SVE2SHA3,
+  AArch64::AEK_SVE2BITPERM, AArch64::AEK_RCPC,AArch64::AEK_FP16FML,
+  AArch64::AEK_SME, AArch64::AEK_SMEF64,  AArch64::AEK_SMEI64,
+  AArch64::AEK_PERFMON};
 
   std::vector Features;
 
@@ -1473,6 +1470,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+sme"));
   EXPECT_TRUE(llvm::is_contained(Features, "+sme-f64"));
   EXPECT_TRUE(llvm::is_contained(Features, "+sme-i64"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+perfmon"));
 }
 
 TEST(TargetParserTest, AArch64ArchFeatures) {
@@ -1520,6 +1518,7 @@
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
   {"mops", "nomops", "+mops", "-mops"},
+  {"pmuv3", "nopmuv3", "+perfmon", "-perfmon"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -118,6 +118,8 @@
 Features.push_back("+hbc");
   if (Extensions & AArch64::AEK_MOPS)
 Features.push_back("+mops");
+  if (Extensions & AArch64::AEK_PERFMON)
+Features.push_back("+perfmon");
 
   return true;
 }
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -71,6 +71,7 @@
   AEK_SMEI64 =  1ULL << 39,
   AEK_HBC = 1ULL << 40,
   AEK_MOPS =1ULL << 41,
+  AEK_PERFMON = 1ULL << 42,
 };
 
 enum class ArchKind {
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -146,6 +146,7 @@
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64",  "-sme-i64")
 AARCH64_ARCH_EXT_NAME("hbc",  AArch64::AEK_HBC, "+hbc",  "-hbc")
 AARCH64_ARCH_EXT_NAME("mops", AArch64::AEK_MOPS,"+mops", "-mops")
+AARCH64_ARCH_EXT_NAME("pmuv3",AArch64::AEK_PERFMON, "+perfmon",  "-perfmon")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME
Index: clang/test/Driver/aarch64-perfmon.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-perfmon.c
@@ -0,0 +1,13 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.4a+pmuv3 %s 2>&1 | FileCheck --check-prefix=CHECK-PERFMON %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.2a+pmuv3 %s 2>&1 | FileCheck --check-prefix=CHECK-PERFMON %s
+// CHECK-PERFMON: "-target-feature" "+perfmon"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.4a+nopmuv3 %s 2>&1 | FileCheck --check-prefix=CHECK-NOPERFMON %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.2a+nopmuv3 %s 2>&1 | FileCheck --check-prefix=CHECK-NOPERFMON %s
+// CHECK-NOPERFMON: "-target-feature" "-perfmon"
+
+// RUN: %clang -### -target aarch64-none-none-eabi %s 2>&1 | FileCheck %s --check-prefix=ABSENTPERFMON
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.4a %s 2>&1 | FileCheck %s --check-prefix=ABSENTPERFM

[clang-tools-extra] 4dedd82 - Re-land [clangd] Elide even more checks in SelectionTree.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-17T15:26:28+01:00
New Revision: 4dedd82cc99341d757a9cc07a8b7b22c8bb61d19

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

LOG: Re-land [clangd] Elide even more checks in SelectionTree.

This reverts commit 1093b9f2e9842982d97534940a643e3a4657c60b.

Fix added for implicit-include case.

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 011e24ee70b2..da80e01a926c 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -274,22 +274,37 @@ class SelectionTester {
 for (unsigned I = 0; I < Sel.size(); ++I) {
   if (shouldIgnore(Sel[I]) || PPIgnored[I])
 continue;
-  SpelledTokens.emplace_back();
-  Tok &S = SpelledTokens.back();
+  SelectedSpelled.emplace_back();
+  Tok &S = SelectedSpelled.back();
   S.Offset = SM.getFileOffset(Sel[I].location());
   if (S.Offset >= SelBegin && S.Offset + Sel[I].length() <= SelEnd)
 S.Selected = SelectionTree::Complete;
   else
 S.Selected = SelectionTree::Partial;
 }
+MaybeSelectedExpanded = computeMaybeSelectedExpandedTokens(Buf);
   }
 
   // Test whether a consecutive range of tokens is selected.
   // The tokens are taken from the expanded token stream.
   SelectionTree::Selection
   test(llvm::ArrayRef ExpandedTokens) const {
-if (SpelledTokens.empty())
+if (ExpandedTokens.empty())
   return NoTokens;
+if (SelectedSpelled.empty())
+  return SelectionTree::Unselected;
+// Cheap (pointer) check whether any of the tokens could touch selection.
+// In most cases, the node's overall source range touches ExpandedTokens,
+// or we would have failed mayHit(). However now we're only considering
+// the *unclaimed* spans of expanded tokens.
+// This is a significant performance improvement when a lot of nodes
+// surround the selection, including when generated by macros.
+if (MaybeSelectedExpanded.empty() ||
+&ExpandedTokens.front() > &MaybeSelectedExpanded.back() ||
+&ExpandedTokens.back() < &MaybeSelectedExpanded.front()) {
+  return SelectionTree::Unselected;
+}
+
 SelectionTree::Selection Result = NoTokens;
 while (!ExpandedTokens.empty()) {
   // Take consecutive tokens from the same context together for efficiency.
@@ -312,14 +327,14 @@ class SelectionTester {
   // If it returns false, test() will return NoTokens or Unselected.
   // If it returns true, test() may return any value.
   bool mayHit(SourceRange R) const {
-if (SpelledTokens.empty())
+if (SelectedSpelled.empty() || MaybeSelectedExpanded.empty())
   return false;
 // If the node starts after the selection ends, it is not selected.
 // Tokens a macro location might claim are >= its expansion start.
 // So if the expansion start > last selected token, we can prune it.
 // (This is particularly helpful for GTest's TEST macro).
 if (auto B = offsetInSelFile(getExpansionStart(R.getBegin(
-  if (*B > SpelledTokens.back().Offset)
+  if (*B > SelectedSpelled.back().Offset)
 return false;
 // If the node ends before the selection begins, it is not selected.
 SourceLocation EndLoc = R.getEnd();
@@ -328,12 +343,73 @@ class SelectionTester {
 // In the rare case that the expansion range is a char range, EndLoc is
 // ~one token too far to the right. We may fail to prune, that's OK.
 if (auto E = offsetInSelFile(EndLoc))
-  if (*E < SpelledTokens.front().Offset)
+  if (*E < SelectedSpelled.front().Offset)
 return false;
 return true;
   }
 
 private:
+  // Plausible expanded tokens that might be affected by the selection.
+  // This is an overestimate, it may contain tokens that are not selected.
+  // The point is to allow cheap pruning in test()
+  llvm::ArrayRef
+  computeMaybeSelectedExpandedTokens(const syntax::TokenBuffer &Toks) {
+if (SelectedSpelled.empty())
+  return {};
+
+auto LastAffectedToken = [&](SourceLocation Loc) {
+  auto Offset = offsetInSelFile(Loc);
+  while (Loc.isValid() && !Offset) {
+Loc = Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getEnd()
+  : SM.getIncludeLoc(SM.getFileID(Loc));
+Offset = offsetInSelFile(Loc);
+  }
+  return Offset;
+};
+auto FirstAffectedToken = [&](SourceLocation Loc) {
+  auto Offset = offsetInSelFile(Loc);
+  while (Loc.isValid() && !Offset) {
+Loc = Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getBegin()
+  : S