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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I actually thought you were on the right lines with your change to 
parseAccessSpecifier() surely if you see "private:"  "public:" or "protected:" 
but not `public::` that's when you call parseAccessSpecifier()

I think mostly I believe clang-format is a 96% done deal in terms of code, a 
lot of what we do here in my view is about closing those final "corner cases", 
sometimes our fixes are quite specific,

Something in the back of my head says this is a `sledge hammer to crack a nut` 
but that is just my view.. I'm just a little uncomfortable here with this 
change although it may open doors for other C specific changes, I'm not 
convinced this solves the problem I just think it moves it a little and maybe 
adds to complexity for what is likely a very small number of cases.




Comment at: clang/lib/Format/TokenAnnotator.cpp:3238
spaceRequiredBeforeParens(Right);
-  if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+  if (Style.Language != FormatStyle::LK_C &&
+  Left.isOneOf(tok::kw_new, tok::kw_delete))

I wonder how many people using C do this?

```
#define new(x) malloc(x)
#define delete(x) free(x)
```

Some at least

https://github.com/jbf/tonsai-scheme/blob/83d2a022f60fc17f9913c017fbde29e19cdf5b2b/src/memory.h



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:100
   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() ||

isC() these large OR/AND conditions become unreadable



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1221
   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() ||

isC()



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1606
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();

I feel everywhere you put isCpp you'll end up doing isCpp() || is C()

This goes back to the arguments for a review I tried before {D80079} its really 
isCStyle() if we WERE going to pursue this, I'd want that and not have clauses 
added everywhere



Comment at: clang/tools/clang-format/ClangFormat.cpp:124
 
+static cl::opt
+SetLanguage("set-language",

Is this required for this patch or is this a nice to have, can we have a 
separate review for this, it may have merit in its own right but it needs it 
own tests



Comment at: clang/tools/clang-format/ClangFormat.cpp:467
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+

I don't understand what you are doing here or why? is this just for C it feels 
wrong



Comment at: clang/tools/clang-format/ClangFormat.cpp:469
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;

we don't use braces on single line if

wouldn't you use a tolower or toupper here?


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] D117407: [clang] Add include path for cppwinrt on Windows SDK 10.0.17134+

[Hans Wennborg 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 rG9c195bae31c4: [clang] Add include path for cppwinrt on 
Windows SDK 10.0.17134+ (authored by saschanaz, committed by hans).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117407

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-sysroot.cpp


Index: clang/test/Driver/cl-sysroot.cpp
===
--- clang/test/Driver/cl-sysroot.cpp
+++ clang/test/Driver/cl-sysroot.cpp
@@ -17,6 +17,7 @@
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}winrt"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}cppwinrt"
 
 // CHECK: "-libpath:[[ROOT]]{{/|}}DIA SDK{{/|}}lib{{/|}}amd64"
 // CHECK: 
"-libpath:[[ROOT]]{{/|}}VC{{/|}}Tools{{/|}}MSVC{{/|}}27.1828.18284{{/|}}lib{{/|}}x64"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1333,6 +1333,15 @@
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include", windowsSDKIncludeVersion,
   "winrt");
+if (major >= 10) {
+  llvm::VersionTuple Tuple;
+  if (!Tuple.tryParse(windowsSDKIncludeVersion) &&
+  Tuple.getSubminor().getValueOr(0) >= 17134) {
+AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+  "Include", windowsSDKIncludeVersion,
+  "cppwinrt");
+  }
+}
   } else {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include");


Index: clang/test/Driver/cl-sysroot.cpp
===
--- clang/test/Driver/cl-sysroot.cpp
+++ clang/test/Driver/cl-sysroot.cpp
@@ -17,6 +17,7 @@
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}winrt"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}cppwinrt"
 
 // CHECK: "-libpath:[[ROOT]]{{/|}}DIA SDK{{/|}}lib{{/|}}amd64"
 // CHECK: "-libpath:[[ROOT]]{{/|}}VC{{/|}}Tools{{/|}}MSVC{{/|}}27.1828.18284{{/|}}lib{{/|}}x64"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1333,6 +1333,15 @@
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include", windowsSDKIncludeVersion,
   "winrt");
+if (major >= 10) {
+  llvm::VersionTuple Tuple;
+  if (!Tuple.tryParse(windowsSDKIncludeVersion) &&
+  Tuple.getSubminor().getValueOr(0) >= 17134) {
+AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+  "Include", windowsSDKIncludeVersion,
+  "cppwinrt");
+  }
+}
   } else {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9c195ba - [clang] Add include path for cppwinrt on Windows SDK 10.0.17134+

[Hans Wennborg via cfe-commits]

Author: Kagami Sascha Rosylight
Date: 2022-01-18T09:14:23+01:00
New Revision: 9c195bae31c4eefc3e5360cefb4f601388a4f6d9

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

LOG: [clang] Add include path for cppwinrt on Windows SDK 10.0.17134+

This fixes https://github.com/llvm/llvm-project/issues/53112 by adding
cppwinrt to the include path when the SDK version is higher than
10.0.17134.0.

Differential revision: https://reviews.llvm.org/D117407

Added: 


Modified: 
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-sysroot.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index 66e9d8ab525a..4e15c3ab51ce 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1333,6 +1333,15 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const 
ArgList &DriverArgs,
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include", windowsSDKIncludeVersion,
   "winrt");
+if (major >= 10) {
+  llvm::VersionTuple Tuple;
+  if (!Tuple.tryParse(windowsSDKIncludeVersion) &&
+  Tuple.getSubminor().getValueOr(0) >= 17134) {
+AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+  "Include", windowsSDKIncludeVersion,
+  "cppwinrt");
+  }
+}
   } else {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include");

diff  --git a/clang/test/Driver/cl-sysroot.cpp 
b/clang/test/Driver/cl-sysroot.cpp
index eb701f28514d..4db213a918a0 100644
--- a/clang/test/Driver/cl-sysroot.cpp
+++ b/clang/test/Driver/cl-sysroot.cpp
@@ -17,6 +17,7 @@
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}winrt"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}cppwinrt"
 
 // CHECK: "-libpath:[[ROOT]]{{/|}}DIA SDK{{/|}}lib{{/|}}amd64"
 // CHECK: 
"-libpath:[[ROOT]]{{/|}}VC{{/|}}Tools{{/|}}MSVC{{/|}}27.1828.18284{{/|}}lib{{/|}}x64"



___
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 added a comment.

For me the push back on the second language and having to pass it on the 
command line for the .h case  is about VS Code, Vim, Visual Studio, Emacs 
having to recognize what language they are in and having to pass the 
-language=XXX flag, now we are pushing the interpretation of what language we 
are to them and not to us, which mean we are now out of control.

Each editor will make different assessments as to what language it is (likely 
only 98% correctly) and we'll get the blame when they say its C# and not C 
because they made the wrong decision

Personally, If we are going to add `C` as a language we need something that can 
control it without external intervention

something like..

  // clang-format language='C' 

or something that reads the header (like we used to have in Makefile)

  ---*- C++ -*-===//

especially if there is a sudo standard for that, and uses that as a language 
hint to say this is a "C" .h header

But I think it might be a mistake for us to rely on the editors to provide us 
that, and even adding the option could open a tsunami of incorrect language 
issues which we cannot solve.


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] D117500: [clang] Mention MS on-demand TLS initialization in release notes

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117500

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

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

[praveen velliengiri via Phabricator via cfe-commits]

pvellien updated this revision to Diff 400750.
pvellien added a comment.

Removed amdgpu-asan-noprintf.cu and added amdgpu-asan-printf.cu testcase.


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

https://reviews.llvm.org/D116216

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/amdgpu-asan-printf.cu


Index: clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -O3 -x hip | FileCheck -check-prefixes=MFCHECK %s
+
+// MFCHECK: !llvm.module.flags = !{![[FLAG1:[0-9]+]], ![[FLAG2:[0-9]+]]}
+// MFCHECK: ![[FLAG1]] = !{i32 4, !"amdgpu_hostcall", i32 1}
+
+// Test to check hostcall module flag metadata is generated correctly
+// when a program has printf call and compiled with -fsanitize=address.
+#include "Inputs/cuda.h"
+__device__ void non_kernel() {
+  printf("sanitized device function");
+}
+
+__global__ void kernel() {
+  non_kernel();
+}
+
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -565,7 +565,9 @@
 "__amdgpu_device_library_preserve_asan_functions_ptr", nullptr,
 llvm::GlobalVariable::NotThreadLocal);
 addCompilerUsedGlobal(Var);
-getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+if (!getModule().getModuleFlag("amdgpu_hostcall")) {
+  getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+}
   }
 
   emitLLVMUsed();


Index: clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -O3 -x hip | FileCheck -check-prefixes=MFCHECK %s
+
+// MFCHECK: !llvm.module.flags = !{![[FLAG1:[0-9]+]], ![[FLAG2:[0-9]+]]}
+// MFCHECK: ![[FLAG1]] = !{i32 4, !"amdgpu_hostcall", i32 1}
+
+// Test to check hostcall module flag metadata is generated correctly
+// when a program has printf call and compiled with -fsanitize=address.
+#include "Inputs/cuda.h"
+__device__ void non_kernel() {
+  printf("sanitized device function");
+}
+
+__global__ void kernel() {
+  non_kernel();
+}
+
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -565,7 +565,9 @@
 "__amdgpu_device_library_preserve_asan_functions_ptr", nullptr,
 llvm::GlobalVariable::NotThreadLocal);
 addCompilerUsedGlobal(Var);
-getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+if (!getModule().getModuleFlag("amdgpu_hostcall")) {
+  getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+}
   }
 
   emitLLVMUsed();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117491: [clangd] Remove redundant check for renamed symbol origin

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117491

___
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 added a comment.

Isn't the reality that developers using 
`delete/new/try/catch/interface/public/protected/private/async/var/let` as 
variables the real problem? I'm slightly averse to pandering to them, and I 
definitely don't want to make my world more complex just to accommodate their 
poor choice of variable name.

I know if someone used any of those words as variables in LLVM surely we'd tell 
them to change it. Honestly how much of a problem are we really talking about 
here that can't be handled by a few corner cases.


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] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:44
+  if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) ||
   (Style.isJavaScript() && CurrentToken->TokenText == "function"))
 return true;

I still wonder if this can't be CurrentToken->is(JSKeywords.kw_function)


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

https://reviews.llvm.org/D117520

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

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

[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 working on this!

Do you have commit rights or do you need that someone lands it for you?
If you need help, please indicate your name and email to be used for the commit.

Also, if you think contributing more patches, have a look at 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.




Comment at: clang/lib/Format/FormatTokenLexer.cpp:433-446
   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< <>".

I hate the naming here.
Unless I'm mistaken we have: Fourth, First + 0, First + 1, First + 2.
But I'll clean it up at some time.


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

https://reviews.llvm.org/D117398

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:9468
"typename ();");
   verifyFormat("auto a =\n"
"new 
(aa(aaa))\n"

you doing something in your editor...



Comment at: clang/unittests/Format/FormatTest.cpp:9380
 
   verifyFormat("struct f {\n"
"  template \n"

just make sure there isn't a whitespace change here you didn't mean


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

https://reviews.llvm.org/D117398

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

run git clang-format before submitting (if making the patch from staged files)


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

https://reviews.llvm.org/D117398

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

[clang-tools-extra] 2d9198c - [clangd] Remove redundant check for renamed symbol origin

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2022-01-18T09:43:53+01:00
New Revision: 2d9198cec994b91fc13ef6cdd6983c61aaa1f726

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

LOG: [clangd] Remove redundant check for renamed symbol origin

This is a follow-up on D116643. `isInSystemHeader` check already detects
symbols coming from the Standard Library, so searching for the qualified name
in StdSymbolMap.inc is no longer necessary.

The tests filtering out purely based on the symbol qualified names are removed.

Reviewed By: hokein

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

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 a00d2f51b56c0..b106664f0a446 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -164,18 +164,8 @@ llvm::DenseSet locateDeclAt(ParsedAST 
&AST,
 // to be good candidates for modification.
 bool isExcluded(const NamedDecl &RenameDecl) {
   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"
-#undef SYMBOL
-  });
-  return StdSymbols->count(printQualifiedName(RenameDecl));
+  return SM.isInSystemHeader(RenameDecl.getLocation()) ||
+ isProtoFile(RenameDecl.getLocation(), SM);
 }
 
 enum class ReasonToReject {

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp 
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index e3de3e7411c78..70b0bbc8f073a 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -880,21 +880,6 @@ TEST(RenameTest, Renameable) {
 void f(X x) {x+^+;})cpp",
   "no symbol", HeaderFile},
 
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- class str^ing {};
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- inline namespace __u {
- class str^ing {};
- }
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-
   {R"cpp(// disallow rename on non-normal identifiers.
  @interface Foo {}
  -(int) fo^o:(int)x; // Token is an identifier, but declaration name 
isn't a simple identifier.
@@ -1199,11 +1184,14 @@ TEST(RenameTest, MainFileReferencesOnly) {
 }
 
 TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
-  // filter out references not from main file.
   llvm::StringRef Test =
   R"cpp(
+#include 
 #include 
+
 SystemSym^bol abc;
+
+void foo() { at^oi("9000"); }
 )cpp";
 
   Annotations Code(Test);
@@ -1211,14 +1199,22 @@ TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
   TU.AdditionalFiles["system"] = R"cpp(
 class SystemSymbol {};
 )cpp";
+  TU.AdditionalFiles["cstdlib"] = R"cpp(
+int atoi(const char *str);
+)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"));
+  // Clangd will not allow renaming symbols from the system headers for
+  // correctness.
+  for (auto &Point : Code.points()) {
+auto Results = rename({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) {



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

[PATCH] D117491: [clangd] Remove redundant check for renamed symbol origin

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2d9198cec994: [clangd] Remove redundant check for renamed 
symbol origin (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117491

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
@@ -880,21 +880,6 @@
 void f(X x) {x+^+;})cpp",
   "no symbol", HeaderFile},
 
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- class str^ing {};
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- inline namespace __u {
- class str^ing {};
- }
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-
   {R"cpp(// disallow rename on non-normal identifiers.
  @interface Foo {}
  -(int) fo^o:(int)x; // Token is an identifier, but declaration name 
isn't a simple identifier.
@@ -1199,11 +1184,14 @@
 }
 
 TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
-  // filter out references not from main file.
   llvm::StringRef Test =
   R"cpp(
+#include 
 #include 
+
 SystemSym^bol abc;
+
+void foo() { at^oi("9000"); }
 )cpp";
 
   Annotations Code(Test);
@@ -1211,14 +1199,22 @@
   TU.AdditionalFiles["system"] = R"cpp(
 class SystemSymbol {};
 )cpp";
+  TU.AdditionalFiles["cstdlib"] = R"cpp(
+int atoi(const char *str);
+)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"));
+  // Clangd will not allow renaming symbols from the system headers for
+  // correctness.
+  for (auto &Point : Code.points()) {
+auto Results = rename({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) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -164,18 +164,8 @@
 // to be good candidates for modification.
 bool isExcluded(const NamedDecl &RenameDecl) {
   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"
-#undef SYMBOL
-  });
-  return StdSymbols->count(printQualifiedName(RenameDecl));
+  return SM.isInSystemHeader(RenameDecl.getLocation()) ||
+ isProtoFile(RenameDecl.getLocation(), SM);
 }
 
 enum class ReasonToReject {


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -880,21 +880,6 @@
 void f(X x) {x+^+;})cpp",
   "no symbol", HeaderFile},
 
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- class str^ing {};
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-  {R"cpp(// disallow rename on excluded symbols (e.g. std symbols)
- namespace std {
- inline namespace __u {
- class str^ing {};
- }
- }
-   )cpp",
-   "not a supported kind", !HeaderFile},
-
   {R"cpp(// disallow rename on non-normal identifiers.
  @interface Foo {}
  -(int) fo^o:(int)x; // Token is an identifier, but declaration name isn't a simple identifier.
@@ -1199,11 +1184,14 @@
 }
 
 TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
-  // filter out references not from main file.
   llvm::StringRef Test

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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

As MyDeveloperDay justly pointed out, adding a new language enum LK_C doesn't 
seem to be the way to go, as you mostly duplicate all the conditions (probably 
missing some of them).
For the usability, it would be really painful to specify the language for each 
file that should be considered as C.
So, sorry for this forth and back, but now at least we see that your initial 
approach was actually better (but please add tests!).

> We'd maybe need to add C as language option and let the user specify the 
> language (-x c?).
> That in turn may be painful (because not automatic).

Yeah, I should have been more explicit here.




Comment at: .arclint:15-16
   }
-}
+}
\ No newline at end of file


Please don't change unrelated files.


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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

I think this LGTM


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] D117472: [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Haojian Wu via Phabricator via cfe-commits]

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

refine the comment.


Repository:
  rG LLVM Github Monorepo

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

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
@@ -516,6 +516,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
@@ -794,13 +794,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().
@@ -822,6 +825,26 @@
 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.
+  // An exception is lambda captured var decl, where AutoTypeLoc is
+  // overlapped with the name loc.
+  //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
@@ -516,6 +516,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
@@ -794,13 +794,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().
@@ -822,6 +825,26 @@
 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.
+  // An exception is lambda captured var decl, where AutoTypeLoc is
+  // overlapped with the name loc.
+  //auto fun = [bar = foo]() { ... }
+  //   

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: .arclint:15-16
   }
-}
+}
\ No newline at end of file


curdeius wrote:
> Please don't change unrelated files.
I don't know about others but I personally don't use arcanist...

I stage my files, make a patch and upload manually

```
git add   (or `git ls files -m | xargs git add` )
git clang-format 
git diff --cached -U4 > patch_to_submitt.diff

```
I use the Create Diff/Update Diff functionality via the web interface (I feel 
that gives me more control)

{F21699407, size=full}

{F21699405, size=full}

Just my 2c


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] D117472: [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Haojian Wu via Phabricator via cfe-commits]

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



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]() { ... }

sammccall wrote:
> 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.
refined the comment.

Testing DeclaratorDecl is safer, but we miss opportunities to know other cases 
(hopefully, there should be no other cases). For now, I'm leaning towards 
merely handling this specific case. If it turns out more cases, we can 
definitely use the DeclaratorDecl.



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 {}.

sammccall wrote:
> I wonder whether we want to keep the CXXConstructExpr hack now, up to you.
if we remove it, than we need to add the `CXXCtorInitializer` case back to the 
earlySourceRange :(


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

[clang-tools-extra] fd598e1 - [clangd] Bring back early-claim approach to fix a selection-tree regression.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-18T10:22:26+01:00
New Revision: fd598e185972f76a50c45e1402ab3b0fd70664b9

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

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

The early-claim hack was removed in 96f5cc1ee417f863f85756d1e56b1bed1bd76a7e,
we see a regression about captured var-decl in lambda.

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

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

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 da80e01a926c..69e99a9a8e28 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -794,13 +794,16 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   }
 
   // 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().
@@ -822,6 +825,26 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 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.
+  // An exception is lambda captured var decl, where AutoTypeLoc is
+  // overlapped with the name loc.
+  //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).

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index c4050b2fc2ba..30e0ff0e41b4 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -516,6 +516,13 @@ TEST(SelectionTest, CommonAncestor) {
 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) {



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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rGfd598e185972: [clangd] Bring back early-claim approach to 
fix a selection-tree regression. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

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
@@ -516,6 +516,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
@@ -794,13 +794,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().
@@ -822,6 +825,26 @@
 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.
+  // An exception is lambda captured var decl, where AutoTypeLoc is
+  // overlapped with the name loc.
+  //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
@@ -516,6 +516,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
@@ -794,13 +794,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().
@@ -822,6 +825,26 @@
 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 ar

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/FormatTokenLexer.cpp:433-446
   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< <>".

curdeius wrote:
> I hate the naming here.
> Unless I'm mistaken we have: Fourth, First + 0, First + 1, First + 2.
> But I'll clean it up at some time.
Me too. Maybe changing `First` to point at `Fourth` and get rid of `Fourth`, 
which has a different level of indirection than `First` now. I would probably 
get rid of `FourthTokenIsLess` as well.


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

https://reviews.llvm.org/D117398

___
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 updated this revision to Diff 400768.
Chenbing.Zheng edited the summary of this revision.
Chenbing.Zheng added a comment.

del cmov，cmix，fsri   in clang


Repository:
  rG LLVM Github Monorepo

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

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/lib/Target/RISCV/RISCVISelLowering.cpp
  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,34 @@
+; 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.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:fsr 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:fsri a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 5)
+  ret i32 %1
+}
Index: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
@@ -897,6 +897,8 @@
   (FSL GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
 def : Pat<(riscv_fsr GPR:$rs3, GPR:$rs1, GPR:$rs2),
   (FSR GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+def : Pat<(riscv_fsr GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
+  (FSRI GPR:$rs1, GPR:$rs3, uimmlog2xlen:$shamt)>;
 
 def : Pat<(fshr GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
   (FSRI GPR:$rs1, GPR:$rs3, uimmlog2xlen:$shamt)>;
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -4242,6 +4242,12 @@
   case Intrinsic::riscv_

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

[Chenbing.Zheng via Phabricator via cfe-commits]

Chenbing.Zheng marked 2 inline comments as done.
Chenbing.Zheng added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:907
   (FSR GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+def : Pat<(riscv_fsr GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
+  (FSRI GPR:$rs1, GPR:$rs3, uimmlog2xlen:$shamt)>;

craig.topper wrote:
> Need to also handle fsl with a constant argument.
Emm but there is no fsli in zbt. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Could you explain more in details the before/after of the fixed bugs, please?
The best would be to develop the patch description.




Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:37
+  auto LikelyDefinition = [this](const AnnotatedLine *Line,
+ const bool ExcludeEnum = false) {
 if ((Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) ||

Nit and personal preference.



Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:146
+  // Handling the case that opening bracket has its own line, with checking
+  // whether the last line already had an opening bracket to guard against
+  // misrecognition.

I find "bracket" ambiguous.


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

https://reviews.llvm.org/D117520

___
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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: .arclint:15-16
   }
-}
+}
\ No newline at end of file


MyDeveloperDay wrote:
> curdeius wrote:
> > Please don't change unrelated files.
> I don't know about others but I personally don't use arcanist...
> 
> I stage my files, make a patch and upload manually
> 
> ```
> git add   (or `git ls files -m | xargs git add` )
> git clang-format 
> git diff --cached -U4 > patch_to_submitt.diff
> 
> ```
> I use the Create Diff/Update Diff functionality via the web interface (I feel 
> that gives me more control)
> 
> {F21699407, size=full}
> 
> {F21699405, size=full}
> 
> Just my 2c
I do, but I never had to modify `.arclint` or whatever config file arcanist 
uses.
It just works as a charm.

Create and send a patch (if sending all changes not in main branch):
```
arc diff main --reviewers ...
```
Update a patch:
```
arc diff main --update --revision D123456789
```


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] D117549: [clangd] Sort targets before printing for tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Targets are not necessarily inserted in the order they appear in source
code. For example we could traverse overload sets, or selectively insert
template patterns after all other decls.
So order the targets before printing to make sure tests are not dependent on
such implementation details. We can also do it in production, but that might be
wasteful as we haven't seen any complaints in the wild around these orderings
yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117549

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1284,10 +1284,6 @@
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
 // Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,6 +33,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
@@ -1170,6 +1171,13 @@
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
   bool First = true;
+  if (!R.Targets.empty()) {
+const auto &SM = R.Targets.front()->getASTContext().getSourceManager();
+llvm::sort(R.Targets, [&SM](const NamedDecl *LHS, const NamedDecl *RHS) {
+  return SM.isBeforeInTranslationUnit(LHS->getLocation(),
+  RHS->getLocation());
+});
+  }
   for (const NamedDecl *T : R.Targets) {
 if (!First)
   OS << ", ";


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1284,10 +1284,6 @@
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
 // Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,6 +33,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
@@ -1170,6 +1171,13 @@
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
   bool First = true;
+  if (!R.Targets.empty()) {
+const auto &SM = R.Targets.front()->getASTContext().getSourceManager();
+llvm::sort(R.Targets, [&SM](const NamedDecl *LHS, const NamedDecl *RHS) {
+  return SM.isBeforeInTranslationUnit(LHS->getLocation(),
+  RHS->getLocation());
+});
+  }
   for (const NamedDecl *T : R.Targets) {
 if (!First)
   OS << ", ";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I see indeed that this change fixes the superlinear complexity when formatting 
code from stdin:

  cat test.cpp | time clang-format --style=LLVM
  
  0:01.44 ==   1.44s  //  9 returns
  0:13.17 ==  13s // 10 returns
  1:56.63 == 116s // 11 returns
  
  time clang-format --style=LLVM test.cpp
  0.03s-0.05s

And after this change the stdin case takes as much time as with a file.

It's definitely a good change, but I'd like to understand what's the reason, 
why stdin is handled so much differently?
Have you investigated this?

Also, a sort of performance test (e.g. a test with a timeout) would be nice to 
have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114837

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

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

[Jino Park via Phabricator via cfe-commits]

pjessesco updated this revision to Diff 400782.
pjessesco added a comment.

Fix revision after running `git clang-format`.

Sorry for annoying, I thought I did not make any changes at that line. :(

name : Jino Park
email : pjesse...@gmail.com


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
@@ -9463,6 +9463,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
@@ -3346,6 +3346,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
@@ -9463,6 +9463,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
@@ -3346,6 +3346,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] D117496: [clang][dataflow] Add transfer function for addrof

[Stanislav Gatev via Phabricator via cfe-commits]

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



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

xazax.hun wrote:
> 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?
In the first example you shared the type of the `LValueToRValue` expression is 
`int *` and the type of the  `UO_Deref` expression is `int` so I think we need 
to do something to translate one into the other. My understanding is that 
`LValueToRValue` performs indirection through references whereas `UO_Deref` 
performs indirection through pointers. This is why I think we ultimately need 
both. I might be wrong so please correct me and I'd be happy to address this in 
a follow up.


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] D117535: [clang-tidy] Force LF newlines when writing files

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

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

LGTM FWIW.
I might even wonder why this code is using `io.open` instead of just plain 
`open`. (I didn't know `io.open` was a thing; StackOverflow tells me that it 
was something in Python2 but that in Python3 it's a synonym for `open`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117535

___
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 400790.
gamesh411 added a comment.

Remove explicit template keyword for MSVC compatibility


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 

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

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

balazske updated this revision to Diff 400791.
balazske added a comment.

- Moved to bugprone module.
- Introduced a common base class, it is possible to add similar check for 
unique_ptr.
- Documentation changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
@@ -45,6 +45,8 @@
 }
 
 void basic() {
+  std::shared_ptr Pt1(new int[22]);
+
   std::shared_ptr P1 = std::shared_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: std::shared_ptr P1 = std::make_shared();
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp
@@ -0,0 +1,93 @@
+// RUN: %check_clang_tidy %s bugprone-shared-ptr-array-mismatch %t
+
+namespace std {
+
+template 
+struct shared_ptr {
+  template 
+  explicit shared_ptr(Y *) {}
+  template 
+  shared_ptr(Y *, Deleter) {}
+};
+
+} // namespace std
+
+struct A {};
+
+void f1() {
+  std::shared_ptr P1{new int};
+  std::shared_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P2{new int[10]};
+  std::shared_ptr<  int  > P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<  int[]  > P3{new int[10]};
+  std::shared_ptr P4(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P4(new int[10]);
+  new std::shared_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  std::shared_ptr P5(new int[10]);
+  std::shared_ptr P6(new int[10], [](const int *Ptr) {});
+}
+
+void f2() {
+  std::shared_ptr P1(new A);
+  std::shared_ptr P2(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P2(new A[10]);
+  std::shared_ptr P3(new A[10]);
+}
+
+void f3() {
+  std::shared_ptr P1{new int}, P2{new int[10]}, P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+struct S {
+  std::shared_ptr P1;
+  std::shared_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  std::shared_ptr P3{new int}, P4{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  S() : P1{new int[10]} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+};
+
+void f_parm(std::shared_ptr);
+
+void f4() {
+  f_parm(std::shared_ptr{new int[10]});
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+std::shared_ptr f_ret() {
+  return std::shared_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+template 
+void f_tmpl() {
+  std::shared_ptr P1{new T[10]};
+}
+
+void f5() {
+  f_tmpl();

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

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

balazske added a comment.

Possible improvement: Add the check for `reset` too.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:48
 void basic() {
+  std::shared_ptr Pt1(new int[22]);
+

This change was not intentional (should be removed).


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

[clang] 59e031f - [clang][dataflow] Add transfer function for addrof

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-18T11:23:08Z
New Revision: 59e031ff9057b103c73f22bebc32304ee79fa139

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

LOG: [clang][dataflow] Add transfer function for addrof

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: xazax.hun

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 6ae4573bbc05f..0979f55c5a5c8 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -174,19 +174,45 @@ class TransferVisitor : public 
ConstStmtVisitor {
   }
 
   void VisitUnaryOperator(const UnaryOperator *S) {
-if (S->getOpcode() == UO_Deref) {
-  assert(S->getSubExpr() != nullptr);
+// The CFG does not contain `ParenExpr` as top-level statements in basic
+// blocks, however sub-expressions can still be of that type.
+assert(S->getSubExpr() != nullptr);
+const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+assert(SubExpr != nullptr);
+
+switch (S->getOpcode()) {
+case UO_Deref: {
   const auto *SubExprVal = cast_or_null(
-  Env.getValue(*S->getSubExpr(), SkipPast::Reference));
+  Env.getValue(*SubExpr, SkipPast::Reference));
   if (SubExprVal == nullptr)
-return;
+break;
 
   auto &Loc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, Loc);
   Env.setValue(Loc, Env.takeOwnership(std::make_unique(
 SubExprVal->getPointeeLoc(;
+  break;
+}
+case UO_AddrOf: {
+  // Do not form a pointer to a reference. If `SubExpr` is assigned a
+  // `ReferenceValue` then form a value that points to the location of its
+  // pointee.
+  StorageLocation *PointeeLoc =
+  Env.getStorageLocation(*SubExpr, SkipPast::Reference);
+  if (PointeeLoc == nullptr)
+break;
+
+  auto &PointerLoc = Env.createStorageLocation(*S);
+  auto &PointerVal =
+  Env.takeOwnership(std::make_unique(*PointeeLoc));
+  Env.setStorageLocation(*S, PointerLoc);
+  Env.setValue(PointerLoc, PointerVal);
+  break;
+}
+default:
+  // FIXME: Add support for UO_LNot.
+  break;
 }
-// FIXME: Add support for UO_AddrOf, UO_LNot.
   }
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 7dfb916c7060f..ea035ba013daf 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1699,4 +1699,63 @@ TEST_F(TransferTest, StaticCast) {
   });
 }
 
+TEST_F(TransferTest, AddrOfValue) {
+  std::string Code = R"(
+void target() {
+  int Foo;
+  int *Bar = &Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+  });
+}
+
+TEST_F(TransferTest, AddrOfReference) {
+  std::string Code = R"(
+void target(int *Foo) {
+  int *Bar = &(*Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+  

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

[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.
sgatev marked an inline comment as done.
Closed by commit rG59e031ff9057: [clang][dataflow] Add transfer function for 
addrof (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117496

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1699,4 +1699,63 @@
   });
 }
 
+TEST_F(TransferTest, AddrOfValue) {
+  std::string Code = R"(
+void target() {
+  int Foo;
+  int *Bar = &Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+  });
+}
+
+TEST_F(TransferTest, AddrOfReference) {
+  std::string Code = R"(
+void target(int *Foo) {
+  int *Bar = &(*Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -174,19 +174,45 @@
   }
 
   void VisitUnaryOperator(const UnaryOperator *S) {
-if (S->getOpcode() == UO_Deref) {
-  assert(S->getSubExpr() != nullptr);
+// The CFG does not contain `ParenExpr` as top-level statements in basic
+// blocks, however sub-expressions can still be of that type.
+assert(S->getSubExpr() != nullptr);
+const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+assert(SubExpr != nullptr);
+
+switch (S->getOpcode()) {
+case UO_Deref: {
   const auto *SubExprVal = cast_or_null(
-  Env.getValue(*S->getSubExpr(), SkipPast::Reference));
+  Env.getValue(*SubExpr, SkipPast::Reference));
   if (SubExprVal == nullptr)
-return;
+break;
 
   auto &Loc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, Loc);
   Env.setValue(Loc, Env.takeOwnership(std::make_unique(
 SubExprVal->getPointeeLoc(;
+  break;
+}
+case UO_AddrOf: {
+  // Do not form a pointer to a reference. If `SubExpr` is assigned a
+  // `ReferenceValue` then form a value that points to the location of its
+  // pointee.
+  StorageLocation *PointeeLoc =
+  Env.getStorageLocation(*SubExpr, SkipPast::Reference);
+  if (PointeeLoc == nullptr)
+break;
+
+  auto &PointerLoc = Env.createStorageLocation(*S);
+  auto &PointerVal =
+  Env.takeOwnership(std::make_unique(*PointeeLoc));
+  Env.setStorageLocation(*S, PointerLoc);
+  Env.setValue(PointerLoc, PointerVal);
+  break;
+}
+default:
+  // FIXME: Add support for UO_LNot.
+  break;
 }
-// FIXME: Add support for UO_AddrOf, UO_LNot.
   }
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

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

balazske added a comment.

Ping.
I want to see opinion of @shafik (or others) about change of test 
`CompleteRecordBeforeImporting` (turn on minimal import mode in this test).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155

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

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

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

xazax.hun added inline comments.



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

sgatev wrote:
> xazax.hun wrote:
> > 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?
> In the first example you shared the type of the `LValueToRValue` expression 
> is `int *` and the type of the  `UO_Deref` expression is `int` so I think we 
> need to do something to translate one into the other. My understanding is 
> that `LValueToRValue` performs indirection through references whereas 
> `UO_Deref` performs indirection through pointers. This is why I think we 
> ultimately need both. I might be wrong so please correct me and I'd be happy 
> to address this in a follow up.
Indeed, the type did change at `UO_Deref`. 
I think the problem with handling both as performing the indirection is that we 
would end up performing two indirections for the first code snippet as both the 
cast and the dereference operator are present and we would need to introduce 
special code to avoid that. 

When we implemented lifetime analysis, initially we also handled  `UO_Deref` as 
performing the dereference operator. It did not quite work, but after replacing 
`UO_Deref` with noop, and relying on Clang to always put an `LValueToRValue` 
cast whenever we need to do a dereference operation worked out pretty well and 
we have not seen a code snippet yet that would not work with that approach.

I agree that the types are confusing in the AST. I am not even convinced about 
their correctness. For us, it just worked out OK as the type of the pointee 
always had the right type, so we did not even have to look at the AST.

Basically, in the current model, there is a decision one needs to make every 
time we look a location up. Namely, whether we should pass 
`SkipPast::Reference`. I wonder if we actually need to make that decision in 
the code at all. I think it might be possible that the AST has `LValueToRValue` 
casts every time we need to look past references, and we could just follow what 
the AST suggests without forcing us to make a decision point at every API call. 
That would make it way less error prone to work with this codebase and probably 
also making it more resilient to future changes in the language or the AST.

Don't get me wrong, I think what you have currently is perfectly OK and the 
tests demonstrate that this approach works, but I wonder if it is possible to 
simplify it. And the main reason I think there should be a way to make this 
simpler, because I implemented something similar in the past and I did not need 
the equivalent of `SkipPast::Reference`. Here is most of that code: 
https://github.com/mgehre/llvm-project/blob/lifetime/clang/lib/Analysis/LifetimePsetBuilder.cpp#L324




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] D93164: [AST] Add generator for source location introspection

[Andrew Anderson via Phabricator via cfe-commits]

andrew-wja added a comment.

In D93164#3048130 , @lancethepants 
wrote:

> In D93164#2653067 , @mgorny wrote:
>
>> This change breaks cross-compilation now, as it tries running an executable 
>> built for the target system:
>>
>>   FAILED: tools/clang/lib/Tooling/ASTNodeAPI.json 
>>   cd /home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling && 
>> /home/mgorny/llvm-project/build.arm64/bin/clang-ast-dump --skip-processing=0 
>> --astheader=/home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling/ASTTU.cpp
>>  -I /home/mgorny/llvm-project/build.arm64/lib/clang/13.0.0/include -I 
>> /home/mgorny/llvm-project/llvm/../clang/include -I 
>> /home/mgorny/llvm-project/build.arm64/tools/clang/include -I 
>> /home/mgorny/llvm-project/build.arm64/include -I 
>> /home/mgorny/llvm-project/llvm/include -I /sysroot/arm64/usr/include/c++/v1 
>> -I /usr/lib/clang/11.0.1/include -I /sysroot/arm64/usr/include 
>> --json-output-path 
>> /home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling/ASTNodeAPI.json
>>   /bin/sh: /home/mgorny/llvm-project/build.arm64/bin/clang-ast-dump: Exec 
>> format error
>>
>> I guess you can look at TableGens how to correctly generate and use host 
>> executables.
>
> I am now running into this error now that clang13 has released.  I cannot 
> figure a way around this. I do not see any cmake options similar to the 
> tablegens that allow you to specify the binary for the build system.  Am I 
> missing something or is clang13 just broken for cross compilation?

Another confirmation that this change has broken cross compilation. I have 
tried setting both `-DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF` and 
`-DCMAKE_CROSSCOMPILING=ON` separately and in combination. This means that LLVM 
cannot be cross compiled for AArch64, which is a pretty serious problem!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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

[PATCH] D115634: [clangd] Cleanup of readability-identifier-naming

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

Discussion in team sync, work for myself:

- most changes are in unittests --> propose something for the matchers
- cleaning up constants makes sense
- clean up this patch, then do another round of reviews


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115634

___
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

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 400813.
avogelsgesang marked an inline comment as done.
avogelsgesang added a comment.

Properly support macros as requested by @xazax.hun

Added test cases for
+#define COUNT_ONES(SET) SET.count(1)
+  // CHECK-FIXES: #define COUNT_ONES(SET) SET.count(1)
+  // Rewriting the macro would break the code
+  if (COUNT_ONES(MySet)) {
+return COUNT_ONES(MySet);
+  }
+#undef COUNT_ONES
+#define COUNT_ONES count(1)
+  // CHECK-FIXES: #define COUNT_ONES count(1)
+  // Rewriting the macro would break the code
+  if (MySet.COUNT_ONES) {
+return MySet.COUNT_ONES;
+  }
+#undef COUNT_ONES
+#define MY_SET MySet
+  // CHECK-FIXES: #define MY_SET MySet
+  // We still want to rewrite one of the two calls to `count`
+  if (MY_SET.count(1)) {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'contains' to check for 
membership [readability-container-contains]
+// CHECK-FIXES: if (MY_SET.contains(1)) {
+   return MY_SET.count(1);
+  }

Please let me know if I missed any cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,223 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+  void *find(const Key &K);
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map &MyMap) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map &MyMap) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-ME

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

TODO: check for remaining `const` then submit.




Comment at: clang-tools-extra/clangd/unittests/FileIndexTests.cpp:252
 TEST(FileIndexTest, TemplateParamsInLabel) {
-  auto Source = R"cpp(
+  const auto *Source = R"cpp(
 template 

remove const here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

___
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 accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks!


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] D112646: [clang-tidy] Add `readability-container-contains` check

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

xazax.hun added a comment.

Oh wait, I think if the call is coming from a macro we still want to warn, we 
just want to skip the FIXIT part and leave it to the user.


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] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 400820.
avogelsgesang added a comment.

Still warn for issues in macros even if not offering a fix-it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,230 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+  void *find(const Key &K);
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map &MyMap) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map &MyMap) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+  auto C5 = MyMap.find(-5) == MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+  return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(std::map &M, std::unordered_set &US, std::set &S, std::multimap &MM) {
+  bool C1 = M.count(1001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = M.contains(1001);
+  bool C2 = US.count(1002);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'contains' to check for membership [readability-container-contains]
+  // C

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

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

@xazax.hun Can you please merge this to `main` on my behalf? (I don't have 
commit rights)


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] D117535: [clang-tidy] Force LF newlines when writing files

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117535

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

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:26-28
+//   struct S {
+// S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
+//   };

```
// e.g. given `struct S{ int x; unique_ptr y; };`, produces:
//   struct S {
// int x; unique_ptr y;
// S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
//   };
```
or just
```
// e.g. given `struct S{ int x; unique_ptr y; };`, produces:
// S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
```

The tweak does not remove the members, as currently suggested by the comment



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+  Class = N->ASTNode.get();
+if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+  return false;

do we also need to exclude anonymous class declarations here? (Not sure if 
those are also modelled as `CXXRecordDecl` in the clang AST...)



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:133
+// Decide what to do based on the field type.
+class Visitor : public TypeVisitor {
+public:

Is this visitor able to look through `using MyType = int;` and `typedef`?
I think we should also add a test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[ksyx via Phabricator via cfe-commits]

ksyx updated this revision to Diff 400821.
ksyx marked 3 inline comments as done.
ksyx edited the summary of this revision.
ksyx added a comment.

- Use function keyword for JavaScript instead of comparing strings
- Apply review suggestions


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

https://reviews.llvm.org/D117520

Files:
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -130,6 +130,41 @@
Style);
 }
 
+TEST_F(DefinitionBlockSeparatorTest, CommentBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  std::string Prefix = "enum Foo { FOO, BAR };\n"
+   "\n"
+   "/*\n"
+   "test1\n"
+   "test2\n"
+   "*/\n"
+   "int foo(int i, int j) {\n"
+   "  int r = i + j;\n"
+   "  return r;\n"
+   "}\n";
+  std::string Suffix = "enum Bar { FOOBAR, BARFOO };\n"
+   "\n"
+   "/* Comment block in one line*/\n"
+   "int bar3(int j, int k) {\n"
+   "  // A comment\n"
+   "  int r = j % k;\n"
+   "  return r;\n"
+   "}\n";
+  std::string CommentedCode = "/*\n"
+  "int bar2(int j, int k) {\n"
+  "  int r = j / k;\n"
+  "  return r;\n"
+  "}\n"
+  "*/\n";
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode + "\n" +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + "\n" + Suffix);
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + Suffix);
+}
+
 TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
   // Returns a std::pair of two strings, with the first one for passing into
   // Always test and the second one be the expected result of the first string.
@@ -172,13 +207,15 @@
   FormatStyle NeverStyle = getLLVMStyle();
   NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
 
-  auto TestKit = MakeUntouchTest("#ifdef FOO\n\n", "\n#elifndef BAR\n\n",
- "\n#endif\n\n", false);
+  auto TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ "#ifdef FOO\n\n",
+ "\n#elifndef BAR\n\n", "\n#endif\n\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
-  TestKit =
-  MakeUntouchTest("#ifdef FOO\n", "#elifndef BAR\n", "#endif\n", false);
+  TestKit = MakeUntouchTest("/* FOOBAR */\n"
+"#ifdef FOO\n",
+"#elifndef BAR\n", "#endif\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
@@ -210,7 +247,7 @@
   "test1\n"
   "test2\n"
   "*/\n"
-  "int foo(int i, int j) {\n"
+  "/*const*/ int foo(int i, int j) {\n"
   "  int r = i + j;\n"
   "  return r;\n"
   "}\n"
@@ -222,8 +259,10 @@
   "// Comment line 2\n"
   "// Comment line 3\n"
   "int bar(int j, int k) {\n"
-  "  int r = j * k;\n"
-  "  return r;\n"
+  "  {\n"
+  "int r = j * k;\n"
+  "return r;\n"
+  "  }\n"
   "}\n"
   "\n"
   "int bar2(int j, int k) {\n"
@@ -234,7 +273,7 @@
   "/* Comment block in one line*/\n"
   "enum Bar { FOOBAR, BARFOO };\n"
   "\n"
-  "int bar3(int j, int k) {\n"
+  "int bar3(int j, int k, const enum Bar b) {\n"
   "  // A comment\n"
   "  int r = j % k;\n"
   "  return r;\n"
@@ -261,7 +300,7 @@
 "test1\n"
 "test2\n"
 "*/\n"
-"int foo(int i, int j) {\n"
+"/

[clang] 67ac3f1 - [Driver] Pass the flag -dI to cc1 invocation

[Erich Keane via cfe-commits]

Author: Qichao Gu
Date: 2022-01-18T06:16:44-08:00
New Revision: 67ac3f1fbeec6ac53a2e32014fe277e49c77b182

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

LOG: [Driver] Pass the flag -dI to cc1 invocation

Hook up the flag -dI in the driver to pass it to cc1 invocation.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/preprocessor.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index d4afefcb24a98..fd300fbe40145 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -,6 +,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 
   Args.AddLastArg(CmdArgs, options::OPT_dM);
   Args.AddLastArg(CmdArgs, options::OPT_dD);
+  Args.AddLastArg(CmdArgs, options::OPT_dI);
 
   Args.AddLastArg(CmdArgs, options::OPT_fmax_tokens_EQ);
 

diff  --git a/clang/test/Driver/preprocessor.c 
b/clang/test/Driver/preprocessor.c
index 09c1f6c29cc65..d396142cb2ec1 100644
--- a/clang/test/Driver/preprocessor.c
+++ b/clang/test/Driver/preprocessor.c
@@ -4,3 +4,11 @@
 #define A B
 A A
 
+// The driver should pass preprocessor dump flags (-dD, -dM and -dI) to cc1 
invocation
+// RUN: %clang -### -E -dD %s 2>&1 | FileCheck --check-prefix=CHECK-dD %s
+// RUN: %clang -### -E -dM %s 2>&1 | FileCheck --check-prefix=CHECK-dM %s
+// RUN: %clang -### -E -dI %s 2>&1 | FileCheck --check-prefix=CHECK-dI %s
+// CHECK-dD: clang{{.*}} "-cc1" {{.*}} "-dD"
+// CHECK-dM: clang{{.*}} "-cc1" {{.*}} "-dM"
+// CHECK-dI: clang{{.*}} "-cc1" {{.*}} "-dI"
+



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

[PATCH] D117292: [Driver] Pass the flag -dI to cc1 invocation

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67ac3f1fbeec: [Driver] Pass the flag -dI to cc1 invocation 
(authored by qichaogu, committed by erichkeane).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117292

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/preprocessor.c


Index: clang/test/Driver/preprocessor.c
===
--- clang/test/Driver/preprocessor.c
+++ clang/test/Driver/preprocessor.c
@@ -4,3 +4,11 @@
 #define A B
 A A
 
+// The driver should pass preprocessor dump flags (-dD, -dM and -dI) to cc1 
invocation
+// RUN: %clang -### -E -dD %s 2>&1 | FileCheck --check-prefix=CHECK-dD %s
+// RUN: %clang -### -E -dM %s 2>&1 | FileCheck --check-prefix=CHECK-dM %s
+// RUN: %clang -### -E -dI %s 2>&1 | FileCheck --check-prefix=CHECK-dI %s
+// CHECK-dD: clang{{.*}} "-cc1" {{.*}} "-dD"
+// CHECK-dM: clang{{.*}} "-cc1" {{.*}} "-dM"
+// CHECK-dI: clang{{.*}} "-cc1" {{.*}} "-dI"
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -,6 +,7 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_dM);
   Args.AddLastArg(CmdArgs, options::OPT_dD);
+  Args.AddLastArg(CmdArgs, options::OPT_dI);
 
   Args.AddLastArg(CmdArgs, options::OPT_fmax_tokens_EQ);
 


Index: clang/test/Driver/preprocessor.c
===
--- clang/test/Driver/preprocessor.c
+++ clang/test/Driver/preprocessor.c
@@ -4,3 +4,11 @@
 #define A B
 A A
 
+// The driver should pass preprocessor dump flags (-dD, -dM and -dI) to cc1 invocation
+// RUN: %clang -### -E -dD %s 2>&1 | FileCheck --check-prefix=CHECK-dD %s
+// RUN: %clang -### -E -dM %s 2>&1 | FileCheck --check-prefix=CHECK-dM %s
+// RUN: %clang -### -E -dI %s 2>&1 | FileCheck --check-prefix=CHECK-dI %s
+// CHECK-dD: clang{{.*}} "-cc1" {{.*}} "-dD"
+// CHECK-dM: clang{{.*}} "-cc1" {{.*}} "-dM"
+// CHECK-dI: clang{{.*}} "-cc1" {{.*}} "-dI"
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -,6 +,7 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_dM);
   Args.AddLastArg(CmdArgs, options::OPT_dD);
+  Args.AddLastArg(CmdArgs, options::OPT_dI);
 
   Args.AddLastArg(CmdArgs, options::OPT_fmax_tokens_EQ);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


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

https://reviews.llvm.org/D116216

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

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

[Mubashar Ahmad via Phabricator via cfe-commits]

mubashar_ updated this revision to Diff 400827.
mubashar_ added a comment.

Addressed comments from @lenary.


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

https://reviews.llvm.org/D116221

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Sema/test-wunaligned-access.c
  clang/test/Sema/test-wunaligned-access.cpp

Index: clang/test/Sema/test-wunaligned-access.cpp
===
--- /dev/null
+++ clang/test/Sema/test-wunaligned-access.cpp
@@ -0,0 +1,274 @@
+// RUN: %clang_cc1 %s -triple=armv7-none-none-eabi -verify -Wunaligned-access -S -emit-llvm -o %t
+// REQUIRES: arm-registered-target
+//
+// This test suite tests the warning triggered by the -Wunaligned-access option.
+// The warning occurs when a struct or other type of record contains a field
+// that is itself a record. The outer record must be a packed structure, while
+// while the inner record must be unpacked. This is the fundamental condition
+// for the warning to be triggered. Some of these tests may have three layers.
+//
+// The command line option -fsyntax-only is not used as Clang needs to be
+// forced to layout the structs used in this test.
+// The triple in the command line above is used for the assumptions about
+// size and alignment of types.
+
+// Packed-Unpacked Tests (No Pragma)
+
+struct T1 {
+  char a;
+  int b;
+};
+
+struct __attribute__((packed)) U1 {
+  char a;
+  T1 b; // expected-warning {{field b within 'U1' is less aligned than 'T1' and is usually due to 'U1' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+struct __attribute__((packed)) U2 {
+  char a;
+  T1 b __attribute__((aligned(4)));
+  int c;
+};
+
+struct __attribute__((packed)) U3 {
+  char a;
+  char b;
+  short c;
+  T1 d;
+};
+
+struct __attribute__((packed)) U4 {
+  T1 a;
+  int b;
+};
+
+struct __attribute__((aligned(4), packed)) U5 {
+  char a;
+  T1 b; // expected-warning {{field b within 'U5' is less aligned than 'T1' and is usually due to 'U5' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+struct __attribute__((aligned(4), packed)) U6 {
+  char a;
+  char b;
+  short c;
+  T1 d;
+};
+
+// Packed-Unpacked Tests with Pragma
+
+#pragma pack(push, 1)
+
+struct __attribute__((packed)) U7 {
+  char a;
+  T1 b; // expected-warning {{field b within 'U7' is less aligned than 'T1' and is usually due to 'U7' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+struct __attribute__((packed)) U8 {
+  char a;
+  T1 b __attribute__((aligned(4))); // expected-warning {{field b within 'U8' is less aligned than 'T1' and is usually due to 'U8' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+struct __attribute__((aligned(4))) U9 {
+  char a;
+  T1 b; // expected-warning {{field b within 'U9' is less aligned than 'T1' and is usually due to 'U9' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+struct U10 {
+  char a;
+  T1 b; // expected-warning {{field b within 'U10' is less aligned than 'T1' and is usually due to 'U10' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+#pragma pack(pop)
+
+// Packed-Packed Tests
+
+struct __attribute__((packed)) T2 {
+  char a;
+  int b;
+};
+
+struct __attribute__((packed)) U11 {
+  char a;
+  T2 b;
+  int c;
+};
+
+#pragma pack(push, 1)
+struct U12 {
+  char a;
+  T2 b;
+  int c;
+};
+#pragma pack(pop)
+
+// Unpacked-Packed Tests
+
+struct U13 {
+  char a;
+  T2 b;
+  int c;
+};
+
+struct U14 {
+  char a;
+  T2 b __attribute__((aligned(4)));
+  int c;
+};
+
+// Unpacked-Unpacked Test
+
+struct T3 {
+  char a;
+  int b;
+};
+
+struct U15 {
+  char a;
+  T3 b;
+  int c;
+};
+
+// Packed-Packed-Unpacked Test (No pragma)
+
+struct __attribute__((packed)) A1 {
+  char a;
+  T1 b; // expected-warning {{field b within 'A1' is less aligned than 'T1' and is usually due to 'A1' being packed, which can lead to unaligned accesses}}
+};
+
+struct __attribute__((packed)) U16 {
+  char a;
+  A1 b;
+  int c;
+};
+
+struct __attribute__((packed)) A2 {
+  char a;
+  T1 b __attribute__((aligned(4)));
+};
+
+struct __attribute__((packed)) U17 {
+  char a;
+  A2 b; // expected-warning {{field b within 'U17' is less aligned than 'A2' and is usually due to 'U17' being packed, which can lead to unaligned accesses}}
+  int c;
+};
+
+// Packed-Unpacked-Packed tests
+
+struct A3 {
+  char a;
+  T2 b;
+};
+
+struct __attribute__((packed)) U18 {
+  char a;
+  A3 b;
+  int c;
+};
+
+struct A4 {
+  char a;
+  T2 b;
+  int c;
+};
+
+#pragma pack(push, 1)
+struct U19 {
+  char a;
+  A4 b; // expected-warning {{field b within 'U19' is less aligned than 'A4' and is usually due to 'U19' being packed

[PATCH] D117560: [C++20][Concepts] Fix a failed assertion on an invalid typename requirement

[Aaron Ballman via Phabricator via cfe-commits]

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

The parsing code for a typename requirement currently asserts when given 
something which is not a valid type-requirement 
(http://eel.is/c++draft/expr.prim.req.type#nt:type-requirement). This removes 
the assertion to continue on to the proper diagnostic.

This resolves PR53057.

Note that in that PR, it is using `_BitInt(N)` as a dependent type name. This 
patch does not attempt to support that as it is not clear that is a valid type 
requirement (it does not match the grammar production for one). The workaround 
in the PR, however, is definitely valid and works as expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117560

Files:
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/Parser/cxx2a-concepts-requires-expr.cpp


Index: clang/test/Parser/cxx2a-concepts-requires-expr.cpp
===
--- clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -144,3 +144,18 @@
 
 bool r41 = requires { requires (); };
 // expected-error@-1 {{expected expression}}
+
+bool r42 = requires { typename long; }; // expected-error {{expected a 
qualified name after 'typename'}}
+
+template 
+requires requires {
+ typename _BitInt(N); // expected-error {{expected a qualified name after 
'typename'}}
+} using r43 = void;
+
+template 
+using BitInt = _BitInt(N);
+
+template 
+requires requires {
+ typename BitInt; // ok
+} using r44 = void;
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3589,7 +3589,7 @@
 
   // We need to consume the typename to allow 'requires { typename a; 
}'
   SourceLocation TypenameKWLoc = ConsumeToken();
-  if (TryAnnotateCXXScopeToken()) {
+  if (TryAnnotateOptionalCXXScopeToken()) {
 TPA.Commit();
 SkipUntil(tok::semi, tok::r_brace, 
SkipUntilFlags::StopBeforeMatch);
 break;


Index: clang/test/Parser/cxx2a-concepts-requires-expr.cpp
===
--- clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -144,3 +144,18 @@
 
 bool r41 = requires { requires (); };
 // expected-error@-1 {{expected expression}}
+
+bool r42 = requires { typename long; }; // expected-error {{expected a qualified name after 'typename'}}
+
+template 
+requires requires {
+ typename _BitInt(N); // expected-error {{expected a qualified name after 'typename'}}
+} using r43 = void;
+
+template 
+using BitInt = _BitInt(N);
+
+template 
+requires requires {
+ typename BitInt; // ok
+} using r44 = void;
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3589,7 +3589,7 @@
 
   // We need to consume the typename to allow 'requires { typename a; }'
   SourceLocation TypenameKWLoc = ConsumeToken();
-  if (TryAnnotateCXXScopeToken()) {
+  if (TryAnnotateOptionalCXXScopeToken()) {
 TPA.Commit();
 SkipUntil(tok::semi, tok::r_brace, SkipUntilFlags::StopBeforeMatch);
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:26-28
+//   struct S {
+// S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
+//   };

avogelsgesang wrote:
> ```
> // e.g. given `struct S{ int x; unique_ptr y; };`, produces:
> //   struct S {
> // int x; unique_ptr y;
> // S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
> //   };
> ```
> or just
> ```
> // e.g. given `struct S{ int x; unique_ptr y; };`, produces:
> // S(int x, unique_ptr y) : x(x), y(std::move(y)) {}
> ```
> 
> The tweak does not remove the members, as currently suggested by the comment
That's just a bad comment, the tweak won't remove the members



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+  Class = N->ASTNode.get();
+if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+  return false;

avogelsgesang wrote:
> do we also need to exclude anonymous class declarations here? (Not sure if 
> those are also modelled as `CXXRecordDecl` in the clang AST...)
Good point, should also ensure there is a test case for this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514

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

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

I'm not sure this is the correct location for these checks. You're essentially 
checking whether the compilation looks like a C/C++ compilation or an assembly 
compilation based on the flags and the file name. However, the Clang driver 
already does something like this: it converts the command line arguments and 
files into a list of jobs to perform. This is done in `Driver::BuildActions`, 
`Driver::BuildJobs`, `Clang::ConstructJob`, and other places.
I think a better place to do this check is in `Clang::ConstructJob`. There is 
already something similar here:

https://github.com/llvm/llvm-project/blob/10ed1eca241f893085b8db40138e588e72aaee3a/clang/lib/Driver/ToolChains/Clang.cpp#L4396-L4398

  // C++ is not supported for IAMCU.
  if (IsIAMCU && types::isCXX(Input.getType()))
D.Diag(diag::err_drv_clang_unsupported) << "C++ for IAMCU";

I think something like this will work, in `Clang::ConstructJob`:

  bool IsAVR = ...
  ...
  
  if (IsAVR) {
  D.Diag(diag::err_drv_clang_unsupported) << "C/C++ for AVR";

There is a different `ConstructJob` for assembly, so this works.




Comment at: clang/lib/Driver/ToolChains/AVR.h:22
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
+  std::string AVRMcu;
+

I think I would have called this `CPU` not `AVRMcu`, but `AVRMcu` is fine.



Comment at: clang/test/Driver/avr-mmcu.S:4
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 %s -c 
2>&1 | FileCheck -check-prefix=CHECKA %s
+// CHECKA-NOT: error: CPU 'attiny11' does not support {{.*}} language mode
+

Is there a reason why you used `{{.*}}` instead of the actual value?


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

https://reviews.llvm.org/D117423

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM, I'll pull it down and try it here..


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

https://reviews.llvm.org/D117520

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

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 400830.
arnamoy10 added a comment.

1. Trying to tackle a test failure
2. Adding `any_of()` as per reviewer's suggestion.


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

https://reviews.llvm.org/D114379

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -1662,6 +1662,40 @@
   EXPECT_FALSE(verifyModule(*M, &errs()));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimd) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(DL, CLI);
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  bool Found = false;
+  if (llvm::any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }))
+Found = true;
+  EXPECT_TRUE(Found);
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/AssumptionCache.h"
@@ -2144,6 +2145,19 @@
   Latch->getTerminator()->setMetadata(LLVMContext::MD_loop, LoopID);
 }
 
+/// Attach llvm.access.group metadata to the memref instructions of \p Block
+static void addSimdMetadata(BasicBlock *Block, MDNode *AccessGroup,
+LoopInfo &LI) {
+  for (Instruction &I : *Block) {
+if (I.mayReadOrWriteMemory()) {
+  // TODO: This instruction may already have access group from
+  // other pragmas e.g. #pragma clang loop vectorize.  Append
+  // so that the existing metadata is not overwritten.
+  I.setMetadata(LLVMContext::MD_access_group, AccessGroup);
+}
+  }
+}
+
 void OpenMPIRBuilder::unrollLoopFull(DebugLoc, CanonicalLoopInfo *Loop) {
   LLVMContext &Ctx = Builder.getContext();
   addLoopMetadata(
@@ -2159,6 +2173,53 @@
 });
 }
 
+void OpenMPIRBuilder::applySimd(DebugLoc, CanonicalLoopInfo *CanonicalLoop) {
+  LLVMContext &Ctx = Builder.getContext();
+
+  Function *F = CanonicalLoop->getFunction();
+
+  FunctionAnalysisManager FAM;
+  FAM.registerPass([]() { return DominatorTreeAnalysis(); });
+  FAM.registerPass([]() { return LoopAnalysis(); });
+  FAM.registerPass([]() { return PassInstrumentationAnalysis(); });
+
+  LoopAnalysis LIA;
+  LoopInfo &&LI = LIA.run(*F, FAM);
+
+  Loop *L = LI.getLoopFor(CanonicalLoop->getHeader());
+
+  SmallSet Reachable;
+
+  // Get the basic blocks from the loop in which memref instructions
+  // can be found.
+  // TODO: Generalize getting all blocks inside a CanonicalizeLoopInfo,
+  // preferably without running any passes.
+  for (BasicBlock *Block : L->getBlocks()) {
+if (Block == CanonicalLoop->getCond() ||
+Block == CanonicalLoop->getHeader())
+  continue;
+Reachable.insert(Block);
+  }
+
+  // Add access group metadata to memory-access instructions.
+  MDNode *AccessGroup = MDNode::getDistinct(Ctx, {});
+  for (BasicBlock *BB : Reachable)
+addSimdMetadata(BB, AccessGroup, LI);
+
+  // Use the above access group metadata to create loop level
+  // metadata, which should be distinct for each loop.
+  ConstantAsMetadata *BoolConst =
+  ConstantAsMetadata::get(ConstantInt::getTrue(Type::getInt1Ty(Ctx)));
+  // TODO:  If the loop has existing parallel access metadata, have
+  // to combine two lists.
+  addLoopMetadata(
+  CanonicalLoop,
+  {MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.parallel_accesses"),
+ AccessGroup}),
+   MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+ BoolConst})});
+}
+
 /// Create the TargetMachine object t

[PATCH] D115982: [clang][AVR] Implement '__flashN' for variables on different flash banks

[Ayke via Phabricator via cfe-commits]

aykevl accepted this revision.
aykevl added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jacquesguan.

Looks good to me!




Comment at: clang/lib/Basic/Targets/AVR.cpp:27
   const char *DefineName;
+  const int MaxFlashBank; // -1 means the device does not support LPM/ELPM.
 };

This works and is fine, but I think you could also name it `NumFlashBanks` so 
that it is the number of flash banks supported on the device (0 if LPM/ELPM is 
not supported). With 0 for the attiny11 (which has no accessible flash banks) 
and 1 for the attiny22 (because it has only one flash bank that can be 
accessed: flash bank 0).

Just a suggestion, the current system looks good to me.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:8290
+if (isTargetAddressSpace(AS) && 1 <= toTargetAddressSpace(AS) &&
+toTargetAddressSpace(AS) <= 6 && !D->getType().isConstQualified())
   CGM.getDiags().Report(D->getLocation(),

These hardcoded numbers are a bit unfortunate, but OK.



Comment at: clang/test/Sema/avr-flash.c:10
+  static __flash5 const int a5[] = {4, 6}; // expected-error {{unknown type 
name '__flash5'}}
+  // TODO: It would be better to report "'__flash5' is not supported on 
at908515".
+  return a0[n] + a1[n];

I think this can be implemented by always setting the `__flash*` defines and 
checking whether they are valid for the current device in 
`getGlobalVarAddressSpace`.


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

https://reviews.llvm.org/D115982

___
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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:907
   (FSR GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
+def : Pat<(riscv_fsr GPR:$rs3, GPR:$rs1, uimmlog2xlen:$shamt),
+  (FSRI GPR:$rs1, GPR:$rs3, uimmlog2xlen:$shamt)>;

Chenbing.Zheng wrote:
> craig.topper wrote:
> > Need to also handle fsl with a constant argument.
> Emm but there is no fsli in zbt. 
Yes because the compiler can change the constant to turn fsli into fsri. We do 
this on line 903.

If the user uses fsl with a constant we should give them good code by turning 
it into fsri.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

[clang] 8b21e07 - [clang] NFC: Remove unused `DirectoryLookup`

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-18T16:02:18+01:00
New Revision: 8b21e074dbdf79062505f28b700bd4502acf3096

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

LOG: [clang] NFC: Remove unused `DirectoryLookup`

This patch removes bitrotten/dead uses of `DirectoryLookup` in 
`InclusionRewriter.cpp`.

Reviewed By: ahoppen

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

Added: 


Modified: 
clang/lib/Frontend/Rewrite/InclusionRewriter.cpp

Removed: 




diff  --git a/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp 
b/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
index 3f2a781274773..931f3a24c5888 100644
--- a/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ b/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -14,7 +14,6 @@
 #include "clang/Rewrite/Frontend/Rewriters.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
-#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Pragma.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/SmallString.h"
@@ -31,10 +30,8 @@ class InclusionRewriter : public PPCallbacks {
   struct IncludedFile {
 FileID Id;
 SrcMgr::CharacteristicKind FileType;
-const DirectoryLookup *DirLookup;
-IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType,
- const DirectoryLookup *DirLookup)
-: Id(Id), FileType(FileType), DirLookup(DirLookup) {}
+IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType)
+: Id(Id), FileType(FileType) {}
   };
   Preprocessor &PP; ///< Used to find inclusion directives.
   SourceManager &SM; ///< Used to read and manage source files.
@@ -57,8 +54,7 @@ class InclusionRewriter : public PPCallbacks {
 public:
   InclusionRewriter(Preprocessor &PP, raw_ostream &OS, bool ShowLineMarkers,
 bool UseLineDirectives);
-  void Process(FileID FileId, SrcMgr::CharacteristicKind FileType,
-   const DirectoryLookup *DirLookup);
+  void Process(FileID FileId, SrcMgr::CharacteristicKind FileType);
   void setPredefinesBuffer(const llvm::MemoryBufferRef &Buf) {
 PredefinesBuffer = Buf;
   }
@@ -162,8 +158,7 @@ void InclusionRewriter::FileChanged(SourceLocation Loc,
 return;
   FileID Id = FullSourceLoc(Loc, SM).getFileID();
   auto P = FileIncludes.insert(
-  std::make_pair(LastInclusionLocation,
- IncludedFile(Id, NewFileType, PP.GetCurDirLookup(;
+  std::make_pair(LastInclusionLocation, IncludedFile(Id, NewFileType)));
   (void)P;
   assert(P.second && "Unexpected revisitation of the same include directive");
   LastInclusionLocation = SourceLocation();
@@ -371,8 +366,7 @@ StringRef InclusionRewriter::NextIdentifierName(Lexer 
&RawLex,
 /// Use a raw lexer to analyze \p FileId, incrementally copying parts of it
 /// and including content of included files recursively.
 void InclusionRewriter::Process(FileID FileId,
-SrcMgr::CharacteristicKind FileType,
-const DirectoryLookup *DirLookup) {
+SrcMgr::CharacteristicKind FileType) {
   MemoryBufferRef FromFile;
   {
 auto B = SM.getBufferOrNone(FileId);
@@ -433,7 +427,7 @@ void InclusionRewriter::Process(FileID FileId,
<< Mod->getFullModuleName(true) << "\n";
 
   // Include and recursively process the file.
-  Process(Inc->Id, Inc->FileType, Inc->DirLookup);
+  Process(Inc->Id, Inc->FileType);
 
   if (Mod)
 OS << "#pragma clang module end /*"
@@ -559,7 +553,7 @@ void clang::RewriteIncludesInInput(Preprocessor &PP, 
raw_ostream *OS,
   Rewrite->handleModuleBegin(Tok);
   } while (Tok.isNot(tok::eof));
   Rewrite->setPredefinesBuffer(SM.getBufferOrFake(PP.getPredefinesFileID()));
-  Rewrite->Process(PP.getPredefinesFileID(), SrcMgr::C_User, nullptr);
-  Rewrite->Process(SM.getMainFileID(), SrcMgr::C_User, nullptr);
+  Rewrite->Process(PP.getPredefinesFileID(), SrcMgr::C_User);
+  Rewrite->Process(SM.getMainFileID(), SrcMgr::C_User);
   OS->flush();
 }



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

[clang-tools-extra] 105c913 - [clang][lex] NFC: Simplify calls to `LookupFile`

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-18T16:02:18+01:00
New Revision: 105c913156e94163c937d812992fe3a56210998c

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

LOG: [clang][lex] NFC: Simplify calls to `LookupFile`

The `{HeaderSearch,Preprocessor}::LookupFile()` functions take an out-parameter 
`const DirectoryLookup *&`. Most callers end up creating a `const 
DirectoryLookup *` variable that's otherwise unused.

This patch changes the out-parameter from reference to a pointer, making it 
possible to simply pass `nullptr` to the function without the ceremony.

Reviewed By: ahoppen

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/lib/Lex/Pragma.cpp
clang/lib/Lex/Preprocessor.cpp
clang/unittests/Lex/HeaderSearchTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
index a3e5c0e26d172..64fef5ff71711 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -93,10 +93,9 @@ void SuspiciousIncludePPCallbacks::InclusionDirective(
 llvm::sys::path::replace_extension(GuessedFileName,
(HFE.size() ? "." : "") + HFE);
 
-const DirectoryLookup *CurDir;
 Optional File =
 PP->LookupFile(DiagLoc, GuessedFileName, IsAngled, nullptr, nullptr,
-   CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
+   nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
 if (File) {
   Check.diag(DiagLoc, "did you mean to include '%0'?", DiagnosticIDs::Note)
   << GuessedFileName;

diff  --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index 0239de2e23de5..9b9d28433c080 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -408,7 +408,7 @@ class HeaderSearch {
   /// found.
   Optional LookupFile(
   StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
-  const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
+  const DirectoryLookup *FromDir, const DirectoryLookup **CurDir,
   ArrayRef> Includers,
   SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath,
   Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,

diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index ea96bb12bec62..c62bf0c4ceb6f 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2051,7 +2051,7 @@ class Preprocessor {
   Optional
   LookupFile(SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
  const DirectoryLookup *FromDir, const FileEntry *FromFile,
- const DirectoryLookup *&CurDir, SmallVectorImpl *SearchPath,
+ const DirectoryLookup **CurDir, SmallVectorImpl *SearchPath,
  SmallVectorImpl *RelativePath,
  ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
  bool *IsFrameworkFound, bool SkipCache = false);
@@ -2300,7 +2300,7 @@ class Preprocessor {
   };
 
   Optional LookupHeaderIncludeOrImport(
-  const DirectoryLookup *&CurDir, StringRef &Filename,
+  const DirectoryLookup **CurDir, StringRef &Filename,
   SourceLocation FilenameLoc, CharSourceRange FilenameRange,
   const Token &FilenameTok, bool &IsFrameworkFound, bool IsImportDecl,
   bool &IsMapped, const DirectoryLookup *LookupFrom,

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index fb8132a5e40ae..5b77c3e01aace 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -310,9 +310,8 @@ bool GenerateHeaderModuleAction::BeginSourceFileAction(
   auto &HS = CI.getPreprocessor().getHeaderSearchInfo();
   SmallVector Headers;
   for (StringRef Name : ModuleHeaders) {
-const DirectoryLookup *CurDir = nullptr;
 Optional FE = HS.LookupFile(
-Name, SourceLocation(), /*Angled*/ false, nullptr, CurDir, None,
+Name, SourceLocation(), /*Angled*/ false, nullptr, nullptr, None,
 nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
 if (!FE) {
   CI.getDiagnostics().Report(diag::err_module_header_file_not_f

[PATCH] D117309: [clang] NFC: Remove unused `DirectoryLookup`

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b21e074dbdf: [clang] NFC: Remove unused `DirectoryLookup` 
(authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117309

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp


Index: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -14,7 +14,6 @@
 #include "clang/Rewrite/Frontend/Rewriters.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
-#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Pragma.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/SmallString.h"
@@ -31,10 +30,8 @@
   struct IncludedFile {
 FileID Id;
 SrcMgr::CharacteristicKind FileType;
-const DirectoryLookup *DirLookup;
-IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType,
- const DirectoryLookup *DirLookup)
-: Id(Id), FileType(FileType), DirLookup(DirLookup) {}
+IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType)
+: Id(Id), FileType(FileType) {}
   };
   Preprocessor &PP; ///< Used to find inclusion directives.
   SourceManager &SM; ///< Used to read and manage source files.
@@ -57,8 +54,7 @@
 public:
   InclusionRewriter(Preprocessor &PP, raw_ostream &OS, bool ShowLineMarkers,
 bool UseLineDirectives);
-  void Process(FileID FileId, SrcMgr::CharacteristicKind FileType,
-   const DirectoryLookup *DirLookup);
+  void Process(FileID FileId, SrcMgr::CharacteristicKind FileType);
   void setPredefinesBuffer(const llvm::MemoryBufferRef &Buf) {
 PredefinesBuffer = Buf;
   }
@@ -162,8 +158,7 @@
 return;
   FileID Id = FullSourceLoc(Loc, SM).getFileID();
   auto P = FileIncludes.insert(
-  std::make_pair(LastInclusionLocation,
- IncludedFile(Id, NewFileType, PP.GetCurDirLookup(;
+  std::make_pair(LastInclusionLocation, IncludedFile(Id, NewFileType)));
   (void)P;
   assert(P.second && "Unexpected revisitation of the same include directive");
   LastInclusionLocation = SourceLocation();
@@ -371,8 +366,7 @@
 /// Use a raw lexer to analyze \p FileId, incrementally copying parts of it
 /// and including content of included files recursively.
 void InclusionRewriter::Process(FileID FileId,
-SrcMgr::CharacteristicKind FileType,
-const DirectoryLookup *DirLookup) {
+SrcMgr::CharacteristicKind FileType) {
   MemoryBufferRef FromFile;
   {
 auto B = SM.getBufferOrNone(FileId);
@@ -433,7 +427,7 @@
<< Mod->getFullModuleName(true) << "\n";
 
   // Include and recursively process the file.
-  Process(Inc->Id, Inc->FileType, Inc->DirLookup);
+  Process(Inc->Id, Inc->FileType);
 
   if (Mod)
 OS << "#pragma clang module end /*"
@@ -559,7 +553,7 @@
   Rewrite->handleModuleBegin(Tok);
   } while (Tok.isNot(tok::eof));
   Rewrite->setPredefinesBuffer(SM.getBufferOrFake(PP.getPredefinesFileID()));
-  Rewrite->Process(PP.getPredefinesFileID(), SrcMgr::C_User, nullptr);
-  Rewrite->Process(SM.getMainFileID(), SrcMgr::C_User, nullptr);
+  Rewrite->Process(PP.getPredefinesFileID(), SrcMgr::C_User);
+  Rewrite->Process(SM.getMainFileID(), SrcMgr::C_User);
   OS->flush();
 }


Index: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -14,7 +14,6 @@
 #include "clang/Rewrite/Frontend/Rewriters.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
-#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Pragma.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/SmallString.h"
@@ -31,10 +30,8 @@
   struct IncludedFile {
 FileID Id;
 SrcMgr::CharacteristicKind FileType;
-const DirectoryLookup *DirLookup;
-IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType,
- const DirectoryLookup *DirLookup)
-: Id(Id), FileType(FileType), DirLookup(DirLookup) {}
+IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType)
+: Id(Id), FileType(FileType) {}
   };
   Preprocessor &PP; ///< Used to find inclusion directives.
   SourceManager &SM; ///< Used to read and manage source files.
@@ -57,8 +54,7 @@
 public:
   InclusionRewriter(Preprocessor &PP, raw_ostream &OS, bool ShowLineMarkers,
 bool UseLineDirectives);
-  void Process(FileID FileId, SrcMgr::CharacteristicKind Fil

[PATCH] D117312: [clang][lex] NFC: Simplify calls to `LookupFile`

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG105c913156e9: [clang][lex] NFC: Simplify calls to 
`LookupFile` (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117312

Files:
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -238,10 +238,9 @@
   /*User=*/None, /*Group=*/None, llvm::sys::fs::file_type::regular_file);
 
   bool IsMapped = false;
-  const DirectoryLookup *CurDir = nullptr;
   auto FoundFile = Search.LookupFile(
   "Foo/Foo.h", SourceLocation(), /*isAngled=*/true, /*FromDir=*/nullptr,
-  CurDir, /*Includers=*/{}, /*SearchPath=*/nullptr,
+  /*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr,
   /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
   /*SuggestedModule=*/nullptr, &IsMapped,
   /*IsFrameworkFound=*/nullptr);
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -566,11 +566,10 @@
   if (!PPOpts->PCHThroughHeader.empty()) {
 // Lookup and save the FileID for the through header. If it isn't found
 // in the search path, it's a fatal error.
-const DirectoryLookup *CurDir;
 Optional File = LookupFile(
 SourceLocation(), PPOpts->PCHThroughHeader,
-/*isAngled=*/false, /*FromDir=*/nullptr, /*FromFile=*/nullptr, CurDir,
-/*SearchPath=*/nullptr, /*RelativePath=*/nullptr,
+/*isAngled=*/false, /*FromDir=*/nullptr, /*FromFile=*/nullptr,
+/*CurDir=*/nullptr, /*SearchPath=*/nullptr, /*RelativePath=*/nullptr,
 /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
 /*IsFrameworkFound=*/nullptr);
 if (!File) {
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -526,9 +526,8 @@
 return llvm::None;
 
   // Search include directories for this file.
-  const DirectoryLookup *CurDir;
   File = PP.LookupFile(FilenameTok.getLocation(), Filename, isAngled, nullptr,
-   nullptr, CurDir, nullptr, nullptr, nullptr, nullptr,
+   nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr);
   if (!File) {
 if (!SuppressIncludeNotFoundError)
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1228,10 +1228,9 @@
 return false;
 
   // Search include directories.
-  const DirectoryLookup *CurDir;
   Optional File =
   PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, LookupFromFile,
-CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
+nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
 
   if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
 SrcMgr::CharacteristicKind FileType = SrcMgr::C_User;
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -818,10 +818,13 @@
 Optional Preprocessor::LookupFile(
 SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
 const DirectoryLookup *FromDir, const FileEntry *FromFile,
-const DirectoryLookup *&CurDir, SmallVectorImpl *SearchPath,
+const DirectoryLookup **CurDirArg, SmallVectorImpl *SearchPath,
 SmallVectorImpl *RelativePath,
 ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
 bool *IsFrameworkFound, bool SkipCache) {
+  const DirectoryLookup *CurDirLocal = nullptr;
+  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
 
@@ -877,7 +880,7 @@
 const DirectoryLookup *TmpCurDir = CurDir;
 const DirectoryLookup *TmpFromDir = nullptr;
 while (Optional FE = HeaderInfo.LookupFile(
-   Filename, FilenameLoc, isAngled, TmpFromDir, TmpCurDir,
+   Filename, FilenameLoc, isAngled, TmpFromDir

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

[Endre Fülöp 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 rG17f74240e6c3: [analyzer][NFC] Refactor GenericTaintChecker 
to use CallDescriptionMap (authored by gamesh411).

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

[clang] 17f7424 - [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

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

Author: Endre Fülöp
Date: 2022-01-18T16:04:04+01:00
New Revision: 17f74240e6c3bb64fe741f12d67903e6b7fc38cb

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

LOG: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

GenericTaintChecker now uses CallDescriptionMap to describe the possible
operation in code which trigger the introduction (sources), the removal
(filters), the passing along (propagations) and detection (sinks) of
tainted values.

Reviewed By: steakhal, NoQ

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

Added: 


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

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
index a383012dc3516..94fa0d9ecdc31 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -84,6 +84,8 @@ class CheckerContext {
 return Eng.getContext();
   }
 
+  const ASTContext &getASTContext() const { return Eng.getContext(); }
+
   const LangOptions &getLangOpts() const {
 return Eng.getContext().getLangOpts();
   }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 66ef781871ec9..1b61b4982931f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/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 ento;
 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.
+   

[PATCH] D117563: [clang][dataflow] Remove obsolete FIXME

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: steakhal, rnkovacs.
sgatev requested review of this revision.
Herald added a project: clang.

The FIXME is no longer relevant as ControlFlowContext centralizes the
construction of the CFG.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117563

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -191,10 +191,6 @@
 runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
   TypeErasedDataflowAnalysis &Analysis,
   const Environment &InitEnv) {
-  // FIXME: Consider enforcing that `Cfg` meets the requirements that
-  // are specified in the header. This could be done by remembering
-  // what options were used to build `Cfg` and asserting on them here.
-
   PostOrderCFGView POV(&CFCtx.getCFG());
   ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
 


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -191,10 +191,6 @@
 runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
   TypeErasedDataflowAnalysis &Analysis,
   const Environment &InitEnv) {
-  // FIXME: Consider enforcing that `Cfg` meets the requirements that
-  // are specified in the header. This could be done by remembering
-  // what options were used to build `Cfg` and asserting on them here.
-
   PostOrderCFGView POV(&CFCtx.getCFG());
   ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117563: [clang][dataflow] Remove obsolete FIXME

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

xazax.hun added a comment.

I think for trivial changes like this you do not need to create a review, you 
can commit it directly :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117563

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

[PATCH] D117563: [clang][dataflow] Remove obsolete FIXME

[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 rGd7c19f947e0c: [clang][dataflow] Remove obsolete FIXME 
(authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117563

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -191,10 +191,6 @@
 runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
   TypeErasedDataflowAnalysis &Analysis,
   const Environment &InitEnv) {
-  // FIXME: Consider enforcing that `Cfg` meets the requirements that
-  // are specified in the header. This could be done by remembering
-  // what options were used to build `Cfg` and asserting on them here.
-
   PostOrderCFGView POV(&CFCtx.getCFG());
   ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
 


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -191,10 +191,6 @@
 runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
   TypeErasedDataflowAnalysis &Analysis,
   const Environment &InitEnv) {
-  // FIXME: Consider enforcing that `Cfg` meets the requirements that
-  // are specified in the header. This could be done by remembering
-  // what options were used to build `Cfg` and asserting on them here.
-
   PostOrderCFGView POV(&CFCtx.getCFG());
   ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d7c19f9 - [clang][dataflow] Remove obsolete FIXME

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-18T15:16:44Z
New Revision: d7c19f947e0c93369e71404c33db836e3b6ac3f7

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

LOG: [clang][dataflow] Remove obsolete FIXME

The FIXME is no longer relevant as ControlFlowContext centralizes the
construction of the CFG.

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: xazax.hun

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp 
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 722b24f5b9718..538cdce206b2f 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -191,10 +191,6 @@ 
std::vector>
 runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
   TypeErasedDataflowAnalysis &Analysis,
   const Environment &InitEnv) {
-  // FIXME: Consider enforcing that `Cfg` meets the requirements that
-  // are specified in the header. This could be done by remembering
-  // what options were used to build `Cfg` and asserting on them here.
-
   PostOrderCFGView POV(&CFCtx.getCFG());
   ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
 



___
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 added a comment.

Amazing, thank you. This is my first patch so I will need help landing, but I 
will also apply for commit access as I intend to keep contributing. Here is my 
name and email for this commit: Elliott Maguire 


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] D117566: [clang][lex] Introduce `DirectoryLookupIterator`

[Jan Svoboda via Phabricator via cfe-commits]

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

The `const DirectoryLookup *` out-parameter of 
`{HeaderSearch,Preprocessor}::LookupFile()` is assigned the most recently used 
search directory, which callers use to implement `#include_next`.

>From the function signature it's not obvious the `const DirectoryLookup *` is 
>being used as an iterator. This patch introduces `DirectoryLookupIterator` to 
>make that affordance obvious. This would've prevented a bug that occurred 
>after initially landing D116750 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117566

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

Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1157,9 +1157,9 @@
 /// EvaluateHasIncludeCommon - Process a '__has_include("path")'
 /// or '__has_include_next("path")' expression.
 /// Returns true if successful.
-static bool EvaluateHasIncludeCommon(Token &Tok,
- IdentifierInfo *II, Preprocessor &PP,
- const DirectoryLookup *LookupFrom,
+static bool EvaluateHasIncludeCommon(Token &Tok, IdentifierInfo *II,
+ Preprocessor &PP,
+ DirectoryLookupIterator LookupFrom,
  const FileEntry *LookupFromFile) {
   // Save the location of the current token.  If a '(' is later found, use
   // that location.  If not, use the end of this location instead.
@@ -1260,7 +1260,7 @@
   // issue a diagnostic.
   // FIXME: Factor out duplication with
   // Preprocessor::HandleIncludeNextDirective.
-  const DirectoryLookup *Lookup = PP.GetCurDirLookup();
+  DirectoryLookupIterator Lookup = PP.GetCurDirLookup();
   const FileEntry *LookupFromFile = nullptr;
   if (PP.isInPrimaryFile() && PP.getLangOpts().IsHeaderFile) {
 // If the main file is a header, then it's either for PCH/AST generation,
Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -66,7 +66,7 @@
 
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
-bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
+bool Preprocessor::EnterSourceFile(FileID FID, DirectoryLookupIterator CurDir,
SourceLocation Loc,
bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
@@ -100,7 +100,7 @@
 /// EnterSourceFileWithLexer - Add a source file to the top of the include stack
 ///  and start lexing tokens from it instead of the current buffer.
 void Preprocessor::EnterSourceFileWithLexer(Lexer *TheLexer,
-const DirectoryLookup *CurDir) {
+DirectoryLookupIterator CurDir) {
 
   // Add the current lexer to the include stack.
   if (CurPPLexer || CurTokenLexer)
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -817,13 +817,13 @@
 
 Optional Preprocessor::LookupFile(
 SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
-const DirectoryLookup *FromDir, const FileEntry *FromFile,
-const DirectoryLookup **CurDirArg, SmallVectorImpl *SearchPath,
+DirectoryLookupIterator FromDir, const FileEntry *FromFile,
+DirectoryLookupIterator *CurDirArg, SmallVectorImpl *SearchPath,
 SmallVectorImpl *RelativePath,
 ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
 bool *IsFrameworkFound, bool SkipCache) {
-  const DirectoryLookup *CurDirLocal = nullptr;
-  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+  DirectoryLookupIterator CurDirLocal = nullptr;
+  DirectoryLookupIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
 
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
@@ -877,8 +877,8 @@
   if (FromFile) {
 // We're supposed to start looking from after a particular file. Search
 // the include path until we find that file or run out of files.
-const DirectoryLookup *TmpCurDir = CurDir;
-const DirectoryLookup *TmpFromDir = nullptr;
+DirectoryLooku

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG, let's hope this solves all the AMDGPU issues we've seen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

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

[PATCH] D116750: [clang][lex] Keep references to `DirectoryLookup` objects up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

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

Rebase on top of D117566 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -23,6 +23,12 @@
 namespace clang {
 namespace {
 
+static std::shared_ptr createTargetOptions() {
+  auto TargetOpts = std::make_shared();
+  TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
+  return TargetOpts;
+}
+
 // The test fixture.
 class HeaderSearchTest : public ::testing::Test {
 protected:
@@ -30,12 +36,10 @@
   : VFS(new llvm::vfs::InMemoryFileSystem), FileMgr(FileMgrOpts, VFS),
 DiagID(new DiagnosticIDs()),
 Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
-SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions),
+SourceMgr(Diags, FileMgr), TargetOpts(createTargetOptions()),
+Target(TargetInfo::CreateTargetInfo(Diags, TargetOpts)),
 Search(std::make_shared(), SourceMgr, Diags,
-   LangOpts, Target.get()) {
-TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
-Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
-  }
+   LangOpts, Target.get()) {}
 
   void addSearchDir(llvm::StringRef Dir) {
 VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
@@ -55,6 +59,27 @@
 Search.AddSystemSearchPath(DL);
   }
 
+  void setSearchDirs(llvm::ArrayRef QuotedDirs,
+ llvm::ArrayRef AngledDirs) {
+auto AddPath = [&](StringRef Dir, bool IsAngled) {
+  VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+   /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+  auto Group = IsAngled ? frontend::IncludeDirGroup::Angled
+: frontend::IncludeDirGroup::Quoted;
+  Search.getHeaderSearchOpts().AddPath(Dir, Group,
+   /*IsFramework=*/false,
+   /*IgnoreSysRoot=*/true);
+};
+
+for (llvm::StringRef Dir : QuotedDirs)
+  AddPath(Dir, /*IsAngled=*/false);
+for (llvm::StringRef Dir : AngledDirs)
+  AddPath(Dir, /*IsAngled=*/true);
+
+clang::ApplyHeaderSearchOptions(Search, Search.getHeaderSearchOpts(),
+LangOpts, Target->getTriple());
+  }
+
   void addHeaderMap(llvm::StringRef Filename,
 std::unique_ptr Buf,
 bool isAngled = false) {
@@ -71,6 +96,17 @@
 Search.AddSearchPath(DL, isAngled);
   }
 
+  void createModule(StringRef Mod) {
+std::string ModDir = ("/" + Mod).str();
+std::string ModHeader = (Mod + ".h").str();
+VFS->addFile(
+ModDir + "/module.modulemap", 0,
+llvm::MemoryBuffer::getMemBufferCopy(
+("module " + Mod + " { header \"" + ModHeader + "\" }").str()));
+VFS->addFile(ModDir + "/" + ModHeader, 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -254,5 +290,31 @@
   EXPECT_EQ(FI->Framework.str(), "Foo");
 }
 
+TEST_F(HeaderSearchTest, SearchPathUsage) {
+  Search.getHeaderSearchOpts().ImplicitModuleMaps = true;
+
+  setSearchDirs(/*QuotedDirs=*/{"/M0"}, /*AngledDirs=*/{"/M2", "/M3"});
+  createModule("M0");
+  createModule("M2");
+  createModule("M3");
+
+  {
+Module *M2 = Search.lookupModule("M2");
+EXPECT_NE(M2, nullptr);
+EXPECT_EQ(Search.getSearchDirUsage(), (std::vector{0, 1, 0}));
+EXPECT_EQ(Search.computeUserEntryUsage(), (std::vector{0, 1, 0}));
+  }
+
+  addSearchDir("/M1");
+  createModule("M1");
+
+  {
+Module *M1 = Search.lookupModule("M1");
+EXPECT_NE(M1, nullptr);
+EXPECT_EQ(Search.getSearchDirUsage(), (std::vector{0, 1, 1, 0}));
+EXPECT_EQ(Search.computeUserEntryUsage(), (std::vector{0, 1, 0}));
+  }
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -79,6 +79,10 @@
 
 ExternalHeaderFileInfoSource::~ExternalHeaderFileInfoSource() = default;
 
+const DirectoryLookup &DirectoryLookupIterator::operator*() const {
+  return *HS->SearchDirs[Idx];
+}
+
 HeaderSearch::HeaderSearch(std::shared_ptr HSOpts,
SourceManager &SourceMgr, DiagnosticsEngine &Diags,
const LangOptions &LangOpts,
@@ -115,19 +119,24 @@
 llvm::DenseMap searchDirToHSEntry) {
   assert(angledDirIdx <= syste

[PATCH] D116750: [clang][lex] Keep references to `DirectoryLookup` objects up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:415
   StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
-  const DirectoryLookup *FromDir, const DirectoryLookup **CurDir,
+  maybe_search_dir_iterator FromDir, maybe_search_dir_iterator *CurDir,
   ArrayRef> Includers,

jansvoboda11 wrote:
> This is the interesting change in the latest revision. Callers of this 
> function performed pointer arithmetics on `const DirectoryLookup *`, 
> essentially treating it as an iterator. Since `DirectoryLookup` objects are 
> no longer stored in contiguous memory, that's unsafe. I'm using an iterator 
> to allow doing that safely and using `llvm::Optional` to provide the 
> nullability clients take advantage of.
> 
> I don't love the long name that causes a bunch of formatting changes. Maybe 
> it's worth extracting it out of `HeaderSearch` and giving it shorter name. I 
> also don't love that the API is not the same as for `const DirectoryLookup 
> *`, leading to some "unnecessary" changes (e.g. `CurDir = nullptr` -> 
> `CurDir.reset()`). I think it might be worth creating custom iterator type 
> that would have the original API, but would behave correctly with the new 
> memory setup.
Resolved in the latest revision by introducing D117566.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

[Jano Simas via Phabricator via cfe-commits]

janosimas updated this revision to Diff 400854.
janosimas added a comment.

I changed the wording and added an example as suggested.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D49864

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
@@ -1,11 +1,11 @@
 // REQUIRES: shell
 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
 // RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
 // RUN: mkdir -p %T/compilation-database-test/
 // RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -p=%T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
 struct A {
   virtual void f() {}
   virtual void g() {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -79,6 +79,8 @@
 - Generalized the `modernize-use-default-member-init` check to handle non-default
   constructors.
 
+- `clang-tidy-diff.py` allows using `clang-tidy` flags after `--`. This is a breaking change, check the script help for usage.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -17,9 +17,9 @@
 detect clang-tidy regressions in the lines touched by a specific patch.
 Example usage for git/svn users:
 
-  git diff -U0 HEAD^ | clang-tidy-diff.py -p1
+  git diff -U0 HEAD^ | clang-tidy-diff.py -strip 1
   svn diff --diff-cmd=diff -x-U0 | \
-  clang-tidy-diff.py -fix -checks=-*,modernize-use-override
+  clang-tidy-diff.py -- -fix -checks=-*,modernize-use-override
 
 """
 
@@ -119,12 +119,14 @@
   parser = argparse.ArgumentParser(description=
'Run clang-tidy against changed files, and '
'output diagnostics only for modified '
-   'lines.')
+   'lines.'
+   ' clang-tidy arguments can be passed after a \'--\'.'
+   ' Ex.: \'git diff -U0 HEAD^ | clang-tidy-diff.py -strip 1 -- -fix -checks=-*,modernize-use-override\'')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
   default='clang-tidy',
   help='path to clang-tidy binary')
-  parser.add_argument('-p', metavar='NUM', default=0,
-  help='strip the smallest prefix containing P slashes')
+  parser.add_argument('-strip', metavar='NUM', default=0,
+  help='strip the smallest prefix containing NUM slashes')
   parser.add_argument('-regex', metavar='PATTERN', default=None,
   help='custom pattern selecting file paths to check '
   '(case sensitive, overrides -iregex)')
@@ -136,34 +138,14 @@
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('-timeout', type=int, default=None,
   help='timeout per each file in seconds.')
-  parser.add_argument('-fix', action='store_true', default=False,
-  help='apply suggested fixes')
-  parser.add_argument('-checks',
-  help='checks filter, when not specified, use clang-tidy '
-  'default',
-  default='')
-  parser.add_argument('-use-color', action='store_true',
-

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

[Jano Simas via Phabricator via cfe-commits]

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

Yes, I'll need help landing it


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D49864

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

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

[David Stone via Phabricator via cfe-commits]

davidstone added a comment.

Is there anything else I need to do to help move this forward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99134

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

[PATCH] D117567: [clang][dataflow] Add a test to justify skipping past references in UO_Deref

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: steakhal, rnkovacs.
sgatev requested review of this revision.
Herald added a project: clang.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117567

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1758,4 +1758,34 @@
   });
 }
 
+TEST_F(TransferTest, DerefDependentPtr) {
+  std::string Code = R"(
+template 
+void target(T *Foo) {
+  T &Bar = *Foo;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -182,6 +182,7 @@
 
 switch (S->getOpcode()) {
 case UO_Deref: {
+  // Skip past a reference to handle dereference of a dependent pointer.
   const auto *SubExprVal = cast_or_null(
   Env.getValue(*SubExpr, SkipPast::Reference));
   if (SubExprVal == nullptr)


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1758,4 +1758,34 @@
   });
 }
 
+TEST_F(TransferTest, DerefDependentPtr) {
+  std::string Code = R"(
+template 
+void target(T *Foo) {
+  T &Bar = *Foo;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -182,6 +182,7 @@
 
 switch (S->getOpcode()) {
 case UO_Deref: {
+  // Skip past a reference to handle dereference of a dependent pointer.
   const auto *SubExprVal = cast_or_null(
   Env.getValue(*SubExpr, SkipPast::Reference));
   if (SubExprVal == nullptr)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1691-1696
+  bool Found = false;
+  if (llvm::any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }))
+Found = true;
+  EXPECT_TRUE(Found);

Can make it a one-liner.


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

https://reviews.llvm.org/D114379

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

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

[praveen velliengiri via Phabricator via cfe-commits]

pvellien added a comment.

In D116216#3251066 , @yaxunl wrote:

> LGTM. Thanks.

Could you please commit on my behalf? I don't have a commit access to llvm trunk


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

https://reviews.llvm.org/D116216

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm getting some sort of seemingly empty replacements going on, after `public:` 
I can't quite put my finger on what's happening.


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

https://reviews.llvm.org/D117520

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

[clang-tools-extra] e598913 - [clang-tidy] Force LF newlines when writing files

[via cfe-commits]

Author: Richard
Date: 2022-01-18T09:39:42-07:00
New Revision: e598913a4734ce682732703bb362dc3c5c0079c0

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

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

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

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/add_new_check.py 
b/clang-tools-extra/clang-tidy/add_new_check.py
index 1e26b07121c6..50a220b3f975 100755
--- a/clang-tools-extra/clang-tidy/add_new_check.py
+++ b/clang-tools-extra/clang-tidy/add_new_check.py
@@ -37,7 +37,7 @@ def adapt_cmake(module_path, check_name_camel):
   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 @@ def write_header(module_path, module, namespace, check_name, 
check_name_camel):
   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 @@ class %(check_name)s : public ClangTidyCheck {
 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 @@ def adapt_module(module_path, module, check_name, 
check_name_camel):
 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 @@ def add_release_notes(module_path, module, check_name):
   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 @@ def write_test(module_path, module, check_name, 
test_extension):
   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 @@ def format_link_alias(doc_file):
   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 @@ def write_docs(module_path, module, check_name):
   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



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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe598913a4734: [clang-tidy] Force LF newlines when writing 
files (authored by LegalizeAdulthood).

Repository:
  rG LLVM Github Monorepo

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

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_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 

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[ksyx via Phabricator via cfe-commits]

ksyx added a comment.

In D117520#3251376 , @MyDeveloperDay 
wrote:

> I'm getting some sort of seemingly empty replacements going on, after 
> `public:` I can't quite put my finger on what's happening.

Not so sure what empty means here. Is it removing empty line following 
`public:` or adding, or something else?


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

https://reviews.llvm.org/D117520

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

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: steakhal, gamesh411, NoQ.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117568

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td


Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -552,7 +552,7 @@
"or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
   WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, 
StreamChecker]>,
-  Documentation;
+  Documentation;
 
 } // end "alpha.unix"
 
Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2333,6 +2333,43 @@
 alpha.unix
 ^^^
 
+.. _alpha-unix-StdCLibraryFunctionArgs:
+
+alpha.unix.StdCLibraryFunctionArgs (C)
+""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.
+
+.. code-block:: c
+
+  void test_alnum_concrete(int v) {
+int ret = isalnum(256); // \
+// warning: Function argument constraint is not satisfied
+(void)ret;
+  }
+
+If the argument's value is unknown then the value is assumed to hold the 
proper value range.
+
+.. code-block:: c
+
+  #define EOF -1
+  void test_alnum_symbolic(int x) {
+int ret = isalnum(x);
+(void)ret;
+clang_analyzer_eval(EOF <= x && x <= 255); // this reports TRUE
+  }
+
+If the user disables the checker then the argument violation warning is
+suppressed. However, the assumption about the argument is still modeled 
(otherwise we
+would be further analyzing an illformed program).
+
+The checker models functions (and emits diagnostics) from the C standard by
+default. The ``ModelPOSIX`` option enables the checker to model (and emit
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+
 .. _alpha-unix-BlockInCriticalSection:
 
 alpha.unix.BlockInCriticalSection (C)


Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -552,7 +552,7 @@
"or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
   WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
-  Documentation;
+  Documentation;
 
 } // end "alpha.unix"
 
Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2333,6 +2333,43 @@
 alpha.unix
 ^^^
 
+.. _alpha-unix-StdCLibraryFunctionArgs:
+
+alpha.unix.StdCLibraryFunctionArgs (C)
+""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.
+
+.. code-block:: c
+
+  void test_alnum_concrete(int v) {
+int ret = isalnum(256); // \
+// warning: Function argument constraint is not satisfied
+(void)ret;
+  }
+
+If the argument's value is unknown then the value is assumed to hold the proper value range.
+
+.. code-block:: c
+
+  #define EOF -1
+  void test_alnum_symbolic(int x) {
+int ret = isalnum(x);
+(void)ret;
+clang_analyzer_eval(EOF <= x && x <= 255); // this reports TRUE
+  }
+
+If the user disables the checker then the argument violation warning is
+suppressed. However, the assumption about the argument is still modeled (otherwise we
+would be further analyzing an illformed program).
+
+The checker models functions (and emits diagnostics) from the C standard by
+default. The ``ModelPOSIX`` option enables the checker to model (and emit
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+
 .. _alpha-unix-BlockInCriticalSection:
 
 alpha.unix.BlockInCriticalSection (C)
___
cfe-commits mailing list
cfe-commits@

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4246
+  case Intrinsic::riscv_fsl:
+return DAG.getNode(RISCVISD::FSL, DL, XLenVT, Op.getOperand(1),
+   Op.getOperand(2), Op.getOperand(3));

The operand order for RISCVISD::FSL/FSR match llvm.fshl and llvm.fshr rather 
than rs1, rs2, rs3 order. I think the operand order you want for riscv.fsl and 
riscv.fsr should be rs1, rs2, rs3.

And the assembly printing for fsl/fsr prints $rd, $rs1, $rs3, $rs2 makes this 
even more confusing.

I'll put up a patch to change RISCVISD::FSL/FSR and RISCVISD::FSLW/FSRW order 
to be in rs1, rs2, rs3 order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
zahiraam added reviewers: JVApen, rsmith.
zahiraam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix for issue https://github.com/llvm/llvm-project/issues/53182


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117569

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenCXX/PR19955.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/SemaCXX/PR19955.cpp
  clang/test/SemaCXX/dllimport-constexpr.cpp

Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- clang/test/SemaCXX/dllimport-constexpr.cpp
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -40,7 +40,6 @@
 // constexpr initialization doesn't work for dllimport things.
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (*constexpr_import_func)() = &imported_func;
-// expected-error@+1{{must be initialized by a constant expression}}
 constexpr int *constexpr_import_int = &imported_int;
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (Foo::*constexpr_memptr)() = &Foo::imported_method;
@@ -60,3 +59,8 @@
   // expected-note@+1 {{requested here}}
   StaticConstexpr::g_fp();
 }
+
+void foo() {
+  extern int __declspec(dllimport) dll_import_int;
+  constexpr int& dll_import_constexpr_ref = dll_import_int;
+}
Index: clang/test/SemaCXX/PR19955.cpp
===
--- clang/test/SemaCXX/PR19955.cpp
+++ clang/test/SemaCXX/PR19955.cpp
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -triple i686-mingw32 -verify -std=c++11 %s
 
 extern int __attribute__((dllimport)) var;
-constexpr int *varp = &var; // expected-error {{must be initialized by a constant expression}}
+constexpr int *varp = &var;
 
 extern __attribute__((dllimport)) void fun();
 constexpr void (*funp)(void) = &fun; // expected-error {{must be initialized by a constant expression}}
Index: clang/test/CodeGenCXX/dllimport.cpp
===
--- clang/test/CodeGenCXX/dllimport.cpp
+++ clang/test/CodeGenCXX/dllimport.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -disable-noundef-analysis -triple x86_64-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -disable-noundef-analysis -triple x86_64-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC64 --check-prefix=M64 %s
 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-gnu-fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -w | FileCheck --check-prefix=GNU --check-prefix=G32 %s
 // RUN: %clang_cc1 -disable-noundef-analysis -triple x86_64-windows-gnu  -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -w | FileCheck --check-prefix=GNU %s
 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=18.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=M18 %s
@@ -39,30 +39,34 @@
 
 // Import declaration.
 // MSC-DAG: @"?ExternGlobalDecl@@3HA" = external dllimport global i32
+// MSC64-DAG: @"?ExternGlobalDecl@@3HA" = external dllimport global i32
 // GNU-DAG: @ExternGlobalDecl= external dllimport global i32
 __declspec(dllimport) extern int ExternGlobalDecl;
 USEVAR(ExternGlobalDecl)
 
 // dllimport implies a declaration.
 // MSC-DAG: @"?GlobalDecl@@3HA" = external dllimport global i32
+// MSC64-DAG: @"?GlobalDecl@@3HA" = external dllimport global i32
 // GNU-DAG: @GlobalDecl= external dllimport global i32
 __declspec(dllimport) int GlobalDecl;
 USEVAR(GlobalDecl)
 
 // Redeclarations
 // MSC-DAG: @"?GlobalRedecl1@@3HA" = external dllimport global i32
+// MSC64-DAG: @"?GlobalRedecl1@@3HA" = external dllimport global i32
 // GNU-DAG: @GlobalRedecl1= external dllimport global i32
 __declspec(dllimport) extern int GlobalRedecl1;
 __declspec(dllimport) extern int GlobalRedecl1;
 USEVAR(GlobalRedecl1)
 
 // MSC-DAG: @"?GlobalRedecl2a@@3HA" = external dllimport global i32
+// MSC64-DAG: @"?GlobalRedecl2a@@3HA" = external dllimport global i32
 // GNU-DAG: @GlobalRedecl2a= external dllimport global i32
 __declspec(dllimport) int GlobalRedecl2a;
 __declspec(dllimport) int GlobalRedecl2a;
 USEVAR(GlobalRedecl2a)
 
-// M32-DAG: @"?GlobalRedecl2b@@3PAHA"   = external dllimport global i32*
+// MSC-DAG: @"?Glob

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

For

  class Test
  {
  public:
  static void foo()
  {
  }
  };

F21703157: image.png 

  $ clang-format -n test.cxx
  test.cxx:3:8: warning: code should be clang-formatted 
[-Wclang-format-violations
  ]
  public:
 ^



  $ clang-format --output-replacements-xml test.cxx
  
  
  

  

but when I run it nothing changes

  $ clang-format test.cxx
  class Test
  {
  public:
  static void foo()
  {
  }
  };

And no character change

F21703226: image.png 


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

https://reviews.llvm.org/D117520

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

[clang] fa596fb - Fix a failed assertion on an invalid typename requirement

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-01-18T11:59:08-05:00
New Revision: fa596fb0779ae9029edbcff80ff95e9d1a816206

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

LOG: Fix a failed assertion on an invalid typename requirement

The parsing code for a typename requirement currently asserts when
given something which is not a valid type-requirement
(http://eel.is/c++draft/expr.prim.req.type#nt:type-requirement). This
removes the assertion to continue on to the proper diagnostic.

This resolves PR53057.

Note that in that PR, it is using _BitInt(N) as a dependent type name.
This patch does not attempt to support that as it is not clear that is
a valid type requirement (it does not match the grammar production for
one). The workaround in the PR, however, is definitely valid and works
as expected.

Added: 


Modified: 
clang/lib/Parse/ParseExprCXX.cpp
clang/test/Parser/cxx2a-concepts-requires-expr.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index cd600c4247a7..2d38891c723f 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3589,7 +3589,7 @@ ExprResult Parser::ParseRequiresExpression() {
 
   // We need to consume the typename to allow 'requires { typename a; 
}'
   SourceLocation TypenameKWLoc = ConsumeToken();
-  if (TryAnnotateCXXScopeToken()) {
+  if (TryAnnotateOptionalCXXScopeToken()) {
 TPA.Commit();
 SkipUntil(tok::semi, tok::r_brace, 
SkipUntilFlags::StopBeforeMatch);
 break;

diff  --git a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp 
b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
index d22f21b786f4..d3f46c163ffd 100644
--- a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -144,3 +144,18 @@ bool r40 = requires { requires (int i) { i; }; };
 
 bool r41 = requires { requires (); };
 // expected-error@-1 {{expected expression}}
+
+bool r42 = requires { typename long; }; // expected-error {{expected a 
qualified name after 'typename'}}
+
+template 
+requires requires {
+ typename _BitInt(N); // expected-error {{expected a qualified name after 
'typename'}}
+} using r43 = void;
+
+template 
+using BitInt = _BitInt(N);
+
+template 
+requires requires {
+ typename BitInt; // ok
+} using r44 = void;



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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[ksyx via Phabricator via cfe-commits]

ksyx added a comment.

In D117520#3251425 , @MyDeveloperDay 
wrote:

> For
>
>   class Test
>   {
>   public:
>   static void foo()
>   {
>   }
>   };
>
> F21703157: image.png 
>
>   $ clang-format -n test.cxx
>   test.cxx:3:8: warning: code should be clang-formatted 
> [-Wclang-format-violations
>   ]
>   public:
>  ^
>
>
>
>   $ clang-format --output-replacements-xml test.cxx
>   
>   
>   

>   
>
> but when I run it nothing changes
>
>   $ clang-format test.cxx
>   class Test
>   {
>   public:
>   static void foo()
>   {
>   }
>   };
>
> And no character change
>
> F21703226: image.png 

Looks like it is just replacing indent whitespaces to tabs


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

https://reviews.llvm.org/D117520

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

[clang-tools-extra] c6fb636 - [clangd][clang-tidy] Remove uses of `std::vector`

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-18T17:59:40+01:00
New Revision: c6fb636667b879bfc8329d2c9ca5873871b46a7b

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

LOG: [clangd][clang-tidy] Remove uses of `std::vector`

LLVM Programmer’s Manual strongly discourages the use of `std::vector` 
and suggests `llvm::BitVector` as a possible replacement.

This patch does just that for clangd and clang-tidy.

Reviewed By: dexonsmith

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clangd/Selection.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index a76fe16effb6..66f60ec60b60 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -863,7 +864,7 @@ void 
ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
 }
   }
 
-  std::vector Apply(ErrorFixes.size(), true);
+  llvm::BitVector Apply(ErrorFixes.size(), true);
   for (auto &FileAndEvents : FileEvents) {
 std::vector &Events = FileAndEvents.second;
 // Sweep.

diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 69e99a9a8e28..cc698631be03 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
@@ -260,7 +261,7 @@ class SelectionTester {
 });
 auto Sel = llvm::makeArrayRef(SelFirst, SelLimit);
 // Find which of these are preprocessed to nothing and should be ignored.
-std::vector PPIgnored(Sel.size(), false);
+llvm::BitVector PPIgnored(Sel.size(), false);
 for (const syntax::TokenBuffer::Expansion &X :
  Buf.expansionsOverlapping(Sel)) {
   if (X.Expanded.empty()) {



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

[PATCH] D117119: [clangd][clang-tidy] Remove uses of `std::vector`

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6fb636667b8: [clangd][clang-tidy] Remove uses of 
`std::vector` (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117119

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  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
@@ -24,6 +24,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
@@ -260,7 +261,7 @@
 });
 auto Sel = llvm::makeArrayRef(SelFirst, SelLimit);
 // Find which of these are preprocessed to nothing and should be ignored.
-std::vector PPIgnored(Sel.size(), false);
+llvm::BitVector PPIgnored(Sel.size(), false);
 for (const syntax::TokenBuffer::Expansion &X :
  Buf.expansionsOverlapping(Sel)) {
   if (X.Expanded.empty()) {
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -863,7 +864,7 @@
 }
   }
 
-  std::vector Apply(ErrorFixes.size(), true);
+  llvm::BitVector Apply(ErrorFixes.size(), true);
   for (auto &FileAndEvents : FileEvents) {
 std::vector &Events = FileAndEvents.second;
 // Sweep.


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
@@ -260,7 +261,7 @@
 });
 auto Sel = llvm::makeArrayRef(SelFirst, SelLimit);
 // Find which of these are preprocessed to nothing and should be ignored.
-std::vector PPIgnored(Sel.size(), false);
+llvm::BitVector PPIgnored(Sel.size(), false);
 for (const syntax::TokenBuffer::Expansion &X :
  Buf.expansionsOverlapping(Sel)) {
   if (X.Expanded.empty()) {
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -863,7 +864,7 @@
 }
   }
 
-  std::vector Apply(ErrorFixes.size(), true);
+  llvm::BitVector Apply(ErrorFixes.size(), true);
   for (auto &FileAndEvents : FileEvents) {
 std::vector &Events = FileAndEvents.second;
 // Sweep.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117560: [C++20][Concepts] Fix a failed assertion on an invalid typename requirement

[Aaron Ballman via Phabricator via cfe-commits]

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

I've commit in fa596fb0779ae9029edbcff80ff95e9d1a816206 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117560

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

[clang] 262cc74 - Fix pair construction with an implicit constructor inside.

[Tres Popp via cfe-commits]

Author: Tres Popp
Date: 2022-01-18T18:01:52+01:00
New Revision: 262cc74e0b699071ff3e2627140b02d01ae2aa7e

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

LOG: Fix pair construction with an implicit constructor inside.

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 1b61b4982931..e2209e3debfd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -469,7 +469,7 @@ void GenericTaintRuleParser::consumeRulesFromConfig(const 
Config &C,
   llvm::SmallVector CallDescParts{NameParts.size()};
   llvm::transform(NameParts, CallDescParts.begin(),
   [](SmallString<32> &S) { return S.c_str(); });
-  Rules.emplace_back(CallDescParts, std::move(Rule));
+  Rules.emplace_back(CallDescription(CallDescParts), std::move(Rule));
 }
 
 void GenericTaintRuleParser::parseConfig(const std::string &Option,



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

[PATCH] D117120: [Doc] Add documentation for the clang-offload-wrapper tool (NFC)

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

Ping :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117120

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

[PATCH] D117567: [clang][dataflow] Add a test to justify skipping past references in UO_Deref

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

xazax.hun added a comment.

Is it planned to support templated code this way? 
I'm not entirely sure if this is worth pursuing as opposed to checking 
instantiations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117567

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

[PATCH] D117348: [Preprocessor] Reduce the memory overhead of `#define` directives

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Just some minor nits from me, but generally LG.




Comment at: clang/include/clang/Lex/MacroInfo.h:243
 
-  using tokens_iterator = SmallVectorImpl::const_iterator;
+  using tokens_iterator = const Token *;
+

I think this should be a `const_tokens_iterator` instead (and it's fine that we 
don't expose a non-const interface for the iterator).



Comment at: clang/include/clang/Lex/MacroInfo.h:256
+  allocateTokens(unsigned NumTokens, llvm::BumpPtrAllocator &PPAllocator) {
+NumReplacementTokens = NumTokens;
+Token *NewReplacementTokens = PPAllocator.Allocate(NumTokens);

Should we assert that we've not already allocated tokens before?



Comment at: clang/lib/Lex/MacroInfo.cpp:33
+
+// MacroInfo is expected to take 40 bytes on platforms with an 8 byte pointer.
+template  class MacroInfoSizeChecker {

Should we do this dance for 32-bit systems as well?



Comment at: clang/lib/Lex/MacroInfo.cpp:59
 
+  auto ReplacementTokens = tokens();
   if (ReplacementTokens.empty())

Please spell out the type.


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

https://reviews.llvm.org/D117348

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

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+  Class = N->ASTNode.get();
+if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+  return false;

njames93 wrote:
> avogelsgesang wrote:
> > do we also need to exclude anonymous class declarations here? (Not sure if 
> > those are also modelled as `CXXRecordDecl` in the clang AST...)
> Good point, should also ensure there is a test case for this as well.
On 2nd thought: To trigger this tweak, I click on the class name, and since 
anonymous structs don't have class names, I think the tweak can't even be 
triggered.

Still probably worth a check here, just to be sure. Or at least an `assert`+ 
comment in the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514

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

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D117129#3246403 , @njames93 wrote:

> In D117129#3239514 , 
> @LegalizeAdulthood wrote:
>
>> In D117129#3239143 , @njames93 
>> wrote:
>>
>>> My proposed idea was putting the `IncludeInserter` instance
>>> inside the `ClangTidyContext`, but then only registering it if
>>> any checks actually want to use it.
>>
>> Use of the `IncludeInserter` would be signaled by an argument
>> to the ctor fo r`ClangTidyCheck` in the c'tor for the derived
>> class?
>
> There is a hurdle to overcome with this, Currently each check can have its 
> own include style - which doesn't really make sense.

Hmm, I think it could make sense for sizeable projects that have components 
written by separate teams. Granted, I think it's a bit of a strange situation, 
but I wouldn't go so far as to say it doesn't make sense.


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

https://reviews.llvm.org/D117129

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