[PATCH] D146874: [clangd] Fix a hover crash on unsigned 64bit value

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:396
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {

Just to make sure I'm not missing anything: this comment change is just fixing 
a pre-existing mistake in the comment (-2^32 was already printed as 
0x), right? i.e. this patch doesn't change how any number is 
printed



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2953
+constexpr unsigned long value = -1; // wrap around
+void foo() { va$1^lue; }
+  )cpp");

you can just write `va^lue` and access it as `T.point()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146874

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

[PATCH] D146904: [clang-tidy] Fix extern fixes in readability-redundant-declaration

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Currently check does not look into LinkageSpecDecl
when removing redundant variable re-declarations.
This were leaving code in non-compiling state.
Fix patch fixes this and adds removal also of 'extern C'.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146904

Files:
  clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
@@ -120,3 +120,9 @@
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' 
declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dumyBegin;extern "C" int externX;int dummyEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant 'externX' declaration 
[readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dumyBegin;int dummyEnd;{{$}}
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -35,7 +35,8 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasAncestor(friendDecl()))
+  hasAncestor(friendDecl()),
+optionally(hasParent(linkageSpecDecl().bind("extern"
   .bind("Decl"),
   this);
 }
@@ -78,9 +79,17 @@
   D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
 auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-if (!MultiVar && !DifferentHeaders)
-  Diag << FixItHint::CreateRemoval(
-  SourceRange(D->getSourceRange().getBegin(), EndLoc));
+if (!MultiVar && !DifferentHeaders) {
+  SourceLocation BeginLoc;
+  if (const LinkageSpecDecl *Extern =
+  Result.Nodes.getNodeAs("extern");
+  Extern && !Extern->hasBraces())
+BeginLoc = Extern->getExternLoc();
+  else
+BeginLoc = D->getSourceRange().getBegin();
+
+  Diag << FixItHint::CreateRemoval(SourceRange(BeginLoc, EndLoc));
+}
   }
   diag(Prev->getLocation(), "previously declared here", DiagnosticIDs::Note);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
@@ -120,3 +120,9 @@
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dumyBegin;extern "C" int externX;int dummyEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant 'externX' declaration [readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dumyBegin;int dummyEnd;{{$}}
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -35,7 +35,8 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasAncestor(friendDecl()))
+  hasAncestor(friendDecl()),
+optionally(hasParent(linkageSpecDecl().bind("extern"
   .bind("Decl"),
   this);
 }
@@ -78,9 +79,17 @@
   D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
 auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-if (!MultiVar && !DifferentHeaders)
-  Diag << FixItHint::CreateRemoval(
-  SourceRange(D->getSourceRange().getBegin(), EndLoc));
+if (!MultiVar && !DifferentHeaders) {
+  SourceLocation BeginLoc;
+  if (const LinkageSpecDecl *Extern =
+

[PATCH] D146874: [clangd] Fix a hover crash on unsigned 64bit value

[Younan Zhang via Phabricator via cfe-commits]

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

Thank you!




Comment at: clang-tools-extra/clangd/Hover.cpp:396
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {

nridge wrote:
> Just to make sure I'm not missing anything: this comment change is just 
> fixing a pre-existing mistake in the comment (-2^32 was already printed as 
> 0x), right? i.e. this patch doesn't change how any number is 
> printed
No, the patch doesn't change the behavior of the function.  
`0xfffe` actually stands for `-(2**32 + 1)`. :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146874

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

[PATCH] D146874: [clangd] Fix a hover crash on unsigned 64bit value

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 508401.
zyounan marked an inline comment as done.
zyounan added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146874

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2947,6 +2947,15 @@
 getHover(AST, P, format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashAPInt64) {
+  Annotations T(R"cpp(
+constexpr unsigned long value = -1; // wrap around
+void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -393,9 +393,9 @@
 // 100   => 0x64
 // Negative numbers are sign-extended to 32/64 bits
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getExtValue();
+  uint64_t Bits = V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2947,6 +2947,15 @@
 getHover(AST, P, format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashAPInt64) {
+  Annotations T(R"cpp(
+constexpr unsigned long value = -1; // wrap around
+void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -393,9 +393,9 @@
 // 100   => 0x64
 // Negative numbers are sign-extended to 32/64 bits
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getExtValue();
+  uint64_t Bits = V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
alvinhochun added reviewers: mstorsjo, MaskRay.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As stated in https://github.com/llvm/llvm-project/issues/61685, by
passing LLD the import lib of the asan DLL first, the asan DLL will be
listed as the first entry in the Import Directory Table, making it be
loaded first before other user DLLs. This allows asan to be initialized
as early as possible to increase its instrumentation coverage to include
other DLLs not built with asan.

This also avoids some false asan reports on `realloc` for memory
allocated during initialization of user DLLs being loaded earlier than
asan, because after this change they will be loaded later than asan.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,18 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,29 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan flags before other libs and objects. Making asan_dynamic the 
first
+  // import lib allows asan to be initialized as early as possible to increase
+  // its instrumentation coverage to include other user DLLs which has not been
+  // built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--require-defined");
+CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
+  ? "___asan_seh_interceptor"
+  : "__asan_seh_interceptor");
+// Make sure the linker consider all object files from the dynamic
+// runtime thunk.
+CmdArgs.push_back("--whole-archive");
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--no-whole-archive");
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));
@@ -292,24 +315,6 @@
   if (Args.hasArg(options::OPT_pthread))
 CmdArgs.push_back("-lpthread");
 
-  if (Sanitize.needsAsanRt()) {
-// MinGW always links against a shared MSVCRT.
-CmdArgs.push_back

[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D146148#4221651 , @rjmccall wrote:

> In D146148#4220591 , @zahiraam 
> wrote:
>
>> In D146148#4220495 , 
>> @aaron.ballman wrote:
>>
>>> In D146148#4220433 , @zahiraam 
>>> wrote:
>>>
 In D146148#4220268 , @rjmccall 
 wrote:

> Okay, so modifying `math.h` to use this attribute is acceptable?  That's 
> great, that's definitely the best outcome for the compiler.
>
> Just a minor request about the diagnostic name, but otherwise LGTM.

 Yes. Added the attribute inside the included math.h header file.
>>>
>>> How does this work for, say, glibc, musl, or MSVC CRT? Won't those math.h 
>>> headers lack the attribute and thus run into problems when used with Clang?
>>
>> Good point! @rjmccall are you thinking of something in particular with the 
>> attribute?
>
> Zahira, this is what I was asking you when I asked whether modifying the 
> math.h header was acceptable: whether you were prepared to accept that the 
> warning would only fire on system math.h headers that we'd modified, or 
> whether you cared about making it work with non-cooperative headers.  I 
> wasn't asking if you were willing to change the test code.

Okay sorry about the confusion. I think the diagnostic should fire when the 
user is including system's math.h and using float_t inside a scope cotaining a 
#pragma clang fp eval-method. 
I am not sure what you mean by "cooperative headers"?

>> If not I guess we will have to rely on string comparison for all the typedef 
>> in the TU. Aaron pointed out the compile time overhead.
>
> Well, the compile-time overhead of doing this on every typedef *declaration* 
> is way better than the overhead of doing this on every type lookup, at least.
>
> Anyway, there are three possibilities I can see:
>
> - We accept that this needs cooperative system headers.

Not sure what you mean by cooperative system headers.

> - We add a math.h compiler header that `#include_next`s the system math.h and 
> then adds the attribute.  I believe you can just add an attribute to a 
> typedef retroactively with something like `typedef float_t float_t 
> __attribute__((whatever))`.

So when a user writes:

  #include 
   int foo()
   {
  #pragma clang fp eval_method(source)
 return sizeof(float_t);
   }

It would be as though the user wrote:

  #include "math.h"
  int foo()
  {
 #pragma clang fp eval_method(source)
return sizeof(float_t);
  }

where the content of this new math header being:

  #include_next 
  typedef float_t float_t __attribute__((whatever));
  typedef double_t double_t __attribute__((whatever));

The compiler would have to recognize that the included file is math.h and 
create a new math.h. Where would this file reside?

> - We do checks on every typedef declaration.  There's a builtin-identifier 
> trick that we do with library functions that we should be able to generalize 
> to typedefs, so you wouldn't need to actually do string comparisons, you'd 
> just check whether the `IdentifierInfo*` was storing a special ID.  We'd set 
> that up in `initializeBuiltins` at the start of the translation unit, so the 
> overhead would just be that we'd create two extra `IdentifierInfo*`s in every 
> TU.
>
> The builtin ID logic is currently specific to builtin *functions*, and I 
> don't think we'd want to hack typedef names into that.  But the same storage 
> in `IdentifierInfo*` is also used for identifying the ObjC context-sensitive 
> keywords, by just offsetting the builtin IDs by the NUM_OBJC_KEYWORD.  You 
> should be able to generalize that by also introducing a concept of a builtin 
> *type* name, so that e.g. IDs in [0,NUM_OBJC_KEYWORDS) are ObjCKeywordKinds, 
> IDs in [NUM_OBJC_KEYWORDS, NUM_OBJC_KEYWORDS+NUM_BUILTIN_TYPES) are 
> BuiltinTypeKinds (when you subtract NUM_OBJC_KEYWORDS), and IDs in 
> [NUM_OBJC_KEYWORDS+NUM_BUILTIN_TYPES,∞) are builtin function IDs (when you 
> subtract NUM_OBJC_KEYWORDS+NUM_BUILTIN_TYPES).

Need to look at this more closely to understand what you are suggesting.

@rjmccall do you have a preference for any one of these solutions? 
@aaron.ballman ?
Thanks.


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

https://reviews.llvm.org/D146148

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

[PATCH] D146913: [clang-tidy] Add option to ignore user-defined literals in readability-magic-numbers

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp created this revision.
Herald added subscribers: PiotrZSL, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Some user-defined literals operate on implementation-defined types, like
"unsigned long long" and "long double", which are not well supported by
this check. Currently, the check gives warnings when using UDLs, without
giving possiblity to the user to whitelist common UDLs. A good compromise
until a proper fix is found (if any) is to allow the user to disable
warnings on UDLs.

Partially fixes #61656


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146913

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
@@ -1,10 +1,18 @@
-// RUN: %check_clang_tidy -std=c++14-or-later %s readability-magic-numbers %t --
+// RUN: %check_clang_tidy -check-suffixes=UDL-ALLOWED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: false}]}' \
+// RUN: --
+// RUN: %check_clang_tidy -check-suffixes=UDL-IGNORED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: true}]}' \
+// RUN: --
 
 namespace std {
   class string {};
   using size_t = decltype(sizeof(int));
   string operator ""s(const char *, std::size_t);
   int operator "" s(unsigned long long);
+  float operator "" s(long double);
 }
 
 void UserDefinedLiteral() {
@@ -12,5 +20,9 @@
   "Hello World"s;
   const int i = 3600s;
   int j = 3600s;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-2]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  float k = 3600.0s;
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 }
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -57,7 +57,7 @@
 
const size_t NUMBER_OF_ELEMENTS = 30;
using containerType = CustomType;
-   
+
struct OtherType {
   containerType container;
}
@@ -144,3 +144,8 @@
 
Boolean value indicating whether to accept magic numbers in ``typedef`` or
``using`` declarations. Default value is `false`.
+
+.. option:: IgnoreUserDefinedLiterals
+
+   Boolean value indicating whether to accept magic numbers in user-defined
+   literals. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -211,6 +211,9 @@
   behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix`
   option to `i` instead of using `EnumConstantHungarianPrefix`.
 
+- Added support to optionally ignore user-defined literals in
+  :doc:`readability-magic-numbers`.
+
 - Fixed a false positive in :doc:`readability-container-size-empty
   ` check when comparing
   ``std::array`` objects to default constructed ones. The behavior for this and
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -49,6 +49,15 @@
   bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult &Result,
const IntegerLiter

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D145617#4221369 , @PiotrZSL wrote:

> @carlosgalvezp Do you want me to shorten documentation ? Or we leave it like 
> it is ?

It's fine for me either way!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145617

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

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp 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/D145617/new/

https://reviews.llvm.org/D145617

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

[PATCH] D146913: [clang-tidy] Add option to ignore user-defined literals in readability-magic-numbers

[Piotr Zegar via Phabricator via cfe-commits]

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

LGTM




Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h:52-59
+  bool isUserDefinedLiteral(
+  const clang::ast_matchers::MatchFinder::MatchResult &Result,
+  const clang::Expr *Literal) const {
+ auto Parents = Result.Context->getParents(*Literal);
+ if (Parents.empty())
+   return false;
+ return Parents[0].get() != nullptr;

Move this to CPP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146913

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

[clang-tools-extra] 52296f5 - [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-26T13:20:14Z
New Revision: 52296f5ed88b20af29cac517e349b221ed84cc2a

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

LOG: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

Check flags always enabled or disabled code blocks in preprocessor '#if'
conditions, such as '#if 0' and '#if 1' etc.

Reviewed By: carlosgalvezp

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

Added: 

clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp

clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h

clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst

clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
 
b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
new file mode 100644
index 0..d92d0e8f2dbf7
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
@@ -0,0 +1,104 @@
+//===--- AvoidUnconditionalPreprocessorIfCheck.cpp - clang-tidy 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidUnconditionalPreprocessorIfCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+struct AvoidUnconditionalPreprocessorIfPPCallbacks : public PPCallbacks {
+
+  explicit AvoidUnconditionalPreprocessorIfPPCallbacks(ClangTidyCheck &Check,
+   Preprocessor &PP)
+  : Check(Check), PP(PP) {}
+
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+  ConditionValueKind ConditionValue) override {
+if (ConditionValue == CVK_NotEvaluated)
+  return;
+SourceManager &SM = PP.getSourceManager();
+if (!isImmutable(SM, PP.getLangOpts(), ConditionRange))
+  return;
+
+if (ConditionValue == CVK_True)
+  Check.diag(Loc, "preprocessor condition is always 'true', consider "
+  "removing condition but leaving its contents");
+else
+  Check.diag(Loc, "preprocessor condition is always 'false', consider "
+  "removing both the condition and its contents");
+  }
+
+  bool isImmutable(SourceManager &SM, const LangOptions &LangOpts,
+   SourceRange ConditionRange) {
+SourceLocation Loc = ConditionRange.getBegin();
+if (Loc.isMacroID())
+  return false;
+
+Token Tok;
+if (Lexer::getRawToken(Loc, Tok, SM, LangOpts, true)) {
+  std::optional TokOpt = Lexer::findNextToken(Loc, SM, LangOpts);
+  if (!TokOpt || TokOpt->getLocation().isMacroID())
+return false;
+  Tok = *TokOpt;
+}
+
+while (Tok.getLocation() <= ConditionRange.getEnd()) {
+  if (!isImmutableToken(Tok))
+return false;
+
+  std::optional TokOpt =
+  Lexer::findNextToken(Tok.getLocation(), SM, LangOpts);
+  if (!TokOpt || TokOpt->getLocation().isMacroID())
+return false;
+  Tok = *TokOpt;
+}
+
+return true;
+  }
+
+  bool isImmutableToken(const Token &Tok) {
+switch (Tok.getKind()) {
+case tok::eod:
+case tok::eof:
+case tok::numeric_constant:
+case tok::char_constant:
+case tok::wide_char_constant:
+case tok::utf8_char_constant:
+case tok::utf16_char_constant:
+case tok::utf32_char_constant:
+case tok::string_literal:
+case tok::wide_string_literal:
+case tok::comment:
+  return true;
+case tok::raw_identifier:
+  return (Tok.getRawIdentifier() == "true" ||
+  Tok.getRawIdentifier() == "false");
+default:
+  return Tok.getKind() >= tok::l_square && Tok.getKind() <= 
tok::caretcaret;
+}
+  }
+
+  ClangTidyCheck &Check;
+  Preprocessor &PP;
+};
+
+} // namespace
+
+void AvoidUnconditionalPreprocessorIfCheck::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(
+  std::make_unique(*this,
+ 

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG52296f5ed88b: [clang-tidy] Add 
readability-avoid-unconditional-preprocessor-if check (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D145617?vs=508321&id=508408#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145617

Files:
  
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
  
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  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/avoid-unconditional-preprocessor-if.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
@@ -0,0 +1,94 @@
+// RUN: %check_clang_tidy %s readability-avoid-unconditional-preprocessor-if %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if 0
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if 1
+// some code
+#endif
+
+#if test
+
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10>5
+
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10<5
+
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10 > 5
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10 < 5
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if !(10 > \
+5)
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if !(10 < \
+5)
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if true
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if false
+// some code
+#endif
+
+#define MACRO
+#ifdef MACRO
+// some code
+#endif
+
+#if !SOMETHING
+#endif
+
+#if !( \
+ defined MACRO)
+// some code
+#endif
+
+
+#if __has_include()
+// some code
+#endif
+
+#if __has_include()
+// some code
+#endif
+
+#define DDD 17
+#define EEE 18
+
+#if 10 > DDD
+// some code
+#endif
+
+#if 10 < DDD
+// some code
+#endif
+
+#if EEE > DDD
+// some code
+#endif
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-avoid-unconditional-preprocessor-if
+
+readability-avoid-unconditional-preprocessor-if
+===
+
+Finds code blocks that are constantly enabled or disabled in preprocessor
+directives by analyzing ``#if`` conditions, such as ``#if 0`` and ``#if 1``,
+etc.
+
+.. code-block:: c++
+
+#if 0
+// some disabled code
+#endif
+
+#if 1
+   // some enabled code that can be disabled manually
+#endif
+
+Unconditional preprocessor directives, such as ``#if 0`` for disabled code
+and ``#if 1`` for enabled code, can lead to dead code and always enabled code,
+

[clang-tools-extra] eb87e55 - [clang-tidy] Correct union & macros handling in modernize-use-equals-default

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-26T13:22:44Z
New Revision: eb87e55c9aade5d7b42606487e69b5f3a6da1e65

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

LOG: [clang-tidy] Correct union & macros handling in 
modernize-use-equals-default

To this moment this check were ignoring only inline
union special members, From now also out-of-line
special members going to be ignored. Also extended
support for IgnoreMacros to cover also macros used
inside a body, or used preprocesor directives.

Fixes:
 - https://github.com/llvm/llvm-project/issues/28300
 - https://github.com/llvm/llvm-project/issues/40554

Reviewed By: alexander-shaposhnikov

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index d48d43f2677f0..afac9f7497ac8 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -235,19 +235,19 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder 
*Finder) {
 
   // Destructor.
   Finder->addMatcher(
-  cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
+  cxxDestructorDecl(isDefinition(), unless(ofClass(IsUnionLikeClass)))
   .bind(SpecialFunction),
   this);
+  // Constructor.
   Finder->addMatcher(
   cxxConstructorDecl(
-  unless(
-  hasParent(decl(anyOf(IsUnionLikeClass, 
functionTemplateDecl(),
-  isDefinition(),
+  isDefinition(), unless(ofClass(IsUnionLikeClass)),
+  unless(hasParent(functionTemplateDecl())),
   anyOf(
   // Default constructor.
-  allOf(unless(hasAnyConstructorInitializer(isWritten())),
-unless(isVariadic()), parameterCountIs(0),
-IsPublicOrOutOfLineUntilCPP20),
+  allOf(parameterCountIs(0),
+unless(hasAnyConstructorInitializer(isWritten())),
+unless(isVariadic()), IsPublicOrOutOfLineUntilCPP20),
   // Copy constructor.
   allOf(isCopyConstructor(),
 // Discard constructors that can be used as a copy
@@ -258,9 +258,9 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder 
*Finder) {
   this);
   // Copy-assignment operator.
   Finder->addMatcher(
-  cxxMethodDecl(unless(hasParent(
-decl(anyOf(IsUnionLikeClass, 
functionTemplateDecl(),
-isDefinition(), isCopyAssignmentOperator(),
+  cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+unless(ofClass(IsUnionLikeClass)),
+unless(hasParent(functionTemplateDecl())),
 // isCopyAssignmentOperator() allows the parameter to be
 // passed by value, and in this case it cannot be
 // defaulted.
@@ -299,6 +299,12 @@ void UseEqualsDefaultCheck::check(const 
MatchFinder::MatchResult &Result) {
   if (!SpecialFunctionDecl->isCopyAssignmentOperator() && !Body->body_empty())
 return;
 
+  // If body contain any preprocesor derictives, don't warn.
+  if (IgnoreMacros && utils::lexer::rangeContainsExpansionsOrDirectives(
+  Body->getSourceRange(), *Result.SourceManager,
+  Result.Context->getLangOpts()))
+return;
+
   // If there are comments inside the body, don't do the change.
   bool ApplyFix = SpecialFunctionDecl->isCopyAssignmentOperator() ||
   bodyEmpty(Result.Context, Body);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 38a64616410ac..1a53aac8fecf4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -203,6 +203,11 @@ Changes in existing checks
   constructors toward hand written constructors so that they are skipped if 
more
   than one exists.
 
+- Fixed false positive in :doc:`modernize-use-equals-default
+  ` check for special member
+  functions containing macros or preprocessor directives, and out-of-line 
special
+  member functions in unions.
+
 - Fixed reading `HungarianNotation.CString.*` options in
   :doc:`readability-identifier-naming
   ` check.

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst 
b/clang-tools-extra/docs/cla

[PATCH] D146882: [clang-tidy] Correct union & macros handling in modernize-use-equals-default

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb87e55c9aad: [clang-tidy] Correct union & macros 
handling in modernize-use-equals-default (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146882

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -42,6 +42,18 @@
   NE Field;
 };
 
+// Skip unions with out-of-line constructor/destructor.
+union NUO {
+  NUO();
+  ~NUO();
+  NE Field;
+};
+
+NUO::NUO() {}
+// CHECK-FIXES: NUO::NUO() {}
+NUO::~NUO() {}
+// CHECK-FIXES: NUO::~NUO() {}
+
 // Skip structs/classes containing anonymous unions.
 struct SU {
   SU() {}
@@ -266,3 +278,21 @@
   };
 
 STRUCT_WITH_DEFAULT(unsigned char, InMacro)
+
+#define ZERO_VALUE 0
+struct PreprocesorDependentTest
+{
+  void something();
+
+  PreprocesorDependentTest() {
+#if ZERO_VALUE
+something();
+#endif
+  }
+
+  ~PreprocesorDependentTest() {
+#if ZERO_VALUE
+something();
+#endif
+  }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
@@ -32,5 +32,6 @@
 
 .. option:: IgnoreMacros
 
-   If set to `true`, the check will not give warnings inside macros. Default
-   is `true`.
+   If set to `true`, the check will not give warnings inside macros and will
+   ignore special members with bodies contain macros or preprocessor directives.
+   Default is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -203,6 +203,11 @@
   constructors toward hand written constructors so that they are skipped if more
   than one exists.
 
+- Fixed false positive in :doc:`modernize-use-equals-default
+  ` check for special member
+  functions containing macros or preprocessor directives, and out-of-line special
+  member functions in unions.
+
 - Fixed reading `HungarianNotation.CString.*` options in
   :doc:`readability-identifier-naming
   ` check.
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -235,19 +235,19 @@
 
   // Destructor.
   Finder->addMatcher(
-  cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
+  cxxDestructorDecl(isDefinition(), unless(ofClass(IsUnionLikeClass)))
   .bind(SpecialFunction),
   this);
+  // Constructor.
   Finder->addMatcher(
   cxxConstructorDecl(
-  unless(
-  hasParent(decl(anyOf(IsUnionLikeClass, functionTemplateDecl(),
-  isDefinition(),
+  isDefinition(), unless(ofClass(IsUnionLikeClass)),
+  unless(hasParent(functionTemplateDecl())),
   anyOf(
   // Default constructor.
-  allOf(unless(hasAnyConstructorInitializer(isWritten())),
-unless(isVariadic()), parameterCountIs(0),
-IsPublicOrOutOfLineUntilCPP20),
+  allOf(parameterCountIs(0),
+unless(hasAnyConstructorInitializer(isWritten())),
+unless(isVariadic()), IsPublicOrOutOfLineUntilCPP20),
   // Copy constructor.
   allOf(isCopyConstructor(),
 // Discard constructors that can be used as a copy
@@ -258,9 +258,9 @@
   this);
   // Copy-assignment operator.
   Finder->addMatcher(
-  cxxMethodDecl(unless(hasParent(
-decl(anyOf(IsUnionLikeClass, functionTemplateDecl(),
-isDefinition(), isCopyAssignmentOperator(),
+  cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+unless(ofClass(IsUnionLikeClass)),
+unless(hasParent(functionTemplateDecl())),
 // isCopyAssignmentOperator() allows the parameter to be
 // passed by value, and in this case it cannot be
 // defaulted.
@@ -299,6 +299,12 @@
   if (!SpecialFunctionDecl->isCopyAssig

[PATCH] D146913: [clang-tidy] Add option to ignore user-defined literals in readability-magic-numbers

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 508410.
carlosgalvezp added a comment.

Move implementation to .cpp file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146913

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
@@ -1,10 +1,18 @@
-// RUN: %check_clang_tidy -std=c++14-or-later %s readability-magic-numbers %t --
+// RUN: %check_clang_tidy -check-suffixes=UDL-ALLOWED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: false}]}' \
+// RUN: --
+// RUN: %check_clang_tidy -check-suffixes=UDL-IGNORED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: true}]}' \
+// RUN: --
 
 namespace std {
   class string {};
   using size_t = decltype(sizeof(int));
   string operator ""s(const char *, std::size_t);
   int operator "" s(unsigned long long);
+  float operator "" s(long double);
 }
 
 void UserDefinedLiteral() {
@@ -12,5 +20,9 @@
   "Hello World"s;
   const int i = 3600s;
   int j = 3600s;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-2]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  float k = 3600.0s;
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 }
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -57,7 +57,7 @@
 
const size_t NUMBER_OF_ELEMENTS = 30;
using containerType = CustomType;
-   
+
struct OtherType {
   containerType container;
}
@@ -144,3 +144,8 @@
 
Boolean value indicating whether to accept magic numbers in ``typedef`` or
``using`` declarations. Default value is `false`.
+
+.. option:: IgnoreUserDefinedLiterals
+
+   Boolean value indicating whether to accept magic numbers in user-defined
+   literals. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -211,6 +211,9 @@
   behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix`
   option to `i` instead of using `EnumConstantHungarianPrefix`.
 
+- Added support to optionally ignore user-defined literals in
+  :doc:`readability-magic-numbers`.
+
 - Fixed a false positive in :doc:`readability-container-size-empty
   ` check when comparing
   ``std::array`` objects to default constructed ones. The behavior for this and
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -49,6 +49,10 @@
   bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult &Result,
const IntegerLiteral &Literal) const;
 
+  bool isUserDefinedLiteral(
+  const clang::ast_matchers::MatchFinder::MatchResult &Result,
+  const clang::Expr &Literal) const;
+
   template 
   void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result,
const char *BoundName) {
@@ -72,6 +76,10 @@
 if (isBitFieldWidth(Result, *MatchedLiteral))
   return;
 
+if (IgnoreUserDefinedLiterals &&
+isUserDefinedLiteral(Result, *MatchedLiteral))
+  return;

[PATCH] D146904: [clang-tidy] Fix extern fixes in readability-redundant-declaration

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp:84
+  SourceLocation BeginLoc;
+  if (const LinkageSpecDecl *Extern =
+  Result.Nodes.getNodeAs("extern");

`auto` could be used because type is explicitly stated in same statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146904

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

[PATCH] D146904: [clang-tidy] Fix extern fixes in readability-redundant-declaration

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 508411.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Use auto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146904

Files:
  clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
@@ -120,3 +120,9 @@
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' 
declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dumyBegin;extern "C" int externX;int dummyEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant 'externX' declaration 
[readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dumyBegin;int dummyEnd;{{$}}
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -35,7 +35,8 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasAncestor(friendDecl()))
+  hasAncestor(friendDecl()),
+optionally(hasParent(linkageSpecDecl().bind("extern"
   .bind("Decl"),
   this);
 }
@@ -78,9 +79,17 @@
   D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
 auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-if (!MultiVar && !DifferentHeaders)
-  Diag << FixItHint::CreateRemoval(
-  SourceRange(D->getSourceRange().getBegin(), EndLoc));
+if (!MultiVar && !DifferentHeaders) {
+  SourceLocation BeginLoc;
+  if (const auto *Extern =
+  Result.Nodes.getNodeAs("extern");
+  Extern && !Extern->hasBraces())
+BeginLoc = Extern->getExternLoc();
+  else
+BeginLoc = D->getSourceRange().getBegin();
+
+  Diag << FixItHint::CreateRemoval(SourceRange(BeginLoc, EndLoc));
+}
   }
   diag(Prev->getLocation(), "previously declared here", DiagnosticIDs::Note);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
@@ -120,3 +120,9 @@
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dumyBegin;extern "C" int externX;int dummyEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant 'externX' declaration [readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dumyBegin;int dummyEnd;{{$}}
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -35,7 +35,8 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasAncestor(friendDecl()))
+  hasAncestor(friendDecl()),
+optionally(hasParent(linkageSpecDecl().bind("extern"
   .bind("Decl"),
   this);
 }
@@ -78,9 +79,17 @@
   D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
 auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-if (!MultiVar && !DifferentHeaders)
-  Diag << FixItHint::CreateRemoval(
-  SourceRange(D->getSourceRange().getBegin(), EndLoc));
+if (!MultiVar && !DifferentHeaders) {
+  SourceLocation BeginLoc;
+  if (const auto *Extern =
+  Result.Nodes.getNodeAs("extern");
+  Extern && !Extern->hasBraces())
+BeginLoc = Extern->getExternLoc();
+  else
+BeginLoc = D->getSourceRange().getBegin();
+
+  Diag << FixItHint::CreateRemoval(SourceRange(BeginLoc, EndLoc));
+}
   }
   diag(Prev->getLocation(), "previously declared here", DiagnosticIDs::Note);
 }
__

[PATCH] D146916: [include-cleaner] Fix crash on unresolved headers

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Make sure unresolved headers are not analyzed as part of unused
includes.

Also introduces a testing fixture for analyze tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146916

Files:
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -24,6 +24,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace clang::include_cleaner {
@@ -178,8 +179,31 @@
   Pair(Code.point("2"), UnorderedElementsAre(HdrFile;
 }
 
-TEST(Analyze, Basic) {
+class AnalyzeTest : public testing::Test {
+protected:
   TestInputs Inputs;
+  PragmaIncludes PI;
+  RecordedPP PP;
+  AnalyzeTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public SyntaxOnlyAction {
+  public:
+Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+  PI.record(CI);
+  return true;
+}
+
+RecordedPP &PP;
+PragmaIncludes Π
+  };
+  return std::make_unique(PP, PI);
+};
+  }
+};
+
+TEST_F(AnalyzeTest, Basic) {
   Inputs.Code = R"cpp(
 #include "a.h"
 #include "b.h"
@@ -194,53 +218,41 @@
   )cpp");
   Inputs.ExtraFiles["c.h"] = guard("int c;");
   Inputs.ExtraFiles["keep.h"] = guard("");
+  TestAST AST(Inputs);
+  auto Decls = AST.context().getTranslationUnitDecl()->decls();
+  auto Results =
+  analyze(std::vector{Decls.begin(), Decls.end()},
+  PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
+  AST.preprocessor().getHeaderSearchInfo());
 
-  RecordedPP PP;
-  PragmaIncludes PI;
-  Inputs.MakeAction = [&PP, &PI] {
-struct Hook : public SyntaxOnlyAction {
-public:
-  Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
-PI.record(CI);
-return true;
-  }
-
-  RecordedPP &PP;
-  PragmaIncludes Π
-};
-return std::make_unique(PP, PI);
-  };
-
-  {
-TestAST AST(Inputs);
-auto Decls = AST.context().getTranslationUnitDecl()->decls();
-auto Results =
-analyze(std::vector{Decls.begin(), Decls.end()},
-PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
-AST.preprocessor().getHeaderSearchInfo());
+  const Include *B = PP.Includes.atLine(3);
+  ASSERT_EQ(B->Spelled, "b.h");
+  EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
+  EXPECT_THAT(Results.Unused, ElementsAre(B));
+}
 
-const Include *B = PP.Includes.atLine(3);
-ASSERT_EQ(B->Spelled, "b.h");
-EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
-EXPECT_THAT(Results.Unused, ElementsAre(B));
-  }
+TEST_F(AnalyzeTest, PrivateUsedInPublic) {
+  // Check that umbrella header uses private include.
+  Inputs.Code = R"cpp(#include "private.h")cpp";
+  Inputs.ExtraFiles["private.h"] =
+  guard("// IWYU pragma: private, include \"public.h\"");
+  Inputs.FileName = "public.h";
+  TestAST AST(Inputs);
+  EXPECT_FALSE(PP.Includes.all().empty());
+  auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
+ AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
+}
 
+TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
   // Check that umbrella header uses private include.
-  {
-Inputs.Code = R"cpp(#include "private.h")cpp";
-Inputs.ExtraFiles["private.h"] =
-guard("// IWYU pragma: private, include \"public.h\"");
-Inputs.FileName = "public.h";
-PP.Includes = {};
-PI = {};
-TestAST AST(Inputs);
-EXPECT_FALSE(PP.Includes.all().empty());
-auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
-   AST.preprocessor().getHeaderSearchInfo());
-EXPECT_THAT(Results.Unused, testing::IsEmpty());
-  }
+  Inputs.Code = R"cpp(#include "not_found.h")cpp";
+  Inputs.ErrorOK = true;
+  TestAST AST(Inputs);
+  EXPECT_FALSE(PP.Includes.all().empty());
+  auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
+ AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
 }
 
 TEST(FixIncludes, Basic) {
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
==

[clang-tools-extra] f5b6e9b - [include-cleaner] Fix crash on unresolved headers

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-03-26T16:37:56+02:00
New Revision: f5b6e9b6d355866c2aa4e440438b870885ec0373

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

LOG: [include-cleaner] Fix crash on unresolved headers

Make sure unresolved headers are not analyzed as part of unused
includes.

Also introduces a testing fixture for analyze tests

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp 
b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index fb0879b7aab63..58402cce0b717 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -10,7 +10,6 @@
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
@@ -19,9 +18,13 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
+#include 
 
 namespace clang::include_cleaner {
 
@@ -91,7 +94,7 @@ AnalysisResults analyze(llvm::ArrayRef ASTRoots,
 
   AnalysisResults Results;
   for (const Include &I : Inc.all()) {
-if (Used.contains(&I))
+if (Used.contains(&I) || !I.Resolved)
   continue;
 if (PI) {
   if (PI->shouldKeep(I.Line))

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index a2084d4f37903..fc7d14582f632 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -8,22 +8,28 @@
 
 #include "clang-include-cleaner/Analysis.h"
 #include "AnalysisInternal.h"
+#include "TypesInternal.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 #include 
 
 namespace clang::include_cleaner {
@@ -178,8 +184,31 @@ TEST_F(WalkUsedTest, MacroRefs) {
   Pair(Code.point("2"), UnorderedElementsAre(HdrFile;
 }
 
-TEST(Analyze, Basic) {
+class AnalyzeTest : public testing::Test {
+protected:
   TestInputs Inputs;
+  PragmaIncludes PI;
+  RecordedPP PP;
+  AnalyzeTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public SyntaxOnlyAction {
+  public:
+Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+  PI.record(CI);
+  return true;
+}
+
+RecordedPP &PP;
+PragmaIncludes Π
+  };
+  return std::make_unique(PP, PI);
+};
+  }
+};
+
+TEST_F(AnalyzeTest, Basic) {
   Inputs.Code = R"cpp(
 #include "a.h"
 #include "b.h"
@@ -194,53 +223,41 @@ int x = a + c;
   )cpp");
   Inputs.ExtraFiles["c.h"] = guard("int c;");
   Inputs.ExtraFiles["keep.h"] = guard("");
+  TestAST AST(Inputs);
+  auto Decls = AST.context().getTranslationUnitDecl()->decls();
+  auto Results =
+  analyze(std::vector{Decls.begin(), Decls.end()},
+  PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
+  AST.preprocessor().getHeaderSearchInfo());
 
-  RecordedPP PP;
-  PragmaIncludes PI;
-  Inputs.MakeAction = [&PP, &PI] {
-struct Hook : public SyntaxOnlyAction {
-public:
-  Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
-PI.record(CI);
-return true;
-  }
-
-  RecordedPP &PP;
-  PragmaIncludes

[PATCH] D146916: [include-cleaner] Fix crash on unresolved headers

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5b6e9b6d355: [include-cleaner] Fix crash on unresolved 
headers (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D146916?vs=508415&id=508417#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146916

Files:
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -8,22 +8,28 @@
 
 #include "clang-include-cleaner/Analysis.h"
 #include "AnalysisInternal.h"
+#include "TypesInternal.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 #include 
 
 namespace clang::include_cleaner {
@@ -178,8 +184,31 @@
   Pair(Code.point("2"), UnorderedElementsAre(HdrFile;
 }
 
-TEST(Analyze, Basic) {
+class AnalyzeTest : public testing::Test {
+protected:
   TestInputs Inputs;
+  PragmaIncludes PI;
+  RecordedPP PP;
+  AnalyzeTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public SyntaxOnlyAction {
+  public:
+Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+  PI.record(CI);
+  return true;
+}
+
+RecordedPP &PP;
+PragmaIncludes Π
+  };
+  return std::make_unique(PP, PI);
+};
+  }
+};
+
+TEST_F(AnalyzeTest, Basic) {
   Inputs.Code = R"cpp(
 #include "a.h"
 #include "b.h"
@@ -194,53 +223,41 @@
   )cpp");
   Inputs.ExtraFiles["c.h"] = guard("int c;");
   Inputs.ExtraFiles["keep.h"] = guard("");
+  TestAST AST(Inputs);
+  auto Decls = AST.context().getTranslationUnitDecl()->decls();
+  auto Results =
+  analyze(std::vector{Decls.begin(), Decls.end()},
+  PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
+  AST.preprocessor().getHeaderSearchInfo());
 
-  RecordedPP PP;
-  PragmaIncludes PI;
-  Inputs.MakeAction = [&PP, &PI] {
-struct Hook : public SyntaxOnlyAction {
-public:
-  Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
-PI.record(CI);
-return true;
-  }
-
-  RecordedPP &PP;
-  PragmaIncludes Π
-};
-return std::make_unique(PP, PI);
-  };
-
-  {
-TestAST AST(Inputs);
-auto Decls = AST.context().getTranslationUnitDecl()->decls();
-auto Results =
-analyze(std::vector{Decls.begin(), Decls.end()},
-PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
-AST.preprocessor().getHeaderSearchInfo());
+  const Include *B = PP.Includes.atLine(3);
+  ASSERT_EQ(B->Spelled, "b.h");
+  EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
+  EXPECT_THAT(Results.Unused, ElementsAre(B));
+}
 
-const Include *B = PP.Includes.atLine(3);
-ASSERT_EQ(B->Spelled, "b.h");
-EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
-EXPECT_THAT(Results.Unused, ElementsAre(B));
-  }
+TEST_F(AnalyzeTest, PrivateUsedInPublic) {
+  // Check that umbrella header uses private include.
+  Inputs.Code = R"cpp(#include "private.h")cpp";
+  Inputs.ExtraFiles["private.h"] =
+  guard("// IWYU pragma: private, include \"public.h\"");
+  Inputs.FileName = "public.h";
+  TestAST AST(Inputs);
+  EXPECT_FALSE(PP.Includes.all().empty());
+  auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
+ AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
+}
 
+TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
   // Check that umbrella header uses private include.
-  {
-Inputs.Code = R"cpp(#i

[clang] a742511 - [clang][RISCV][test] Add test cases for empty structs and the FP calling conventions

[Alex Bradbury via cfe-commits]

Author: Alex Bradbury
Date: 2023-03-26T16:11:18+01:00
New Revision: a742511cbe9ada478e8a42847f443aa807bb147c

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

LOG: [clang][RISCV][test] Add test cases for empty structs and the FP calling 
conventions

As reported in https://github.com/llvm/llvm-project/issues/58929, Clang
currently differs from GCC in the handling of empty structs. This commit
adds some test coverage for the handling of such structs.

A follow-up patch implements a fix to match g++.

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

Added: 
clang/test/CodeGen/RISCV/abi-empty-structs.c

Modified: 


Removed: 




diff  --git a/clang/test/CodeGen/RISCV/abi-empty-structs.c 
b/clang/test/CodeGen/RISCV/abi-empty-structs.c
new file mode 100644
index 0..94cc29c95c293
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -0,0 +1,173 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --function-signature --full-function-signature --filter "^define 
|^entry:"
+// RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f 
-emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-feature +d 
-target-abi ilp32d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-abi lp64f 
-emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK64-C %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d 
-target-abi lp64d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK64-C %s
+// RUN: %clang_cc1 -x c++ -triple riscv32 -target-feature +f -target-abi 
ilp32f -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK32-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv32 -target-feature +f -target-feature 
+d -target-abi ilp32d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK32-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv64 -target-feature +f -target-abi lp64f 
-emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK64-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv64 -target-feature +f -target-feature 
+d -target-abi lp64d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK64-CXX %s
+
+#include 
+
+// Fields containing empty structs or unions are ignored when flattening
+// structs for the hard FP ABIs, even in C++.
+// FIXME: This isn't currently respected.
+
+struct empty { struct { struct { } e; }; };
+struct s1 { struct empty e; float f; };
+
+// CHECK-C-LABEL: define dso_local float @test_s1
+// CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
+// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
+// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK64-CXX:  entry:
+//
+struct s1 test_s1(struct s1 a) {
+  return a;
+}
+
+struct s2 { struct empty e; int32_t i; float f; };
+
+// CHECK-C-LABEL: define dso_local { i32, float } @test_s2
+// CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 
[[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX:  entry:
+//
+struct s2 test_s2(struct s2 a) {
+  return a;
+}
+
+struct s3 { struct empty e; float f; float g; };
+
+// CHECK-C-LABEL: define dso_local { float, float } @test_s3
+// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 
[[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX:  entry:
+//
+struct s3 test_s3(struct s3 a) {
+  return a;
+}
+
+struct s4 { struct empty e; float __complex__ c; };
+
+// CHECK-C-LABEL: define dso_local { float, float } @test_s4
+// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
+// CHECK32-CXX-SAME: (ptr noalia

[PATCH] D142326: [clang][RISCV][test] Add test cases for empty structs and the FP calling conventions

[Alex Bradbury 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 rGa742511cbe9a: [clang][RISCV][test] Add test cases for empty 
structs and the FP calling… (authored by asb).
Herald added a subscriber: jobnoorman.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142326

Files:
  clang/test/CodeGen/RISCV/abi-empty-structs.c

Index: clang/test/CodeGen/RISCV/abi-empty-structs.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -0,0 +1,173 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --full-function-signature --filter "^define |^entry:"
+// RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-feature +d -target-abi ilp32d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-abi lp64f -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK64-C %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-abi lp64d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-C,CHECK64-C %s
+// RUN: %clang_cc1 -x c++ -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK32-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv32 -target-feature +f -target-feature +d -target-abi ilp32d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK32-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv64 -target-feature +f -target-abi lp64f -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK64-CXX %s
+// RUN: %clang_cc1 -x c++ -triple riscv64 -target-feature +f -target-feature +d -target-abi lp64d -emit-llvm %s -o - \
+// RUN:   | FileCheck -check-prefixes=CHECK-CXX,CHECK64-CXX %s
+
+#include 
+
+// Fields containing empty structs or unions are ignored when flattening
+// structs for the hard FP ABIs, even in C++.
+// FIXME: This isn't currently respected.
+
+struct empty { struct { struct { } e; }; };
+struct s1 { struct empty e; float f; };
+
+// CHECK-C-LABEL: define dso_local float @test_s1
+// CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
+// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
+// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK64-CXX:  entry:
+//
+struct s1 test_s1(struct s1 a) {
+  return a;
+}
+
+struct s2 { struct empty e; int32_t i; float f; };
+
+// CHECK-C-LABEL: define dso_local { i32, float } @test_s2
+// CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX:  entry:
+//
+struct s2 test_s2(struct s2 a) {
+  return a;
+}
+
+struct s3 { struct empty e; float f; float g; };
+
+// CHECK-C-LABEL: define dso_local { float, float } @test_s3
+// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX:  entry:
+//
+struct s3 test_s3(struct s3 a) {
+  return a;
+}
+
+struct s4 { struct empty e; float __complex__ c; };
+
+// CHECK-C-LABEL: define dso_local { float, float } @test_s4
+// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
+// CHECK-C:  entry:
+//
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX:  entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX:  entry:
+//
+struct s4 test_s4(struct s4 a) {
+  return a;
+}
+
+// An array of empty fields isn't ignored in C++ (this isn't explicit in the
+// psABI, but matches observed g+

[clang] 938deaa - [clang][RISCV] Fix ABI lowering for _Float16 for FP ABIs

[Alex Bradbury via cfe-commits]

Author: Alex Bradbury
Date: 2023-03-26T16:18:47+01:00
New Revision: 938deaad8ac92d9a45db8ef2d8a71fbf04958d90

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

LOG: [clang][RISCV] Fix ABI lowering for _Float16 for FP ABIs

For trivial cases (`_Float16` as a standalone argument), it was
previously correctly lowered to half. But the logic for catching cases
involving structs was gated off, as at the time that logic was written
the ABI for half was unclear.

This patch fixes that and adds a release note.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/RISCV/riscv32-abi.c
clang/test/CodeGen/RISCV/riscv64-abi.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1f18de92b920c..5ea223eca9564 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -306,6 +306,9 @@ RISC-V Support
   length. Valid values are powers of 2 between 64 and 65536. A value of 32
   should eventually be supported. We also accept "zvl" to use the Zvl*b
   extension from ``-march`` or ``-mcpu`` to the be the upper and lower bound.
+- Fixed incorrect ABI lowering of ``_Float16`` in the case of structs
+  containing ``_Float16`` that are eligible for passing via GPR+FPR or
+  FPR+FPR.
 
 CUDA/HIP Language Changes
 ^

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index a95401bf6bf80..78b1b402eef45 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -11157,10 +11157,9 @@ bool 
RISCVABIInfo::detectFPCCEligibleStructHelper(QualType Ty, CharUnits CurOff,
 uint64_t Size = getContext().getTypeSize(Ty);
 if (IsInt && Size > XLen)
   return false;
-// Can't be eligible if larger than the FP registers. Half precision isn't
-// currently supported on RISC-V and the ABI hasn't been confirmed, so
-// default to the integer ABI in that case.
-if (IsFloat && (Size > FLen || Size < 32))
+// Can't be eligible if larger than the FP registers. Handling of half
+// precision values has been specified in the ABI, so don't block those.
+if (IsFloat && Size > FLen)
   return false;
 // Can't be eligible if an integer type was already found (int+int pairs
 // are not eligible).

diff  --git a/clang/test/CodeGen/RISCV/riscv32-abi.c 
b/clang/test/CodeGen/RISCV/riscv32-abi.c
index 5cabbf4d90a1c..040ae500fc60e 100644
--- a/clang/test/CodeGen/RISCV/riscv32-abi.c
+++ b/clang/test/CodeGen/RISCV/riscv32-abi.c
@@ -1533,24 +1533,29 @@ union float_u f_ret_float_u(void) {
 // separate arguments in IR. They are passed by the same rules for returns,
 // but will be lowered to simple two-element structs if necessary (as LLVM IR
 // functions cannot return multiple values).
-// FIXME: Essentially all test cases below involving _Float16 in structs
-// aren't lowered according to the rules in the FP calling convention (i.e.
-// are incorrect for ilp32f/ilp32d).
 
 struct float16_s { _Float16 f; };
 
 // A struct containing just one floating-point real is passed as though it
 // were a standalone floating-point real.
 
-// ILP32-ILP32F-ILP32D-LABEL: define dso_local void @f_float16_s_arg
-// ILP32-ILP32F-ILP32D-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// ILP32-ILP32F-ILP32D:  entry:
+// ILP32-LABEL: define dso_local void @f_float16_s_arg
+// ILP32-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// ILP32:  entry:
+//
+// ILP32F-ILP32D-LABEL: define dso_local void @f_float16_s_arg
+// ILP32F-ILP32D-SAME: (half [[TMP0:%.*]]) #[[ATTR0]] {
+// ILP32F-ILP32D:  entry:
 //
 void f_float16_s_arg(struct float16_s a) {}
 
-// ILP32-ILP32F-ILP32D-LABEL: define dso_local i32 @f_ret_float16_s
-// ILP32-ILP32F-ILP32D-SAME: () #[[ATTR0]] {
-// ILP32-ILP32F-ILP32D:  entry:
+// ILP32-LABEL: define dso_local i32 @f_ret_float16_s
+// ILP32-SAME: () #[[ATTR0]] {
+// ILP32:  entry:
+//
+// ILP32F-ILP32D-LABEL: define dso_local half @f_ret_float16_s
+// ILP32F-ILP32D-SAME: () #[[ATTR0]] {
+// ILP32F-ILP32D:  entry:
 //
 struct float16_s f_ret_float16_s(void) {
   return (struct float16_s){1.0};
@@ -1562,29 +1567,45 @@ struct float16_s f_ret_float16_s(void) {
 struct zbf_float16_s { int : 0; _Float16 f; };
 struct zbf_float16_zbf_s { int : 0; _Float16 f; int : 0; };
 
-// ILP32-ILP32F-ILP32D-LABEL: define dso_local void @f_zbf_float16_s_arg
-// ILP32-ILP32F-ILP32D-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// ILP32-ILP32F-ILP32D:  entry:
+// ILP32-LABEL: define dso_local void @f_zbf_float16_s_arg
+// ILP32-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// ILP32:  entry:
+//
+// ILP32F-ILP32D-LABEL: define dso_local void @f_zbf_float16_s_arg
+// ILP32F-ILP32D-S

[PATCH] D145074: [clang][RISCV] Fix ABI lowering for _Float16 for FP ABIs

[Alex Bradbury via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG938deaad8ac9: [clang][RISCV] Fix ABI lowering for _Float16 
for FP ABIs (authored by asb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145074

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/RISCV/riscv32-abi.c
  clang/test/CodeGen/RISCV/riscv64-abi.c

Index: clang/test/CodeGen/RISCV/riscv64-abi.c
===
--- clang/test/CodeGen/RISCV/riscv64-abi.c
+++ clang/test/CodeGen/RISCV/riscv64-abi.c
@@ -1492,24 +1492,29 @@
 // separate arguments in IR. They are passed by the same rules for returns,
 // but will be lowered to simple two-element structs if necessary (as LLVM IR
 // functions cannot return multiple values).
-// FIXME: Essentially all test cases below involving _Float16 in structs
-// aren't lowered according to the rules in the FP calling convention (i.e.
-// are incorrect for lp64f/lp64d).
 
 struct float16_s { _Float16 f; };
 
 // A struct containing just one floating-point real is passed as though it
 // were a standalone floating-point real.
 
-// LP64-LP64F-LP64D-LABEL: define dso_local void @f_float16_s_arg
-// LP64-LP64F-LP64D-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local void @f_float16_s_arg
+// LP64-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local void @f_float16_s_arg
+// LP64F-LP64D-SAME: (half [[TMP0:%.*]]) #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 void f_float16_s_arg(struct float16_s a) {}
 
-// LP64-LP64F-LP64D-LABEL: define dso_local i64 @f_ret_float16_s
-// LP64-LP64F-LP64D-SAME: () #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local i64 @f_ret_float16_s
+// LP64-SAME: () #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local half @f_ret_float16_s
+// LP64F-LP64D-SAME: () #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 struct float16_s f_ret_float16_s(void) {
   return (struct float16_s){1.0};
@@ -1521,29 +1526,45 @@
 struct zbf_float16_s { int : 0; _Float16 f; };
 struct zbf_float16_zbf_s { int : 0; _Float16 f; int : 0; };
 
-// LP64-LP64F-LP64D-LABEL: define dso_local void @f_zbf_float16_s_arg
-// LP64-LP64F-LP64D-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local void @f_zbf_float16_s_arg
+// LP64-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local void @f_zbf_float16_s_arg
+// LP64F-LP64D-SAME: (half [[TMP0:%.*]]) #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 void f_zbf_float16_s_arg(struct zbf_float16_s a) {}
 
-// LP64-LP64F-LP64D-LABEL: define dso_local i64 @f_ret_zbf_float16_s
-// LP64-LP64F-LP64D-SAME: () #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local i64 @f_ret_zbf_float16_s
+// LP64-SAME: () #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local half @f_ret_zbf_float16_s
+// LP64F-LP64D-SAME: () #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 struct zbf_float16_s f_ret_zbf_float16_s(void) {
   return (struct zbf_float16_s){1.0};
 }
 
-// LP64-LP64F-LP64D-LABEL: define dso_local void @f_zbf_float16_zbf_s_arg
-// LP64-LP64F-LP64D-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local void @f_zbf_float16_zbf_s_arg
+// LP64-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local void @f_zbf_float16_zbf_s_arg
+// LP64F-LP64D-SAME: (half [[TMP0:%.*]]) #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 void f_zbf_float16_zbf_s_arg(struct zbf_float16_zbf_s a) {}
 
-// LP64-LP64F-LP64D-LABEL: define dso_local i64 @f_ret_zbf_float16_zbf_s
-// LP64-LP64F-LP64D-SAME: () #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LABEL: define dso_local i64 @f_ret_zbf_float16_zbf_s
+// LP64-SAME: () #[[ATTR0]] {
+// LP64:  entry:
+//
+// LP64F-LP64D-LABEL: define dso_local half @f_ret_zbf_float16_zbf_s
+// LP64F-LP64D-SAME: () #[[ATTR0]] {
+// LP64F-LP64D:  entry:
 //
 struct zbf_float16_zbf_s f_ret_zbf_float16_zbf_s(void) {
   return (struct zbf_float16_zbf_s){1.0};
@@ -1554,15 +1575,23 @@
 
 struct double_float16_s { double f; _Float16 g; };
 
-// LP64-LP64F-LP64D-LABEL: define dso_local void @f_double_float16_s_arg
-// LP64-LP64F-LP64D-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
-// LP64-LP64F-LP64D:  entry:
+// LP64-LP64F-LABEL: define dso_local void @f_double_float16_s_arg
+// LP64-LP64F-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// LP64-LP64F:  entry:
+//
+// LP64D-LABEL: define dso_local void @f_double_float16_s_arg
+// LP64D-SAME: (double [[TMP0:%.*]], half [[TMP1:%.*]]) #[[ATTR0]] {
+// LP64D:  entry:
 //
 void f_double_float16_s

[PATCH] D146895: [clang-format] Don't annotate left brace of struct as FunctionLBrace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay 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/D146895/new/

https://reviews.llvm.org/D146895

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

[PATCH] D146888: [clang-tidy] Flag std::forward on non-forwarding references

[Chris Cotter via Phabricator via cfe-commits]

ccotter abandoned this revision.
ccotter added a comment.

Indeed - sorry i missed that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146888

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

[clang-tools-extra] 556600a - [clang-tidy] Add option to ignore user-defined literals in readability-magic-numbers

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2023-03-26T16:41:17Z
New Revision: 556600af6a8a7f241277f7a22c3e3746e7b09123

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

LOG: [clang-tidy] Add option to ignore user-defined literals in 
readability-magic-numbers

Some user-defined literals operate on implementation-defined types, like
"unsigned long long" and "long double", which are not well supported by
this check. Currently, the check gives warnings when using UDLs, without
giving possiblity to the user to whitelist common UDLs. A good compromise
until a proper fix is found (if any) is to allow the user to disable
warnings on UDLs.

Partially fixes #61656

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst

clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
index ca81b6e3c6e61..cffe858c39ad1 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -81,6 +81,8 @@ MagicNumbersCheck::MagicNumbersCheck(StringRef Name, 
ClangTidyContext *Context)
   IgnorePowersOf2IntegerValues(
   Options.get("IgnorePowersOf2IntegerValues", false)),
   IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)),
+  IgnoreUserDefinedLiterals(
+  Options.get("IgnoreUserDefinedLiterals", false)),
   RawIgnoredIntegerValues(
   Options.get("IgnoredIntegerValues", DefaultIgnoredIntegerValues)),
   RawIgnoredFloatingPointValues(Options.get(
@@ -130,6 +132,7 @@ void 
MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnorePowersOf2IntegerValues",
 IgnorePowersOf2IntegerValues);
   Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases);
+  Options.store(Opts, "IgnoreUserDefinedLiterals", IgnoreUserDefinedLiterals);
   Options.store(Opts, "IgnoredIntegerValues", RawIgnoredIntegerValues);
   Options.store(Opts, "IgnoredFloatingPointValues",
 RawIgnoredFloatingPointValues);
@@ -243,5 +246,14 @@ bool MagicNumbersCheck::isBitFieldWidth(
   });
 }
 
+bool MagicNumbersCheck::isUserDefinedLiteral(
+const clang::ast_matchers::MatchFinder::MatchResult &Result,
+const clang::Expr &Literal) const {
+  DynTypedNodeList Parents = Result.Context->getParents(Literal);
+  if (Parents.empty())
+return false;
+  return Parents[0].get() != nullptr;
+}
+
 } // namespace tidy::readability
 } // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h 
b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
index 39e9baea3adc8..79787845de619 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -49,6 +49,10 @@ class MagicNumbersCheck : public ClangTidyCheck {
   bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult 
&Result,
const IntegerLiteral &Literal) const;
 
+  bool isUserDefinedLiteral(
+  const clang::ast_matchers::MatchFinder::MatchResult &Result,
+  const clang::Expr &Literal) const;
+
   template 
   void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result,
const char *BoundName) {
@@ -72,6 +76,10 @@ class MagicNumbersCheck : public ClangTidyCheck {
 if (isBitFieldWidth(Result, *MatchedLiteral))
   return;
 
+if (IgnoreUserDefinedLiterals &&
+isUserDefinedLiteral(Result, *MatchedLiteral))
+  return;
+
 const StringRef LiteralSourceText = Lexer::getSourceText(
 CharSourceRange::getTokenRange(MatchedLiteral->getSourceRange()),
 *Result.SourceManager, getLangOpts());
@@ -85,6 +93,7 @@ class MagicNumbersCheck : public ClangTidyCheck {
   const bool IgnoreBitFieldsWidths;
   const bool IgnorePowersOf2IntegerValues;
   const bool IgnoreTypeAliases;
+  const bool IgnoreUserDefinedLiterals;
   const StringRef RawIgnoredIntegerValues;
   const StringRef RawIgnoredFloatingPointValues;
 

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 1a53aac8fecf4..03ba9e160b738 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -223,6 +223,9 @@ Changes in existing checks
   behavior of us

[PATCH] D146913: [clang-tidy] Add option to ignore user-defined literals in readability-magic-numbers

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG556600af6a8a: [clang-tidy] Add option to ignore user-defined 
literals in readability-magic… (authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146913

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-userliteral.cpp
@@ -1,10 +1,18 @@
-// RUN: %check_clang_tidy -std=c++14-or-later %s readability-magic-numbers %t --
+// RUN: %check_clang_tidy -check-suffixes=UDL-ALLOWED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: false}]}' \
+// RUN: --
+// RUN: %check_clang_tidy -check-suffixes=UDL-IGNORED -std=c++14-or-later %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreUserDefinedLiterals, value: true}]}' \
+// RUN: --
 
 namespace std {
   class string {};
   using size_t = decltype(sizeof(int));
   string operator ""s(const char *, std::size_t);
   int operator "" s(unsigned long long);
+  float operator "" s(long double);
 }
 
 void UserDefinedLiteral() {
@@ -12,5 +20,9 @@
   "Hello World"s;
   const int i = 3600s;
   int j = 3600s;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-2]]:11: warning: 3600s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  float k = 3600.0s;
+  // CHECK-MESSAGES-UDL-ALLOWED: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  // CHECK-MESSAGES-UDL-IGNORED-NOT: :[[@LINE-1]]:13: warning: 3600.0s is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 }
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -57,7 +57,7 @@
 
const size_t NUMBER_OF_ELEMENTS = 30;
using containerType = CustomType;
-   
+
struct OtherType {
   containerType container;
}
@@ -144,3 +144,8 @@
 
Boolean value indicating whether to accept magic numbers in ``typedef`` or
``using`` declarations. Default value is `false`.
+
+.. option:: IgnoreUserDefinedLiterals
+
+   Boolean value indicating whether to accept magic numbers in user-defined
+   literals. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -223,6 +223,9 @@
   behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix`
   option to `i` instead of using `EnumConstantHungarianPrefix`.
 
+- Added support to optionally ignore user-defined literals in
+  :doc:`readability-magic-numbers`.
+
 - Fixed a false positive in :doc:`readability-container-size-empty
   ` check when comparing
   ``std::array`` objects to default constructed ones. The behavior for this and
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -49,6 +49,10 @@
   bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult &Result,
const IntegerLiteral &Literal) const;
 
+  bool isUserDefinedLiteral(
+  const clang::ast_matchers::MatchFinder::MatchResult &Result,
+  const clang::Expr &Literal) const;
+
   template 
   void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result,
const char *BoundName) {
@@ -72,6 +76,10 @@
 if (isBitFieldWidth(Result, *MatchedLiteral))
   return;
 
+if (Ign

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:139-142
+  // Clang does not append all target features to the clang -cc1 invocation.
+  // Some AMDGPU features are passed implicitly by the Clang frontend.
+  // That's why we need to extract implicit AMDGPU target features and add
+  // them to the target features specified by the user

domada wrote:
> awarzynski wrote:
> > [nit] IMHO this is documenting an implementation detail that would be more 
> > relevant inside `getExplicitAndImplicitAMDGPUTargetFeatures`.
> > 
> > More importantly, you are suggesting that the driver is doing whatever it 
> > is doing "because that's what Clang does". Consistency with Clang is 
> > important (you could call it out in the commit message) :) However, it 
> > would be even nicer to understand the actual rationale behind these 
> > implicit features. Any idea? Perhaps there are some clues in git history?
> > 
> > Also, perhaps a TODO to share this code with Clang? (to make it even 
> > clearer that the frontend driver aims for full consistency with Clang here).
> I think that the main difference between Clang and Flang is the lack of 
> `TargetInfo` class.
> 
> [[ https://clang.llvm.org/doxygen/classclang_1_1TargetInfo.html | TargetInfo 
> classes ]] are Clang specific and they are responsible for parsing/adding 
> default target features. Every target performs initialization in different 
> way (compare for example [[ 
> https://clang.llvm.org/doxygen/Basic_2Targets_2AArch64_8cpp_source.html#l00960
>  | AArch64 ]] vs [[ 
> https://clang.llvm.org/doxygen/Basic_2Targets_2AMDGPU_8cpp_source.html#l00179 
> | AMDGPU ]] target initialization.
> 
> I don't want to make TargetInfo class Clang indendent (see discussion: 
> https://discourse.llvm.org/t/rfc-targetinfo-library/64342 ). I also don't 
> want to reimplement the whole TargetInfo class in Flang, because Flang 
> already uses LLVM TargetMachine class (see: 
> https://llvm.org/doxygen/classllvm_1_1TargetMachine.html and 
> https://github.com/llvm/llvm-project/blob/main/flang/lib/Frontend/FrontendActions.cpp#L614
>  )  which can play similar role as Clang TargetInfo IMO.
> 
> That's why I decided to implement 
> `getExplicitAndImplicitAMDGPUTargetFeatures` function which performs 
> initialization of default AMDGPU target features.
Thanks for this comprehensive overview! It would be helpful if this rationale 
was included in the summary (in the spirit of documenting things for our future 
selves).

So, there isn't anything special about AMDGPU here, is there? We will have to 
implement similar hooks for other targets at some point too, right? Or perhaps 
there's some reason to do this for AMDGPU sooner rather than later?

I'm not against this change, just want to better understand the wider context.


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

https://reviews.llvm.org/D145579

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

[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

Is it worth adding a cppcoreguidelines alias (ES.56)?




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst:98
+
+.. option:: DisableTypeMismatchSuggestion
+

Curious what others thing but I think the tool should by default not remove or 
replace `forward` with static_cast.  When an author has written `forward`, 
there is a good chance they intended to forward something (e.g., they forgot to 
use `&&` in the parameter declaration). My hypothesis is that it's more likely 
the author intended to forward something, rather than than they were trying to 
use it as a cast (but had not intention of forward - why else would they have 
typed it out?). In this case, I think it merits manual review by default (so 
the tool should always warn, but without the fixits by default).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144347

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

[PATCH] D146921: Reviewers:

[Chris Cotter via Phabricator via cfe-commits]

ccotter created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, kbarton, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146921

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// NOLINTBEGIN
+namespace std {
+
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference { using type = T; };
+
+template  using remove_reference_t = typename remove_reference::type;
+
+template  constexpr T &&forward(remove_reference_t &t) noexcept;
+template  constexpr T &&forward(remove_reference_t &&t) noexcept;
+template  constexpr remove_reference_t &&move(T &&x);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+  S();
+  S(const S&);
+  S(S&&) noexcept;
+  S& operator=(const S&);
+  S& operator=(S&&) noexcept;
+};
+
+template 
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template 
+void does_not_forward(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t;
+}
+
+template 
+void does_not_forward_invoked(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t();
+}
+
+template 
+void forwards_pairwise(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  auto first = std::forward(t.first);
+  auto second = std::forward(t.second);
+}
+
+template 
+void does_not_forward_pack(Ts&&... ts) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  consumes_all(ts...);
+}
+
+template 
+class AClass {
+  template 
+  void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+  }
+};
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template 
+void does_forward(T&& t) {
+  T other = std::forward(t);
+}
+
+template 
+void does_forward_pack(Ts&&... ts) {
+  consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+  S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+  S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+  S other = std::move(s);
+}
+
+template 
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+  T other = std::move(t);
+}
+
+template 
+class AClass {
+  void rvalue_ref(T&& t) {
+T other = std::move(t);
+  }
+};
+
+} // namespace negative_cases
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -186,6 +186,7 @@
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-avoid-reference-coroutine-parameters `_,
+   `cppcoreguidelines-forwarding-reference-param-not-forwarded `_, "Yes"
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
`cppcoreguidelines-macro-usage `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
===

[PATCH] D146922: [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

[André Schackier via Phabricator via cfe-commits]

AMS21 created this revision.
AMS21 added a reviewer: njames93.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a project: All.
AMS21 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Previously a struct like this:

template 
struct A { A(A&&) = default; };

Would trigger a false positive, since it didn't realize that a defaulted
move constructor is automatically considered being noexcept.
Now we only give a warning if the defaulted move constrcutor has a
noexcept expression which evaluated to false but is not simply
`noexcept(false)`.

This fixes llvm#56026


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146922

Files:
  clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
@@ -13,6 +13,44 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
 };
 
+template 
+struct C
+{
+  C(C &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  C& operator=(C &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct D
+{
+  static constexpr bool kFalse = false;
+  D(D &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  D& operator=(D &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template 
+struct E
+{
+  static constexpr bool kFalse = false;
+  E(E &&) noexcept(kFalse);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  E& operator=(E &&) noexcept(kFalse);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template 
+struct F
+{
+  static constexpr bool kFalse = false;
+  F(F &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  F& operator=(F &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
 class OK {};
 
 void f() {
@@ -52,3 +90,38 @@
   OK5(OK5 &&) noexcept(true) = default;
   OK5 &operator=(OK5 &&) noexcept(true) = default;
 };
+
+struct OK6 {
+  OK6(OK6 &&) = default;
+  OK6& operator=(OK6 &&) = default;
+};
+
+template 
+struct OK7 {
+  OK7(OK7 &&) = default;
+  OK7& operator=(OK7 &&) = default;
+};
+
+template 
+struct OK8 {
+  OK8(OK8 &&) noexcept = default;
+  OK8& operator=(OK8 &&) noexcept = default;
+};
+
+template 
+struct OK9 {
+  OK9(OK9 &&) noexcept(true) = default;
+  OK9& operator=(OK9 &&) noexcept(true) = default;
+};
+
+template 
+struct OK10 {
+  OK10(OK10 &&) noexcept(false) = default;
+  OK10& operator=(OK10 &&) noexcept(false) = default;
+};
+
+template 
+struct OK11 {
+  OK11(OK11 &&) = delete;
+  OK11& operator=(OK11 &&) = delete;
+};
Index: clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
@@ -31,9 +31,7 @@
 };
 
 C_3::C_3(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 C_3& C_3::operator=(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 
 template 
 struct C_4 {
@@ -41,7 +39,6 @@
 // CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
  ~C_4() {}
  C_4& operator=(C_4&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 };
 
 template 
@@ -50,7 +47,6 @@
 // CHECK-FIXES:){{.*}}noexcept{{.*}} {}
  ~C_5() {}
  auto operator=(C_5&& a)->C_5 = default;
-// CHECK-FIXES:){{.*}}noexcept{{.*}} = default;
 };
 
 template 
Index: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
===
--- clang-tools-extra/cl

[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D144347#4222534 , @ccotter wrote:

> Is it worth adding a cppcoreguidelines alias (ES.56)?

Yes, you can pull this change (arc patch D144347 
 --nobranch) and create change that would 
depend on this one, and would register alias, also there you could override 
DisableRemoveSuggestion.
You can also help reviewing this check, to push it forward, point missing 
tests, scenarios. We could merge tests...




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst:98
+
+.. option:: DisableTypeMismatchSuggestion
+

ccotter wrote:
> Curious what others thing but I think the tool should by default not remove 
> or replace `forward` with static_cast.  When an author has written `forward`, 
> there is a good chance they intended to forward something (e.g., they forgot 
> to use `&&` in the parameter declaration). My hypothesis is that it's more 
> likely the author intended to forward something, rather than than they were 
> trying to use it as a cast (but had not intention of forward - why else would 
> they have typed it out?). In this case, I think it merits manual review by 
> default (so the tool should always warn, but without the fixits by default).
No, in this case if they used forward, they used this with different type, not 
adding & is not sufficient to trigger this case.

In theory we could, create alias bugprone-std-forward that would redirect to 
this check, but would set DisableTypeMismatchSuggestion to false, and would set 
DisableRemoveSuggestion & DisableMoveSuggestion to true.

In such case we could have readability check that would just suggest remove of 
std::forward or usage of std::move, and bugprone check that would only warn of 
casts between different types.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144347

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

[PATCH] D146922: [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

[Piotr Zegar via Phabricator via cfe-commits]

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

Current solution is invalid, defaulted move constructors can throw by default.
There are 2 more issues for this case:
https://github.com/llvm/llvm-project/issues/38081
https://github.com/llvm/llvm-project/issues/41414




Comment at: 
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp:46
+// specifier
+if (Decl->isDefaulted() && ProtoType->getExceptionSpecType() == EST_None)
+  return;

that's not true...

```
#include 

struct Field
{
Field() = default;
Field(Field&&) {

}
};

struct Test
{
Test(Test&&) = default;
Field field;
};

static_assert(std::is_nothrow_move_constructible::value);
```

Is default, not noexcept...

Try something like this:
```
bool cannotThrow(const FunctionDecl *Func) {
  const auto *FunProto = Func->getType()->getAs();
  if (!FunProto)
return false;

  switch (FunProto->canThrow()) {
  case CT_Cannot:
return true;
  case CT_Dependent: {
const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
bool Result;
return NoexceptExpr && !NoexceptExpr->isValueDependent() &&
   NoexceptExpr->EvaluateAsBooleanCondition(
   Result, Func->getASTContext(), true) &&
   Result;
  }
  default:
return false;
  };
}

// Source: https://reviews.llvm.org/D145865
```

so you could do something like this:

```
if (cannotThrow(Decl))
return;
```

I don't see reason to limit this check only to default'ed. This could be 
improved, to simply validate also things like 
```noexcept(std::is_nothrow_move_constructible::value)```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146922

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

[PATCH] D146922: [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Update ReleaseNotes for this check, and validate documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146922

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

maybe some other name for check, like missing-std-forward.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:62
+  namedDecl(hasUnderlyingDecl(hasName("::std::forward")),
+  argumentCountIs(1),
+  hasArgument(0, 
declRefExpr(to(equalsBoundNode("param"))).bind("ref")))

move this up to be checkered first



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:67
+  Finder->addMatcher(
+  parmVarDecl(
+  parmVarDecl().bind("param"), isTemplateTypeOfFunction(),

anyway invert this logic.
would be nice to invert this to start matching with functionDecl, but we don't 
have forEachParameter



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:69-72
+  anyOf(hasAncestor(cxxConstructorDecl(
+ToParam, isDefinition(),
+optionally(hasDescendant(ForwardCallMatcher,
+hasAncestor(functionDecl(

use isDefinition also for functionDecl,
anyway maybe
```
hasAncestor(functionnDecl(isDefinition(),
ToParam,

unless(hasDescendant(ForwardCallMatcher)))
```
I don't see reason for eplicit handling of cxxConstructorDecl, hasDescendant 
will match also initialization list.
Exclude code that is used in decltype, like:

```
template
void test(T&& t) {
   using Type = decltype(callSomething(std::forward(t)));
   callOther(t);
}
```



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84
+
+  if (!Param)
+return;
+

I thing this can never happen



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+  }
+};
+

add here that thing about UnlessSpelledInSource


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a updated this revision to Diff 508436.
bolshakov-a added a comment.

Add test cases on unspecified inheritance.


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

https://reviews.llvm.org/D146386

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -272,7 +272,7 @@
 };
 extern const record inst;
 void recref(type1) {}
-// CHECK: "?recref@@YAXU?$type1@$E?inst@@3Urecord@@B@@@Z"
+// CHECK: "?recref@@YAXU?$type1@$1?inst@@3Urecord@@B@@@Z"
 
 struct _GUID {};
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
@@ -286,7 +286,7 @@
 void fun(UUIDType1 a) {}
 // CHECK: "?fun@@YAXU?$UUIDType1@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 void fun(UUIDType2 b) {}
-// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$E?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
+// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 
 template  struct TypeWithFriendDefinition {
   friend void FunctionDefinedWithInjectedName(TypeWithFriendDefinition) {}
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -15,7 +15,7 @@
 
 int n = 0;
 // CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
-// MSABI: define {{.*}} @"??$f@$2UB@@PEBH1?n@@3HAH0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UB@@PEBHE?n@@3HAH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BLPKi0ELi1vv(
 // MSABI: define {{.*}} @"??$f@$2UB@@PEBH0A@H00@@@YAXXZ"
@@ -86,13 +86,13 @@
 struct D { const int Derived::*p; int k; };
 template void f() {}
 // CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H00@@@YAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH00@@@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DEEEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DadL_ZN7Derived1zEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0BA@H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H82@z@@H0AYAXXZ"
 template void f();
 #ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN1A1aEEvv
@@ -112,7 +112,62 @@
 template void f();
 #endif
 
-// FIXME: Pointers to member functions.
+struct DerivedVirtually : virtual A, Nested { int z; };
+struct D2 { const int DerivedVirtually::*p; int k; };
+template void f() {}
+// CHECK: define weak_odr void @_Z1fIXtl2D2LM16DerivedVirtuallyKi0ELi1vv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFA@?0H00@@@YAXXZ"
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2D2EEEvv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFA@?0H0AYAXXZ"
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2D2adL_ZN16DerivedVirtually1zEvv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFBA@A@H0AYAXXZ"
+template void f();
+
+// Forward-decl without MS inheritance keyword means unspecified inheritance
+// which is different from e. g. single inheritance.
+struct UnspecInherStruct;
+struct D3 { const int UnspecInherStruct::*p; };
+template void f() {}
+struct UnspecInherStruct { int i; };
+// CHECK: define weak_odr void @_Z1fIXtl2D3adL_ZN17UnspecInherStruct1iEvv
+// MSABI: define {{.*}} @"??$f@$2UD3@@PERUnspecInherStruct@@HGA@A@AYAXXZ"
+template void f();
+
+// Pointers to member functions.
+// Test struct templates instead of function templates so as to cover
+// the separate code which handles nullptr in their pointer-to-member arguments.
+struct Derived2 : A, Nested { void f(); virtual void g(); };
+struct D4 { void (Derived2::*p)(); };
+template  struct S1 { static void fn() {} };
+// CHECK: define weak_odr void @_ZN2S1IXtl2D4adL_ZN8Derived21fEv2fnEv
+// MSABI: define {{.*}} @"?fn@?$S1@$2UD4@@P8Derived2@@EAAXXZE?f@2@QEAAXXZ@@@SAXXZ"
+template void S1::fn();
+// CHECK: define weak_odr void @_ZN2S1IXtl2D4adL_ZN8Derived21gEv2fnEv
+// MSABI: define {{.*}} @"?fn@?$S1@$2UD4@@P8Derived2@@EAAXXZE??_92@$BA@AA@@@SAXXZ"
+template void S1::fn();
+// CHECK: define weak_odr void @_ZN2S1IXtl2D4EEE2fnEv
+// MSABI: define {{.*}} @"?fn@?$S1@$2UD4@@P8Derived2@@EAAXXZHASAXXZ"
+template void S1::fn();
+
+struct NoInheritance { void f(); };
+struct D5 { void (NoInheritance::*p)(); };
+template  struct S2 { static void fn() {} };
+// CHECK: define weak_odr void @_ZN2S2IXtl2D5ad

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a updated this revision to Diff 508437.
bolshakov-a added a comment.

Mangle pointer-to-members with conversion.

Pointer-to-member-functions with conversions across hierarchy work fine, as I 
can see.

I don't still understand how to mangle nested unnamed tags in general.


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

https://reviews.llvm.org/D146386

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -272,7 +272,7 @@
 };
 extern const record inst;
 void recref(type1) {}
-// CHECK: "?recref@@YAXU?$type1@$E?inst@@3Urecord@@B@@@Z"
+// CHECK: "?recref@@YAXU?$type1@$1?inst@@3Urecord@@B@@@Z"
 
 struct _GUID {};
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
@@ -286,7 +286,7 @@
 void fun(UUIDType1 a) {}
 // CHECK: "?fun@@YAXU?$UUIDType1@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 void fun(UUIDType2 b) {}
-// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$E?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
+// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 
 template  struct TypeWithFriendDefinition {
   friend void FunctionDefinedWithInjectedName(TypeWithFriendDefinition) {}
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -15,7 +15,7 @@
 
 int n = 0;
 // CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
-// MSABI: define {{.*}} @"??$f@$2UB@@PEBH1?n@@3HAH0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UB@@PEBHE?n@@3HAH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BLPKi0ELi1vv(
 // MSABI: define {{.*}} @"??$f@$2UB@@PEBH0A@H00@@@YAXXZ"
@@ -86,33 +86,87 @@
 struct D { const int Derived::*p; int k; };
 template void f() {}
 // CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H00@@@YAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH00@@@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DEEEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DadL_ZN7Derived1zEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0BA@H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H82@z@@H0AYAXXZ"
 template void f();
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN1A1aEEvv
-// MSABI-FIXME: define {{.*}} @"??$f@$2UD@@PERDerived@@H0A@H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H8A@@a@@H0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN1A1bEEvv
-// MSABI-FIXME: define {{.*}} @"??$f@$2UD@@PERDerived@@H03H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H8A@@b@@H0AYAXXZ"
 template void f();
+#ifndef _WIN32
 // FIXME: Is the Ut_1 mangling here correct?
 // CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN6NestedUt_1kEE8ELi2vv
-// FIXME: This mangles the same as &A::a (bug in the MS ABI).
-// MSABI-FIXME: define {{.*}} @"??$f@$2UD@@PERDerived@@H0A@H01@@@YAXXZ"
+// MSABI-FIXME: define {{.*}} @"??$f@$2UD@@PERDerived@@H8@Nested@@k@@H01@@@YAXXZ"
 template void f();
+#endif
 struct MoreDerived : A, Derived { int z; };
 // CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN11MoreDerived1zEEn8vv
-// MSABI-FIXME: define {{.*}} @"??$f@$2UD@@PERDerived@@H0BI@H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H8MoreDerived@@z@@H0AYAXXZ"
 template void f();
-#endif
 
-// FIXME: Pointers to member functions.
+struct DerivedVirtually : virtual A, Nested { int z; };
+struct D2 { const int DerivedVirtually::*p; int k; };
+template void f() {}
+// CHECK: define weak_odr void @_Z1fIXtl2D2LM16DerivedVirtuallyKi0ELi1vv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFA@?0H00@@@YAXXZ"
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2D2EEEvv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFA@?0H0AYAXXZ"
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2D2adL_ZN16DerivedVirtually1zEvv
+// MSABI: define {{.*}} @"??$f@$2UD2@@PERDerivedVirtually@@HFBA@A@H0AYAXXZ"
+template void f();
+
+// Forward-decl without MS inheritance keyword means unspecified inheritance
+// which is different from e. g. single inheritance.
+struct UnspecInherStruct;
+struct D3 { const int UnspecInherStruct::*p; };
+template void f() {}
+struct UnspecInhe

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:720
+const CXXRecordDecl *RD, const ValueDecl *VD) {
+  MSInheritanceModel IM = RD->getMSInheritanceModel();
+  //  ::= 

rjmccall wrote:
> efriedma wrote:
> > It's not obvious to me why the inheritance model is relevant here.  If you 
> > want to check if the class has virtual bases, please do that explicitly.  
> > (Note that the inheritance model can be messed with using attributes; see 
> > https://learn.microsoft.com/en-us/cpp/cpp/inheritance-keywords.)
> It would at least be good to test what exactly this is based on.
> 
> If RD is the base class of the member pointer type, isn't RD followed by the 
> unqualified name of VD potentially ambiguous when there's any inheritance at 
> all?  You don't even need multiple inheritance, just the presence of multiple 
> classes in the hierarchy that declare a field with the same name.  I mean, if 
> that's what MSVC does then we need to match it, but we should make certain by 
> testing member pointers at different positions in the hierarchy of the 
> declared base.
> 
> (And this is assuming the MSVC model that restricts what subclass member 
> pointers are allowed in superclass member pointer types.)
@efriedma, MS inheritance model overrides real inheritance. I've added a couple 
of test cases to reflect this. Btw, thanks for the link! I really didn't 
understand what "unspecified inheritance" is and how it is even possible.

@rjmccall, thank you! I've updated the mangling to a more correct algorithm.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:1857
 T->castAs()->getMostRecentCXXRecordDecl();
 const ValueDecl *D = V.getMemberPointerDecl();
 if (T->isMemberDataPointerType())

efriedma wrote:
> It might be more clear here to use `APValue::isNullPointer()`, instead of 
> checking if `getMemberPointerDecl()` returns null.  At first glance, it's not 
> really obvious why `getMemberPointerDecl()` would return null.  And in 
> theory, it's possible to have a non-null APValue where 
> `getMemberPointerDecl()` returns null (although in this context, such cases 
> should get rejected earlier).
`APValue::isNullPointer()` works only with `LValue`s, not with `MemberPointer`s:
https://github.com/llvm/llvm-project/blob/llvmorg-17-init/clang/lib/AST/APValue.cpp#L1000


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

https://reviews.llvm.org/D146386

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

[PATCH] D146924: [clang] Add support for dollar sign in ud_suffix of numeric constants

[Anna Arad via Phabricator via cfe-commits]

annara created this revision.
Herald added a project: All.
annara 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/D146924

Files:
  clang/lib/Lex/Lexer.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp


Index: clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
===
--- clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
+++ clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
@@ -16,3 +16,6 @@
 
 template constexpr int operator "" _x3() { return sizeof...(Cs); }
 static_assert(123456789012345678901234567890123456789012345678901234567890_x3 
== 60, "");
+
+int operator "" _$ (unsigned long long);
+int i6 = 42_$;
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1971,6 +1971,10 @@
 return LexNumericConstant(Result, CurPtr);
   if (!isASCII(C) && tryConsumeIdentifierUTF8Char(CurPtr))
 return LexNumericConstant(Result, CurPtr);
+  if (C == '$' && LangOpts.DollarIdents) {
+CurPtr = ConsumeChar(CurPtr, Size, Result);
+return LexNumericConstant(Result, CurPtr);
+  }
 
   // Update the location of token as well as BufferPtr.
   const char *TokStart = BufferPtr;


Index: clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
===
--- clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
+++ clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
@@ -16,3 +16,6 @@
 
 template constexpr int operator "" _x3() { return sizeof...(Cs); }
 static_assert(123456789012345678901234567890123456789012345678901234567890_x3 == 60, "");
+
+int operator "" _$ (unsigned long long);
+int i6 = 42_$;
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1971,6 +1971,10 @@
 return LexNumericConstant(Result, CurPtr);
   if (!isASCII(C) && tryConsumeIdentifierUTF8Char(CurPtr))
 return LexNumericConstant(Result, CurPtr);
+  if (C == '$' && LangOpts.DollarIdents) {
+CurPtr = ConsumeChar(CurPtr, Size, Result);
+return LexNumericConstant(Result, CurPtr);
+  }
 
   // Update the location of token as well as BufferPtr.
   const char *TokStart = BufferPtr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146501: [clang-format] Don't format already formatted integer literals

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

This patch is effectively NFC.


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

https://reviews.llvm.org/D146501

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

[PATCH] D146501: [clang-format] Don't format already formatted integer literals

[Owen Pan 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 rG5b5c49ad4563: [clang-format] Don't format already 
formatted integer literals (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146501

Files:
  clang/lib/Format/IntegerLiteralSeparatorFixer.cpp


Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -130,13 +130,12 @@
   DigitsPerGroup = Hex;
 if (DigitsPerGroup > 0 && checkSeparator(Text, DigitsPerGroup))
   continue;
+const auto &Formatted = format(Text, DigitsPerGroup);
+assert(Formatted != Text);
 if (Start > 0)
   Location = Location.getLocWithOffset(Start);
-if (const auto &Formatted = format(Text, DigitsPerGroup);
-Formatted != Text) {
-  cantFail(Result.add(
-  tooling::Replacement(SourceMgr, Location, Length, Formatted)));
-}
+cantFail(Result.add(
+tooling::Replacement(SourceMgr, Location, Length, Formatted)));
   }
 
   return {Result, 0};
@@ -153,9 +152,9 @@
 return false;
   I = 0;
 } else {
-  ++I;
   if (I == DigitsPerGroup)
 return false;
+  ++I;
 }
   }
 


Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -130,13 +130,12 @@
   DigitsPerGroup = Hex;
 if (DigitsPerGroup > 0 && checkSeparator(Text, DigitsPerGroup))
   continue;
+const auto &Formatted = format(Text, DigitsPerGroup);
+assert(Formatted != Text);
 if (Start > 0)
   Location = Location.getLocWithOffset(Start);
-if (const auto &Formatted = format(Text, DigitsPerGroup);
-Formatted != Text) {
-  cantFail(Result.add(
-  tooling::Replacement(SourceMgr, Location, Length, Formatted)));
-}
+cantFail(Result.add(
+tooling::Replacement(SourceMgr, Location, Length, Formatted)));
   }
 
   return {Result, 0};
@@ -153,9 +152,9 @@
 return false;
   I = 0;
 } else {
-  ++I;
   if (I == DigitsPerGroup)
 return false;
+  ++I;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5b5c49a - [clang-format] Don't format already formatted integer literals

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2023-03-26T13:25:41-07:00
New Revision: 5b5c49ad4563f75debccbc6c3017d27a47ca217d

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

LOG: [clang-format] Don't format already formatted integer literals

Fixes a bug in IntegerLiteralSeparatorFixer::checkSeparator() so that
only unformatted integer literals will be formatted.

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

Added: 


Modified: 
clang/lib/Format/IntegerLiteralSeparatorFixer.cpp

Removed: 




diff  --git a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp 
b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
index e5bee2e855bb..0eac3498d721 100644
--- a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -130,13 +130,12 @@ IntegerLiteralSeparatorFixer::process(const Environment 
&Env,
   DigitsPerGroup = Hex;
 if (DigitsPerGroup > 0 && checkSeparator(Text, DigitsPerGroup))
   continue;
+const auto &Formatted = format(Text, DigitsPerGroup);
+assert(Formatted != Text);
 if (Start > 0)
   Location = Location.getLocWithOffset(Start);
-if (const auto &Formatted = format(Text, DigitsPerGroup);
-Formatted != Text) {
-  cantFail(Result.add(
-  tooling::Replacement(SourceMgr, Location, Length, Formatted)));
-}
+cantFail(Result.add(
+tooling::Replacement(SourceMgr, Location, Length, Formatted)));
   }
 
   return {Result, 0};
@@ -153,9 +152,9 @@ bool IntegerLiteralSeparatorFixer::checkSeparator(
 return false;
   I = 0;
 } else {
-  ++I;
   if (I == DigitsPerGroup)
 return false;
+  ++I;
 }
   }
 



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

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

This seem to case problems when building with asan enabled 
(LLVM_USE_SANITIZER='Address'):

  Failed Tests (24):
Clang :: CXX/basic/basic.link/p2.cpp
Clang :: CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
Clang :: CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
Clang :: CXX/module/basic/basic.def.odr/p4.cppm
Clang :: CXX/module/basic/basic.def.odr/p6.cppm
Clang :: CXX/module/basic/basic.link/module-declaration.cpp
Clang :: CXX/module/basic/basic.link/p2.cppm
Clang :: CXX/module/dcl.dcl/dcl.module/dcl.module.import/p1.cppm
Clang :: CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
Clang :: CXX/module/dcl.dcl/dcl.module/p1.cpp
Clang :: CXX/module/dcl.dcl/dcl.module/p5.cpp
Clang :: CXX/module/module.interface/p1.cpp
Clang :: CXX/module/module.interface/p2.cpp
Clang :: CXX/module/module.unit/p8.cpp
Clang :: CodeGenCXX/cxx20-module-impl-1a.cpp
Clang :: CodeGenCXX/module-intializer.cpp
Clang :: Modules/cxx20-10-1-ex1.cpp
Clang :: Modules/cxx20-10-1-ex2.cpp
Clang :: Modules/cxx20-10-2-ex5.cpp
Clang :: Modules/cxx20-10-3-ex2.cpp
Clang :: Modules/cxx20-impl-module-conditionally-load.cppm
Clang :: Modules/cxx20-import-diagnostics-a.cpp
Clang :: Modules/cxx20-partition-redeclarations.cpp
Clang :: Modules/pr58532.cppm

Here is a typical log from Modules/cxx20-partition-redeclarations.cpp:

  ==80746==ERROR: LeakSanitizer: detected memory leaks
  
  Direct leak of 2080 byte(s) in 1 object(s) allocated from:
  #0 0x61ea6d in operator new(unsigned long) 
/repo/old//compiler-rt/lib/asan/asan_new_delete.cpp:99:3
  #1 0x11a9ae7a in 
clang::ModuleMap::createModuleUnitWithKind(clang::SourceLocation, 
llvm::StringRef, clang::Module::ModuleKind) 
/repo/clang/lib/Lex/ModuleMap.cpp:894:7
  #2 0x11a9bc57 in 
clang::ModuleMap::createModuleForImplementationUnit(clang::SourceLocation, 
llvm::StringRef) /repo/clang/lib/Lex/ModuleMap.cpp:933:7
  #3 0xe84a76d in clang::Sema::ActOnModuleDecl(clang::SourceLocation, 
clang::SourceLocation, clang::Sema::ModuleDeclKind, 
llvm::ArrayRef>, 
llvm::ArrayRef>, 
clang::Sema::ModuleImportState&) /repo/clang/lib/Sema/SemaModule.cpp:350:17
  #4 0xd347fc5 in 
clang::Parser::ParseModuleDecl(clang::Sema::ModuleImportState&) 
/repo/clang/lib/Parse/Parser.cpp:2478:18
  #5 0xd3451b4 in 
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) /repo/clang/lib/Parse/Parser.cpp:658:14
  #6 0xd343014 in 
clang::Parser::ParseFirstTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) /repo/clang/lib/Parse/Parser.cpp:593:26
  #7 0xd32c624 in clang::ParseAST(clang::Sema&, bool, bool) 
/repo/clang/lib/Parse/ParseAST.cpp:161:25
  #8 0x9a0a494 in clang::FrontendAction::Execute() 
/repo/clang/lib/Frontend/FrontendAction.cpp:1058:8
  #9 0x98025a0 in 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) 
/repo/clang/lib/Frontend/CompilerInstance.cpp:1048:33
  #10 0x9ca0f3d in 
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) 
/repo/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:264:25
  #11 0x63ad58 in cc1_main(llvm::ArrayRef, char const*, void*) 
/repo/clang/tools/driver/cc1_main.cpp:251:15
  #12 0x62d5cc in ExecuteCC1Tool(llvm::SmallVectorImpl&, 
llvm::ToolContext const&) /repo/clang/tools/driver/driver.cpp:366:12
  #13 0x628173 in clang_main(int, char**, llvm::ToolContext const&) 
/repo/clang/tools/driver/driver.cpp:407:12
  #14 0x65de41 in main /repo/tools/clang/tools/driver/clang-driver.cpp:15:10
  #15 0x7fd7c0406554 in __libc_start_main (/lib64/libc.so.6+0x22554)
  
  SUMMARY: AddressSanitizer: 2080 byte(s) leaked in 1 allocation(s).

It is for example seen in this buildbot 
(sanitizer-x86_64-linux-bootstrap-asan): 
https://lab.llvm.org/buildbot/#/builders/168


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

[Michael Buch via Phabricator via cfe-commits]

Michael137 added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

probinson wrote:
> This doesn't become `Foo` ?
The name stays as `Foo>` but the type of the template parameter 
becomes `BarInt`. So the dwarf would look like:
```
DW_TAG_structure_type
  DW_AT_name  ("Foo >")

  DW_TAG_template_type_parameter
DW_AT_type(0x0095 "BarInt")
DW_AT_name("T")

```
Will add the parameter metadata to the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D146874: [clangd] Fix a hover crash on unsigned 64bit value

[Nathan Ridge via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146874

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508441.
ccotter marked 4 inline comments as done.
ccotter retitled this revision from "[clang-tidy] Implement cppcoreguidelines 
F.19  " to "[clang-tidy] Implement cppcoreguidelines F.19".
ccotter added a comment.

- add tests, simplify expr, handle unevaluated exprs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// NOLINTBEGIN
+namespace std {
+
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference { using type = T; };
+
+template  using remove_reference_t = typename remove_reference::type;
+
+template  constexpr T &&forward(remove_reference_t &t) noexcept;
+template  constexpr T &&forward(remove_reference_t &&t) noexcept;
+template  constexpr remove_reference_t &&move(T &&x);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+  S();
+  S(const S&);
+  S(S&&) noexcept;
+  S& operator=(const S&);
+  S& operator=(S&&) noexcept;
+};
+
+template 
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template 
+void does_not_forward(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t;
+}
+
+template 
+void does_not_forward_invoked(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t();
+}
+
+template 
+void forwards_pairwise(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  auto first = std::forward(t.first);
+  auto second = std::forward(t.second);
+}
+
+template 
+void does_not_forward_pack(Ts&&... ts) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  consumes_all(ts...);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(u) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  AClass& operator=(U&& u) { }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+  }
+
+  T data;
+};
+
+template 
+void does_not_forward_in_evaluated_code(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  using result_t = decltype(std::forward(t));
+  unsigned len = sizeof(std::forward(t));
+  T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template 
+void just_a_decl(T&&t);
+
+template 
+void does_forward(T&& t) {
+  T other = std::forward(t);
+}
+
+template 
+void does_forward_pack(Ts&&... ts) {
+  consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+  S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+  S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+  S other = std::move(s);
+}
+
+template 
+void templated_rvalue_ref(std::remove_refer

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84
+
+  if (!Param)
+return;
+

PiotrZSL wrote:
> I thing this can never happen
Another review suggested I check the matched node for nullness, even though it 
was the only possible match (https://reviews.llvm.org/D140793). I'll keep it as 
is since in the spirit of defensive programming and to assist future developers 
who may add more matches that might invalidate the invariant that `Param` is 
always non-null.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+  }
+};
+

PiotrZSL wrote:
> add here that thing about UnlessSpelledInSource
Sorry, I think I'm missing some context here. Would you mind clarifying your 
comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I don't still understand how to mangle nested unnamed tags in general.

According to some quick experiments, for the non-virtual case, you mangle a 
member of an unnamed union it the same way as a regular member, except you 
stick `@` into the mangling.  Additional levels of nesting append 
3's: `@3`, `@33`, etc.



The mangling with virtual inheritance seems pretty broken.  The following 
produces a name collision on MSVC:

  #pragma pointers_to_members(full_generality, virtual_inheritance)
  struct Nested { int a; union { int k; int k2;}; };
  struct DerivedVirtually : Nested { int a; };
  struct D2 { int DerivedVirtually::*p; };
  template void f() {}
  template void f();
  template void f();

The following crashes MSVC:

  struct A {};
  struct Nested { int a; };
  struct DerivedVirtually : virtual A, Nested { };
  struct D2 { int DerivedVirtually::*p; };
  template void f() {}
  template void f();




Comment at: clang/lib/AST/MicrosoftMangle.cpp:1247
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error, "cannot mangle anonymous struct/union 
yet");
+  Diags.Report(DiagID);

maybe say "anonymous struct/union member pointer", if you don't implement this.


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

https://reviews.llvm.org/D146386

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

[PATCH] D146924: [clang] Add support for dollar sign in ud_suffix of numeric constants

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

@cor3ntin, do you have any thoughts about this? CWG 1871 (Non-identifier 
characters in ud-suffix)  is somewhat related. I 
think the changed behavior makes sense when `-fdollars-in-identifiers` is in 
effect (which is the default).




Comment at: clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp:20-21
+
+int operator "" _$ (unsigned long long);
+int i6 = 42_$;

If we're going to allow `$` in user-defined literal (UDL) suffixes, then I 
think we should do so for all UDL kinds (and have tests for each kind).

We should also test for `$` specified as a UCN (`\u0024`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146924

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

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

[Adheesh Wadkar via Phabricator via cfe-commits]

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

Added the DecorateReflowComments option to control whether '* ' are added
to the beginnings of continuation lines for block comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146926

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -901,6 +901,7 @@
Style.ConstructorInitializerIndentWidth);
 IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
 IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
+IO.mapOptional("DecorateReflowedComments", Style.DecorateReflowedComments);
 IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
 IO.mapOptional("DisableFormat", Style.DisableFormat);
 IO.mapOptional("EmptyLineAfterAccessModifier",
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -403,7 +403,7 @@
   }
 
   Decoration = "* ";
-  if (Lines.size() == 1 && !FirstInLine) {
+  if ((Lines.size() == 1 && !FirstInLine) || !Style.DecorateReflowedComments) {
 // Comments for which FirstInLine is false can start on arbitrary column,
 // and available horizontal space can be too small to align consecutive
 // lines with the first one.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2007,6 +2007,20 @@
   /// \version 3.4
   bool Cpp11BracedListStyle;
 
+  /// If ``true``, when a block comment is reflowed, add a ``* `` to the
+  /// beginning of the continuation line.
+  /// \code
+  /// false:
+  /// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+  ///blah blah blah blah blah blah blah blah */
+  ///
+  /// true:
+  /// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+  ///  * blah blah blah blah blah blah blah blah */
+  /// \endcode
+  /// \version 17
+  bool DecorateReflowedComments;
+
   /// This option is **deprecated**. See ``DeriveLF`` and ``DeriveCRLF`` of
   /// ``LineEnding``.
   /// \version 10


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -901,6 +901,7 @@
Style.ConstructorInitializerIndentWidth);
 IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
 IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
+IO.mapOptional("DecorateReflowedComments", Style.DecorateReflowedComments);
 IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
 IO.mapOptional("DisableFormat", Style.DisableFormat);
 IO.mapOptional("EmptyLineAfterAccessModifier",
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -403,7 +403,7 @@
   }
 
   Decoration = "* ";
-  if (Lines.size() == 1 && !FirstInLine) {
+  if ((Lines.size() == 1 && !FirstInLine) || !Style.DecorateReflowedComments) {
 // Comments for which FirstInLine is false can start on arbitrary column,
 // and available horizontal space can be too small to align consecutive
 // lines with the first one.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2007,6 +2007,20 @@
   /// \version 3.4
   bool Cpp11BracedListStyle;
 
+  /// If ``true``, when a block comment is reflowed, add a ``* `` to the
+  /// beginning of the continuation line.
+  /// \code
+  /// false:
+  /// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+  ///blah blah blah blah blah blah blah blah */
+  ///
+  /// true:
+  /// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+  ///  * blah blah blah blah blah blah blah blah */
+  /// \endcode
+  /// \version 17
+  bool DecorateReflowedComments;
+
   /// This option is **deprecated**. See ``DeriveLF`` and ``DeriveCRLF`` of
   /// ``LineEnding``.
   /// \version 10
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

You need to regenerate the documentation after changing Format.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146926

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

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

You need to add unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146926

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

[PATCH] D146922: [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp:64
 // CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
\ No newline at end of file


Please add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146922

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:127
+
+  Warns when a forwarding reference parameter is not forwarded within the 
function body.
+

Please follow 80 characters limit for text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

[Adheesh Wadkar via Phabricator via cfe-commits]

apwadkar added a comment.

Sorry about the premature review, this is my first commit, and I didn't realize 
it wouldn't create a draft first.

- For rebuilding the docs, I assume I need to add `-DLLVM_BUILD_DOCS=true` to 
CMake and then run `ninja -C build`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146926

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

[clang] 6cef325 - [clang-format] Don't squash Verilog escaped identifiers

[via cfe-commits]

Author: sstwcw
Date: 2023-03-26T22:45:44Z
New Revision: 6cef325481a8efc039ae9df5630609fd3a84560c

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

LOG: [clang-format] Don't squash Verilog escaped identifiers

An escaped identifier always needs a space following it so the parser
can tell it apart from the next token.

The unit tests are changed to use `FormatTestBase.h` because we need the
2-argument version of `verifyFormat`.  We also added the `messUp`
virtual function because Verilog needs a different version of it.

Reviewed By: HazardyKnusperkeks

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestBase.h
clang/unittests/Format/FormatTestVerilog.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 91f9d5719b5a..5da5dc96032a 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4348,6 +4348,11 @@ bool TokenAnnotator::spaceRequiredBefore(const 
AnnotatedLine &Line,
   return true;
 }
   } else if (Style.isVerilog()) {
+// An escaped identifier ends with whitespace.
+if (Style.isVerilog() && Left.is(tok::identifier) &&
+Left.TokenText[0] == '\\') {
+  return true;
+}
 // Add space between things in a primitive's state table unless in a
 // transition like `(0?)`.
 if ((Left.is(TT_VerilogTableItem) &&

diff  --git a/clang/unittests/Format/FormatTestBase.h 
b/clang/unittests/Format/FormatTestBase.h
index ea214468965a..7d34bcc2d835 100644
--- a/clang/unittests/Format/FormatTestBase.h
+++ b/clang/unittests/Format/FormatTestBase.h
@@ -31,6 +31,10 @@ class FormatTestBase : public ::testing::Test {
 
   virtual FormatStyle getDefaultStyle() const { return getLLVMStyle(); }
 
+  virtual std::string messUp(llvm::StringRef Code) const {
+return test::messUp(Code);
+  }
+
   std::string format(llvm::StringRef Code,
  const std::optional &Style = {},
  StatusCheck CheckComplete = SC_ExpectComplete,
@@ -105,8 +109,7 @@ class FormatTestBase : public ::testing::Test {
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef 
Code,
const std::optional &Style = {}) {
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Code.str(),
-  format(test::messUp(Code), Style, SC_ExpectIncomplete));
+EXPECT_EQ(Code.str(), format(messUp(Code), Style, SC_ExpectIncomplete));
   }
 
   void
@@ -139,4 +142,4 @@ class FormatTestBase : public ::testing::Test {
 } // namespace format
 } // namespace clang
 
-#endif
\ No newline at end of file
+#endif

diff  --git a/clang/unittests/Format/FormatTestVerilog.cpp 
b/clang/unittests/Format/FormatTestVerilog.cpp
index 3a03525c2db7..88ce08207c95 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -6,47 +6,26 @@
 //
 
//===--===//
 
-#include "FormatTestUtils.h"
-#include "clang/Format/Format.h"
-#include "llvm/Support/Debug.h"
-#include "gtest/gtest.h"
+#include "FormatTestBase.h"
 
 #define DEBUG_TYPE "format-test"
 
 namespace clang {
 namespace format {
-
-class FormatTestVerilog : public ::testing::Test {
+namespace test {
+namespace {
+class FormatTestVerilog : public test::FormatTestBase {
 protected:
-  static std::string format(llvm::StringRef Code, unsigned Offset,
-unsigned Length, const FormatStyle &Style) {
-LLVM_DEBUG(llvm::errs() << "---\n");
-LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-std::vector Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-return format(Code, 0, Code.size(), Style);
+  FormatStyle getDefaultStyle() const override {
+return getLLVMStyle(FormatStyle::LK_Verilog);
   }
-
-  static void verifyFormat(
-  llvm::StringRef Code,
-  const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not 
stable";
-EXPECT_EQ(Code.str(),
-  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  std::string messUp(llvm::StringRef Code) const override {
+return test::messUp(Code, /*HandleHash=*/false);
  

[PATCH] D146401: [clang-format] Don't squash Verilog escaped identifiers

[sstwcw 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 rG6cef325481a8: [clang-format] Don't squash Verilog 
escaped identifiers (authored by sstwcw).

Changed prior to commit:
  https://reviews.llvm.org/D146401?vs=506454&id=508444#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146401

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestBase.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -6,47 +6,26 @@
 //
 //===--===//
 
-#include "FormatTestUtils.h"
-#include "clang/Format/Format.h"
-#include "llvm/Support/Debug.h"
-#include "gtest/gtest.h"
+#include "FormatTestBase.h"
 
 #define DEBUG_TYPE "format-test"
 
 namespace clang {
 namespace format {
-
-class FormatTestVerilog : public ::testing::Test {
+namespace test {
+namespace {
+class FormatTestVerilog : public test::FormatTestBase {
 protected:
-  static std::string format(llvm::StringRef Code, unsigned Offset,
-unsigned Length, const FormatStyle &Style) {
-LLVM_DEBUG(llvm::errs() << "---\n");
-LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-std::vector Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-return format(Code, 0, Code.size(), Style);
+  FormatStyle getDefaultStyle() const override {
+return getLLVMStyle(FormatStyle::LK_Verilog);
   }
-
-  static void verifyFormat(
-  llvm::StringRef Code,
-  const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
-EXPECT_EQ(Code.str(),
-  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  std::string messUp(llvm::StringRef Code) const override {
+return test::messUp(Code, /*HandleHash=*/false);
   }
 };
 
 TEST_F(FormatTestVerilog, Align) {
-  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  FormatStyle Style = getDefaultStyle();
   Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("x<= x;\n"
"sfdbddfbdfbb <= x;\n"
@@ -242,7 +221,7 @@
"instruction3(ir);\n"
"endcase");
   // Test indention options.
-  auto Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  auto Style = getDefaultStyle();
   Style.IndentCaseLabels = false;
   verifyFormat("case (data)\n"
"16'd0:\n"
@@ -268,7 +247,7 @@
"endcase",
Style);
   // Other colons should not be mistaken as case colons.
-  Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  Style = getDefaultStyle();
   Style.BitFieldColonSpacing = FormatStyle::BFCS_None;
   verifyFormat("case (x[1:0])\n"
"endcase",
@@ -283,7 +262,7 @@
   verifyFormat("default:\n"
"  x[1 : 0] = x[1 : 0];",
Style);
-  Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  Style = getDefaultStyle();
   Style.SpacesInContainerLiterals = true;
   verifyFormat("case ('{x : x, default : 9})\n"
"endcase",
@@ -355,8 +334,8 @@
   verifyFormat("#1.5s;");
   // The following expression should be on the same line.
   verifyFormat("#1 x = x;");
-  EXPECT_EQ("#1 x = x;", format("#1\n"
-"x = x;"));
+  verifyFormat("#1 x = x;", "#1\n"
+"x = x;");
 }
 
 TEST_F(FormatTestVerilog, Headers) {
@@ -486,7 +465,7 @@
" b);\n"
"endmodule");
   // With a concatenation in the names.
-  auto Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  auto Style = getDefaultStyle();
   Style.ColumnLimit = 40;
   verifyFormat("`define X(x)   \\\n"
"  module test  \\\n"
@@ -577,6 +556,28 @@
"endfunction : x");
 }
 
+TEST_F(FormatTestVerilog, Identifiers) {
+  // Escaped identifiers should not be split.
+  verifyFormat("\\busa+index");
+  verifyFormat("\\-clock");
+  verifyFormat("\\***error-condition***");
+  verifyFormat("\\net1\\/net2");
+  verifyFormat("\\{a,b}");
+  verifyFormat("\\a*(b+c)");
+  // Escaped identifiers can't be joined with the next token.  Extra space
+  // should be removed.
+  verifyFormat("\\busa+index ;", "

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508450.
ccotter added a comment.

- add tests, simplify expr, handle unevaluated exprs
- formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// NOLINTBEGIN
+namespace std {
+
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference { using type = T; };
+
+template  using remove_reference_t = typename remove_reference::type;
+
+template  constexpr T &&forward(remove_reference_t &t) noexcept;
+template  constexpr T &&forward(remove_reference_t &&t) noexcept;
+template  constexpr remove_reference_t &&move(T &&x);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+  S();
+  S(const S&);
+  S(S&&) noexcept;
+  S& operator=(const S&);
+  S& operator=(S&&) noexcept;
+};
+
+template 
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template 
+void does_not_forward(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t;
+}
+
+template 
+void does_not_forward_invoked(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t();
+}
+
+template 
+void forwards_pairwise(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  auto first = std::forward(t.first);
+  auto second = std::forward(t.second);
+}
+
+template 
+void does_not_forward_pack(Ts&&... ts) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  consumes_all(ts...);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(u) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  AClass& operator=(U&& u) { }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+  }
+
+  T data;
+};
+
+template 
+void does_not_forward_in_evaluated_code(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  using result_t = decltype(std::forward(t));
+  unsigned len = sizeof(std::forward(t));
+  T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template 
+void just_a_decl(T&&t);
+
+template 
+void does_forward(T&& t) {
+  T other = std::forward(t);
+}
+
+template 
+void does_forward_pack(Ts&&... ts) {
+  consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+  S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+  S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+  S other = std::move(s);
+}
+
+template 
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+  T other = std::move(t);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(std::forward(u)) {}
+
+  template 
+  AClass& operator=(

[PATCH] D145138: [clang-tidy] Implement FixIts for C arrays

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508451.
ccotter added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145138

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/TypeUtils.cpp
  clang-tools-extra/clang-tidy/utils/TypeUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-cxx17.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+// RUN: %check_clang_tidy -std=c++11 %s modernize-avoid-c-arrays %t
+
+//CHECK-FIXES: #include 
 
 int a[] = {1, 2};
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
@@ -86,3 +88,262 @@
   int j[1];
 };
 }
+
+template  struct TStruct {};
+
+void replacements() {
+  int ar[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array ar;
+  TStruct ar2[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array, 10> ar2;
+  TStruct< int > ar3[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array, 10> ar3;
+  int * ar4[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array ar4;
+  int * /*comment*/ar5[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array /*comment*/ar5;
+  volatile const int * ar6[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array ar6;
+  volatile int ar7[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: volatile std::array ar7;
+  int const * ar8[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array ar8;
+  int ar9[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array ar9;
+  static int volatile constexpr ar10[10] = {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: static volatile constexpr std::array ar10 = { {} };
+  thread_local int ar11[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: thread_local std::array ar11;
+  thread_local/*a*/int/*b*/ar12[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: thread_local/*a*/std::array/*b*/ar12;
+  /*a*/ int/*b*/ /*c*/*/*d*/ /*e*/ /*f*/ar13[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: /*a*/ std::array/*d*/ /*e*/ /*f*/ar13;
+  TStruct ar14[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array, 10> ar14;
+  volatile TStruct ar15[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: volatile std::array, 10> ar15;
+  TStruct ar16[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array, 10> ar16;
+  TStruct ar17[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: std::array, 10> ar17;
+  volatile int static thread_local * ar18[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-FIXES: static thread_local std::array ar18;
+
+  // Note, there is a tab '\t' before the semicolon in the declaration below.
+  int ar19[3]	;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare 

[PATCH] D141892: Implement modernize-use-constraints

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508452.
ccotter added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints-first-greatergreater.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp
@@ -0,0 +1,693 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-constraints %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template  struct enable_if { };
+
+template  struct enable_if { typedef T type; };
+
+template 
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template 
+struct ConsumeVariadic;
+
+struct Obj {
+};
+
+namespace enable_if_in_return_type {
+
+
+// Section 1: enable_if in return type of function
+
+
+
+// General tests
+
+
+template 
+typename std::enable_if::type basic() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t basic_t() {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}Obj basic_t() requires T::some_value {{{$}}
+
+template 
+auto basic_trailing() -> typename std::enable_if::type {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:26: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}auto basic_trailing() -> Obj requires T::some_value {{{$}}
+
+template 
+typename std::enable_if::type existing_constraint() requires (T::another_value) {
+  return Obj{};
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}typename std::enable_if::type existing_constraint() requires (T::another_value) {{{$}}
+
+template 
+typename std::enable_if::type decl_without_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def();
+
+template 
+typename std::enable_if::type decl_with_separate_def() {
+  return Obj{};
+}
+// FIXME - Support definitions with separate decls
+
+template 
+std::enable_if_t no_dependent_type(U) {
+  return Obj{};
+}
+// FIXME - Support non-dependent enable_ifs. Low priority though...
+
+template 
+typename std::enable_if::type* pointer_of_enable_if() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t* pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int* pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+const std::enable_if_t* const_pointer_of_enable_if_t() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}const int* const_pointer_of_enable_if_t() requires T::some_value {{{$}}
+
+template 
+std::enable_if_t const * const_pointer_of_enable_if_t2() {
+  return nullptr;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int const * const_pointer_of_enable_if_t2() requires T::some_value {{{$}}
+
+
+template 
+std::enable_if_t& reference_of_enable_if_t() {
+  static int x; return x;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints]
+// CHECK-FIXES: {{^}}template {{$}}
+// CHECK-FIXES-NEXT: {{^}}int& reference_of_enable_if_t() requires T::some

[PATCH] D146929: [clang-tidy] Ignore unevaluated exprs in rvalue-reference-param-not-moved

[Chris Cotter via Phabricator via cfe-commits]

ccotter created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Ignore unevaluated expressions in rvalue-reference-param-not-moved
check since they are not actual uses of a move().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146929

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -156,6 +156,13 @@
   Obj moved = std::move((o));
 }
 
+void does_not_move_in_evaluated(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' 
is never moved from inside the function body 
[cppcoreguidelines-rvalue-reference-param-not-moved]
+  using result_t = decltype(std::move(o));
+  unsigned size = sizeof(std::move(o));
+  Obj moved = o;
+}
+
 template 
 struct mypair {
   T1 first;
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "RvalueReferenceParamNotMovedCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -15,6 +16,23 @@
 namespace clang::tidy::cppcoreguidelines {
 
 namespace {
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+  if (isa(Node) || isa(Node))
+return true;
+  if (const auto *UnaryExpr = dyn_cast(&Node)) {
+switch (UnaryExpr->getKind()) {
+case UETT_SizeOf:
+case UETT_AlignOf:
+  return true;
+default:
+  return false;
+}
+  }
+  if (const auto *TypeIDExpr = dyn_cast(&Node))
+return !TypeIDExpr->isPotentiallyEvaluated();
+  return false;
+}
+
 AST_MATCHER_P(LambdaExpr, valueCapturesVar, DeclarationMatcher, VarMatcher) {
   return std::find_if(Node.capture_begin(), Node.capture_end(),
   [&](const LambdaCapture &Capture) {
@@ -39,16 +57,18 @@
 
   StatementMatcher MoveCallMatcher =
   callExpr(
+  argumentCountIs(1),
   anyOf(callee(functionDecl(hasName("::std::move"))),
 callee(unresolvedLookupExpr(hasAnyDeclaration(
 namedDecl(hasUnderlyingDecl(hasName("::std::move"))),
-  argumentCountIs(1),
   hasArgument(
   0, argumentOf(
  AllowPartialMove,
  declRefExpr(to(equalsBoundNode("param"))).bind("ref"))),
   unless(hasAncestor(
-  lambdaExpr(valueCapturesVar(equalsBoundNode("param"))
+  lambdaExpr(valueCapturesVar(equalsBoundNode("param"),
+  unless(anyOf(hasAncestor(typeLoc()),
+   hasAncestor(expr(hasUnevaluatedContext())
   .bind("move-call");
 
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -156,6 +156,13 @@
   Obj moved = std::move((o));
 }
 
+void does_not_move_in_evaluated(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  using result_t = decltype(std::move(o));
+  unsigned size = sizeof(std::move(o));
+  Obj moved = o;
+}
+
 template 
 struct mypair {
   T1 first;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "RvalueReferenceParamNotMovedCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matche

[PATCH] D146929: [clang-tidy] Ignore unevaluated exprs in rvalue-reference-param-not-moved

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

I didn't update the release notes since this check is new.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146929

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

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.
Herald added subscribers: mgehre, shchenz.
Herald added projects: clang-tools-extra, All.

bump - I know this is really old, but @lewmpk do you plan on finishing up the 
last remaining comments?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58818

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

[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt added a comment.

I am sorry for protracting implementation. I am still interested to finish this 
(if no volunteer takes over sooner).

In D134475#4070995 , @RIscRIpt wrote:

> I asked MSFT 
> 
>  to comment, let's see if they can share the spec for this attribute.

They did answer! Thank you Cody! For sake of backup, I'll re-post Cody Miller's 
comment here

> In Microsoft Developer Community / msvc::constexpr-specification 
> ,
>  **Cody Miller** wrote:
> Hello!
>
> We added `[[msvc::constexpr]]` to support `std::construct_at` and 
> `std::ranges::construct_at`. These functions are used in lieu of placement 
> new in constexpr contexts to support the forms of dynamic memory allocation 
> that C++20 supports.
>
> We originally implemented this by intercepting the names of the invoked 
> functions and implemented them within the compiler. We (the compiler team) 
> ultimately weren’t satisfied with this approach because it added complexity 
> to the frontend and led to some surprising behavior (such as users 
> implementing their own `std::construct_at` (technically invoking 
> undefined-behavior, I think) and seeing behavior that may have differed from 
> their implementation).
>
> We added the attribute to allow limited contexts to invoke placement new 
> during constexpr evaluation, which allowed the compiler to handle the 
> standard library’s implementation of the two special functions mentioned 
> above.
>
> The semantics are (from memory on a Sunday evening, apologies for unclear or 
> forgotten explanations):
>
> - A function may be either `[[msvc::constexpr]]`, `constexpr`, or neither, 
> but no combination of these is allowed.
> - A function annotated with `[[msvc::constexpr]]` is unofficially called an 
> “extended constexpr” function and may call other `constexpr` or extended 
> constexpr functions.
> - An extended constexpr function has the same restrictions as a constexpr 
> function, except when otherwise noted here.
> - A return statement may be annotated with `[[msvc::constexpr]]` to allow its 
> containing constexpr function to call an extended constexpr function.
> - An extended constexpr function can call other extended constexpr functions 
> without a statement-level annotation.
> - A constexpr function may use placement new within a return statement 
> annotated with `[[msvc::constexpr]]`.
>
> We aimed to add a general mechanism to support this feature, but we only are 
> officially supporting it for use in `std::construct_at` and 
> `std::ranges::construct_at` at the moment – feel free to file bugs if you 
> find issues through experimentation, though!
>
> The restriction to the `return` statement is reflective of us wanting to keep 
> the scope of the attribute small.
>
> Here’s a godbolt link that hopefully clearly captures the above.
>
> We use the annotation (behind the `_MSVC_CONSTEXPR` macro) in our STL 
> implementation in two 
> 
>  places 
> .
>
> Hope this explanation helps! Let me know if anything is unclear.

Currently I am lingered to decide how to implement checks for constant 
evaluation in clang/lib/AST/ExprConstant.cpp 
.
At the moment I have two ideas:
Idea 1: Using helper RAII classes like `BlockScopeRAII` keep track of 
`AttributedStmt` and `ReturnStmt` in `EvalInfo` and/or `CallStackFrame`; then 
in CheckConstexprFunction 

 check whether above rules are followed. However I don't see that `EvalInfo` or 
`CallStackFrame` allow storing references to arbitrary statements, and 
extending these classes only to support `[[msvc::constexpr]]` does not sound 
like a reasonable idea.
Idea 2: Pass `Expr* CallerExpr` to CheckConstexprFunction 
,
 and then traverse parents of `CallerExpr` to check that above rules are 
followed. This would require implementing a simple `ParentVisitor` class which 
would use `ParentMapContext`. I'm not completely sure about this idea, because 
I see that `ParentMapContext` is used seldom and its docs mention about 
"opportunities for optimization" (sounds like performance problems).

By the way, it feels weird that we have to implement complex checks (for 
attributed return statement) only to limit the possibilities of constant 
evaluation; on the other hand without these chec

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a added a comment.

> Additional levels of nesting append 3's: @3, @33, 
> etc.

Yes, but if you have such a code:

  struct A {
union {
  union {
struct B {};
using T = B;
  };
};
  };
  void f(A::T);

you have `?f@@YAXUB@@2A@@@Z` for `f`, and for such a code:

  struct A {
union {
  union {
struct B { struct C {}; };
using T = B::C;
  };
};
  };
  void f(A::T);

you'd have `?f@@YAXUC@B@@3A@@@Z`. What are `2` and `3` in general? 
I don't know whether the aforementioned code is acceptable according to the 
standard, honestly. But if I put a code into the common place, I should figure 
out the common algorithm.


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

https://reviews.llvm.org/D146386

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

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:1247
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error, "cannot mangle anonymous struct/union 
yet");
+  Diags.Report(DiagID);

efriedma wrote:
> maybe say "anonymous struct/union member pointer", if you don't implement 
> this.
It is not specific only to member pointers, as the examples in my previous 
comment show.


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

https://reviews.llvm.org/D146386

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

[clang-tools-extra] 0b103ed - [clangd] Fix a hover crash on unsigned 64bit value

[Younan Zhang via cfe-commits]

Author: Younan Zhang
Date: 2023-03-27T10:27:11+08:00
New Revision: 0b103edf5b2c6178c2725a9ff085c6ccc866bccb

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

LOG: [clangd] Fix a hover crash on unsigned 64bit value

This patch adapts to D140059, which makes an assumption that the
caller of `APSInt::getExtValue` promises no narrowing conversion
happens, i.e., from unsigned int64 to signed int64.

It also fixes clangd/clangd#1557.

Reviewed By: nridge

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

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index e240c22259f35..27663991a7f3c 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -400,9 +400,9 @@ void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
 // 100   => 0x64
 // Negative numbers are sign-extended to 32/64 bits
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getExtValue();
+  uint64_t Bits = V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 728f5444014dc..df46e2fa09951 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3045,6 +3045,15 @@ TEST(Hover, NoCrash) {
 getHover(AST, P, format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashAPInt64) {
+  Annotations T(R"cpp(
+constexpr unsigned long value = -1; // wrap around
+void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1



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

[PATCH] D146874: [clangd] Fix a hover crash on unsigned 64bit value

[Younan Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0b103edf5b2c: [clangd] Fix a hover crash on unsigned 64bit 
value (authored by zyounan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146874

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3045,6 +3045,15 @@
 getHover(AST, P, format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashAPInt64) {
+  Annotations T(R"cpp(
+constexpr unsigned long value = -1; // wrap around
+void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -400,9 +400,9 @@
 // 100   => 0x64
 // Negative numbers are sign-extended to 32/64 bits
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getExtValue();
+  uint64_t Bits = V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3045,6 +3045,15 @@
 getHover(AST, P, format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashAPInt64) {
+  Annotations T(R"cpp(
+constexpr unsigned long value = -1; // wrap around
+void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -400,9 +400,9 @@
 // 100   => 0x64
 // Negative numbers are sign-extended to 32/64 bits
 // -2=> 0xfffe
-// -2^32 => 0xfffe
+// -2^32 => 0x
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getExtValue();
+  uint64_t Bits = V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
 return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Could you upload an updated sample HTML file? It is easier to review the HTML 
generation and javascript code when an example is available.




Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:388
 
+std::unique_ptr flagLogger() {
+  if (DataflowLog.empty())

The name is too short for me. Also it is ambiguous whether "flag" is a noun or 
a verb.

How about `makeLoggerFromCommandLine`?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:388
 
+std::unique_ptr flagLogger() {
+  if (DataflowLog.empty())

gribozavr2 wrote:
> The name is too short for me. Also it is ambiguous whether "flag" is a noun 
> or a verb.
> 
> How about `makeLoggerFromCommandLine`?
Since it is a file-local helper, we can make it static.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:395
+llvm::errs() << "Failed to create log dir: " << EC.message() << "\n";
+  // Separate analyses will create loggers writing to the same directory.
+  // Share a counter so they don't all overwrite each other's 0.html.





Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19
+Function
+
+

Could you add the file name and line number of the start location of the 
function?

It might be helpful not only for the reader (to confirm that they are debugging 
the right thing), but also when finding the right html file using grep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 508461.
4vtomat added a comment.

Resolved MaskRay and Craig's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-marchs.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvMarchHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,22 @@
   return 0;
 }
 
+/// Print supported marchs of the given target.
+static int PrintSupportedMarchs(std::string TargetStr) {
+  std::string Error;
+  const llvm::Target *TheTarget =
+  llvm::TargetRegistry::lookupTarget(TargetStr, Error);
+  if (!TheTarget) {
+llvm::errs() << Error;
+return 1;
+  }
+
+  llvm::riscvMarchHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +240,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-marchs takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedMarchs)
+return PrintSupportedMarchs(Clang->getTargetOpts().Triple);
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-marchs.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-marchs.c
@@ -0,0 +1,98 @@
+// Test that --print-supported-marchs lists supported Marchs.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-marchs 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// Test -march=help alias.
+// RUN: %clang --target=riscv64 -march=help 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The numbers are backreferences of the sort generated by mangleSourceName(), I 
think.  If you nest deeply enough, MSVC stops using them; for example:

  struct A {
  union {union { union {union {
  struct B {struct C {struct D {struct E {struct F {struct G {
  struct H {struct I {struct J {struct K {
  };};};};};};};};};};
  using T = B::C::D::E::F::G::H::I::J::K;
  T x;
  using T2 = B::C::D::E::F::G::H::I::J;
  T2 y;
  };};};};
  };
  void f(decltype(A::x)) {}
  void f2(decltype(A::y)) {}
  void *g = (void*)&f;
  void *g2 = (void*)&f2;


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

https://reviews.llvm.org/D146386

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

[PATCH] D146895: [clang-format] Don't annotate left brace of struct as FunctionLBrace

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 508464.
owenpan added a comment.

Handles consecutive attributes too.


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

https://reviews.llvm.org/D146895

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -366,6 +366,14 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct EXPORT_MACRO [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct [[deprecated]] [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25275,6 +25275,11 @@
"};",
Style);
 
+  verifyFormat("struct EXPORT_MACRO [[nodiscard]] C {\n"
+   "  int i;\n"
+   "};",
+   Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2596,16 +2596,17 @@
   // Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
   if (FormatTok->is(TT_AttributeMacro))
 nextToken();
-  handleCppAttributes();
+  if (FormatTok->is(tok::l_square))
+handleCppAttributes();
 }
 
 bool UnwrappedLineParser::handleCppAttributes() {
   // Handle [[likely]] / [[unlikely]] attributes.
-  if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute()) {
-parseSquare();
-return true;
-  }
-  return false;
+  assert(FormatTok->is(tok::l_square));
+  if (!tryToParseSimpleAttribute())
+return false;
+  parseSquare();
+  return true;
 }
 
 /// Returns whether \c Tok begins a block.
@@ -3704,13 +3705,13 @@
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken &InitialToken = *FormatTok;
   nextToken();
-  handleAttributes();
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, tok::l_square) ||
  ((Style.Language == FormatStyle::LK_Java || Style.isJavaScript()) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
 if (Style.isJavaScript() &&
@@ -3724,16 +3725,15 @@
 continue;
   }
 }
+if (FormatTok->is(tok::l_square) && handleCppAttributes())
+  continue;
 bool IsNonMacroIdentifier =
 FormatTok->is(tok::identifier) &&
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros in between 'class' and the class name.
-if (!IsNonMacroIdentifier) {
-  if (FormatTok->is(tok::l_paren)) {
-parseParens();
-  }
-}
+if (!IsNonMacroIdentifier && FormatTok->is(tok::l_paren))
+  parseParens();
   }
 
   // Note that parsing away template declarations here leads to incorrectly


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -366,6 +366,14 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct EXPORT_MACRO [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct [[deprecated]] [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25275,6 +25275,11 @@
  

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126959#4222637 , @bjope wrote:

> This seem to case problems when building with asan enabled 
> (LLVM_USE_SANITIZER='Address'):

investigating... do you need the patch reverted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:189
+  std::string Error;
+  const llvm::Target *TheTarget =
+  llvm::TargetRegistry::lookupTarget(TargetStr, Error);

Why do we need to lookup the TargetRegistry?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4346
   MarshallingInfoFlag>;
+def print_supported_marchs : Flag<["-", "--"], "print-supported-marchs">,
+  Group, Flags<[CC1Option, CoreOption]>,

Maybe this should be -print-supported-extensions to match RISC-V terminology?



Comment at: clang/include/clang/Driver/Options.td:4349
+  HelpText<"Print supported marchs for the given target (if target is not 
specified,"
+   " it will print the supported marchs for the default target)">,
+  MarshallingInfoFlag>;

There's no default target behavior. It's always RISC-V.



Comment at: clang/lib/Driver/Driver.cpp:4232
+if (!C.getDefaultToolChain().getTriple().isRISCV()) {
+  llvm::errs() << "The -march=help only supports for RISC-V target.\n";
+  return;

"-march=help is only supported for RISC-V"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D146891: [Driver][NetBSD] Simplify NetBSD version handling

[Michał Górny via Phabricator via cfe-commits]

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

WFM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146891

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

[clang] 9a9827d - [Driver][NetBSD] Simplify NetBSD version handling

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2023-03-27T00:01:42-04:00
New Revision: 9a9827dc1185f3757433b973fbb37c7ad7219e32

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

LOG: [Driver][NetBSD] Simplify NetBSD version handling

NetBSD 6.x and older is ancient. Remove now unnecessary version check.

Reviewed By: mgorny

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/NetBSD.cpp
clang/test/Driver/netbsd.c
clang/test/Driver/netbsd.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/NetBSD.cpp 
b/clang/lib/Driver/ToolChains/NetBSD.cpp
index 7a7c905e3e7a6..07bdc20e85545 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -272,28 +272,25 @@ void netbsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back(Args.MakeArgString(ToolChain.getCompilerRTPath()));
   }
 
-  VersionTuple OsVersion = Triple.getOSVersion();
   bool useLibgcc = true;
-  if (OsVersion >= VersionTuple(7) || OsVersion.getMajor() == 0) {
-switch (ToolChain.getArch()) {
-case llvm::Triple::aarch64:
-case llvm::Triple::aarch64_be:
-case llvm::Triple::arm:
-case llvm::Triple::armeb:
-case llvm::Triple::thumb:
-case llvm::Triple::thumbeb:
-case llvm::Triple::ppc:
-case llvm::Triple::ppc64:
-case llvm::Triple::ppc64le:
-case llvm::Triple::sparc:
-case llvm::Triple::sparcv9:
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
-  useLibgcc = false;
-  break;
-default:
-  break;
-}
+  switch (ToolChain.getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::aarch64_be:
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
+  case llvm::Triple::sparc:
+  case llvm::Triple::sparcv9:
+  case llvm::Triple::x86:
+  case llvm::Triple::x86_64:
+useLibgcc = false;
+break;
+  default:
+break;
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
@@ -412,26 +409,23 @@ Tool *NetBSD::buildAssembler() const {
 Tool *NetBSD::buildLinker() const { return new tools::netbsd::Linker(*this); }
 
 ToolChain::CXXStdlibType NetBSD::GetDefaultCXXStdlibType() const {
-  VersionTuple OsVersion = getTriple().getOSVersion();
-  if (OsVersion >= VersionTuple(7) || OsVersion.getMajor() == 0) {
-switch (getArch()) {
-case llvm::Triple::aarch64:
-case llvm::Triple::aarch64_be:
-case llvm::Triple::arm:
-case llvm::Triple::armeb:
-case llvm::Triple::thumb:
-case llvm::Triple::thumbeb:
-case llvm::Triple::ppc:
-case llvm::Triple::ppc64:
-case llvm::Triple::ppc64le:
-case llvm::Triple::sparc:
-case llvm::Triple::sparcv9:
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
-  return ToolChain::CST_Libcxx;
-default:
-  break;
-}
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::aarch64_be:
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
+  case llvm::Triple::sparc:
+  case llvm::Triple::sparcv9:
+  case llvm::Triple::x86:
+  case llvm::Triple::x86_64:
+return ToolChain::CST_Libcxx;
+  default:
+break;
   }
   return ToolChain::CST_Libstdcxx;
 }

diff  --git a/clang/test/Driver/netbsd.c b/clang/test/Driver/netbsd.c
index 3fefaeb100e2c..4daa35a4a7f94 100644
--- a/clang/test/Driver/netbsd.c
+++ b/clang/test/Driver/netbsd.c
@@ -17,9 +17,6 @@
 // RUN: %clang --target=x86_64-unknown-netbsd7.0.0 \
 // RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=X86_64-7 %s
-// RUN: %clang --target=x86_64-unknown-netbsd6.0.0 \
-// RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=X86_64-6 %s
 // RUN: %clang --target=aarch64-unknown-netbsd \
 // RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=AARCH64 %s
@@ -62,9 +59,6 @@
 // RUN: %clang --target=arm-unknown-netbsd7.0.0-eabi \
 // RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=ARM-7 %s
-// RUN: %clang --target=arm-unknown-netbsd6.0.0-eabi \
-// RUN: --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=ARM-6 %s
 // RUN: %clang --target=sparc-unknown-netbsd \
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC %s
@@ -84,9 +78,6 @@
 // RUN: %clang --target=x86_64-unknown-netbsd7.0.0 -static \
 // RUN

[PATCH] D146891: [Driver][NetBSD] Simplify NetBSD version handling

[Brad Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a9827dc1185: [Driver][NetBSD] Simplify NetBSD version 
handling (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146891

Files:
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Driver/netbsd.c
  clang/test/Driver/netbsd.cpp

Index: clang/test/Driver/netbsd.cpp
===
--- clang/test/Driver/netbsd.cpp
+++ clang/test/Driver/netbsd.cpp
@@ -4,12 +4,6 @@
 // RUN: %clangxx --target=x86_64-unknown-netbsd7.0.0 \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=X86_64-7 %s
-// RUN: %clangxx --target=x86_64-unknown-netbsd6.0.0 \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=X86_64-6 %s
-// RUN: %clangxx --target=arm-unknown-netbsd6.0.0-eabi \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=ARM %s
 // RUN: %clangxx --target=arm-unknown-netbsd7.0.0-eabi \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=ARM-7 %s
@@ -28,18 +22,12 @@
 // RUN: %clangxx --target=sparc-unknown-netbsd \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC %s
-// RUN: %clangxx --target=sparc-unknown-netbsd6.0.0 \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC-6 %s
 // RUN: %clangxx --target=sparc-unknown-netbsd7.0.0 \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC-7 %s
 // RUN: %clangxx --target=sparc64-unknown-netbsd \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC64 %s
-// RUN: %clangxx --target=sparc64-unknown-netbsd6.0.0 \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=SPARC64-6 %s
 // RUN: %clangxx --target=sparc64-unknown-netbsd7.0.0 \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC64-7 %s
@@ -56,12 +44,6 @@
 // RUN: %clangxx --target=x86_64-unknown-netbsd7.0.0 -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-X86_64-7 %s
-// RUN: %clangxx --target=x86_64-unknown-netbsd6.0.0 -static \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=S-X86_64-6 %s
-// RUN: %clangxx --target=arm-unknown-netbsd6.0.0-eabi -static \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=S-ARM %s
 // RUN: %clangxx --target=arm-unknown-netbsd7.0.0-eabi -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-ARM-7 %s
@@ -80,18 +62,12 @@
 // RUN: %clangxx --target=sparc-unknown-netbsd -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-SPARC %s
-// RUN: %clangxx --target=sparc-unknown-netbsd6.0.0 -static \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=S-SPARC-6 %s
 // RUN: %clangxx --target=sparc-unknown-netbsd7.0.0 -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-SPARC-7 %s
 // RUN: %clangxx --target=sparc64-unknown-netbsd -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-SPARC64 %s
-// RUN: %clangxx --target=sparc64-unknown-netbsd6.0.0 -static \
-// RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
-// RUN: | FileCheck -check-prefix=S-SPARC64-6 %s
 // RUN: %clangxx --target=sparc64-unknown-netbsd7.0.0 -static \
 // RUN: -stdlib=platform --sysroot=%S/Inputs/basic_netbsd_tree -### %s 2>&1 \
 // RUN: | FileCheck -check-prefix=S-SPARC64-7 %s
@@ -114,20 +90,6 @@
 // X86_64-7: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc++"
 // X86_64-7: "-lm" "-lc" "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o"
 
-// X86_64-6: "-cc1" "-triple" "x86_64-unknown-netbsd6.0.0"
-// X86_64-6: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/libexec/ld.elf_so"
-// X86_64-6: "-o" "a.out" "{{.*}}/usr/lib{{/|}}crt0.o" "{{.*}}/usr/lib{{/|}}crti.o"
-// X86_64-6: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lstdc++"
-// X86_64-6: "-lm" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
-// X86_64-6: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508474.
ccotter added a comment.

- fix bad rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
@@ -5,6 +5,22 @@
 // CHECK-FIXES-NOT: for ({{.*[^:]:[^:].*}})
 // CHECK-MESSAGES-NOT: modernize-loop-convert
 
+namespace somenamespace {
+  template  auto begin(T& t) -> decltype(t.begin());
+  template  auto begin(const T& t) -> decltype(t.begin());
+  template  auto end(T& t) -> decltype(t.end());
+  template  auto end(const T& t) -> decltype(t.end());
+  template  auto size(const T& t) -> decltype(t.size());
+} // namespace somenamespace
+
+struct SomeClass {
+  template  static auto begin(T& t) -> decltype(t.begin());
+  template  static auto begin(const T& t) -> decltype(t.begin());
+  template  static auto end(T& t) -> decltype(t.end());
+  template  static auto end(const T& t) -> decltype(t.end());
+  template  static auto size(const T& t) -> decltype(t.size());
+};
+
 namespace Negative {
 
 const int N = 6;
@@ -92,7 +108,7 @@
   }
 }
 
-}
+} // namespace Negative
 
 namespace NegativeIterator {
 
@@ -103,6 +119,10 @@
 struct BadBeginEnd : T {
   iterator notBegin();
   iterator notEnd();
+  iterator begin(int);
+  iterator end(int);
+  iterator begin();
+  iterator end();
 };
 
 void notBeginOrEnd() {
@@ -112,6 +132,9 @@
 
   for (T::iterator I = Bad.begin(), E = Bad.notEnd();  I != E; ++I)
 int K = *I;
+
+  for (T::iterator I = Bad.begin(0), E = Bad.end(0);  I != E; ++I)
+int K = *I;
 }
 
 void badLoopShapes() {
@@ -202,6 +225,8 @@
   T Other;
   for (T::iterator I = Tt.begin(), E = Other.end();  I != E; ++I)
 int K = *I;
+  for (T::iterator I = begin(Tt), E = end(Other);  I != E; ++I)
+int K = *I;
 
   for (T::iterator I = Other.begin(), E = Tt.end();  I != E; ++I)
 int K = *I;
@@ -214,6 +239,24 @@
 MutableVal K = *I;
 }
 
+void mixedMemberAndADL() {
+  for (T::iterator I = Tt.begin(), E = end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = end(Tt);  I != E; ++I)
+int K = *I;
+}
+
+void nonADLOrStdCalls() {
+  for (T::iterator I = SomeClass::begin(Tt), E = SomeClass::end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = somenamespace::begin(Tt), E = somenamespace::end(Tt);  I != E; ++I)
+int K = *I;
+}
+
 void wrongIterators() {
   T::iterator Other;
   for (T::iterator I = Tt.begin(), E = Tt.end(); I != Other; ++I)
@@ -379,6 +422,13 @@
 Sum += V[I];
 }
 
+void nonADLOrStdCalls() {
+  for (int I = 0, E = somenamespace::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+  for (int I = 0, E = SomeClass::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+}
+
 // Checks to see that non-const member functions are not called on the container
 // object.
 // These could be conceivably allowed with a lower required confidence level.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,41 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s0 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s0 has value %d\n", It.X);
+
+  for (S::iterator It = std::begin(Ss), E = std::end(Ss); It != E; ++It) {
+printf("s1 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s1 has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps), E = end(*Ps); It 

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 508475.
4vtomat added a comment.

Resolved Craig's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-marchs.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvMarchHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,14 @@
   return 0;
 }
 
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +232,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return PrintSupportedExtensions(Clang->getTargetOpts().Triple);
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-marchs.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-marchs.c
@@ -0,0 +1,98 @@
+// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// Test -march=help alias.
+// RUN: %clang --target=riscv64 -march=help 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz  1.0
+// CHECK-NEXT: 	zicsr   2.0
+// CHECK-NEXT: 	zifencei2.0
+// CHECK-NEXT: 	zihintpause 2.0
+// CHECK-NEXT: 	zmmul  

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508476.
ccotter added a comment.

- alpha


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-negative.cpp
@@ -5,6 +5,22 @@
 // CHECK-FIXES-NOT: for ({{.*[^:]:[^:].*}})
 // CHECK-MESSAGES-NOT: modernize-loop-convert
 
+namespace somenamespace {
+  template  auto begin(T& t) -> decltype(t.begin());
+  template  auto begin(const T& t) -> decltype(t.begin());
+  template  auto end(T& t) -> decltype(t.end());
+  template  auto end(const T& t) -> decltype(t.end());
+  template  auto size(const T& t) -> decltype(t.size());
+} // namespace somenamespace
+
+struct SomeClass {
+  template  static auto begin(T& t) -> decltype(t.begin());
+  template  static auto begin(const T& t) -> decltype(t.begin());
+  template  static auto end(T& t) -> decltype(t.end());
+  template  static auto end(const T& t) -> decltype(t.end());
+  template  static auto size(const T& t) -> decltype(t.size());
+};
+
 namespace Negative {
 
 const int N = 6;
@@ -92,7 +108,7 @@
   }
 }
 
-}
+} // namespace Negative
 
 namespace NegativeIterator {
 
@@ -103,6 +119,10 @@
 struct BadBeginEnd : T {
   iterator notBegin();
   iterator notEnd();
+  iterator begin(int);
+  iterator end(int);
+  iterator begin();
+  iterator end();
 };
 
 void notBeginOrEnd() {
@@ -112,6 +132,9 @@
 
   for (T::iterator I = Bad.begin(), E = Bad.notEnd();  I != E; ++I)
 int K = *I;
+
+  for (T::iterator I = Bad.begin(0), E = Bad.end(0);  I != E; ++I)
+int K = *I;
 }
 
 void badLoopShapes() {
@@ -202,6 +225,8 @@
   T Other;
   for (T::iterator I = Tt.begin(), E = Other.end();  I != E; ++I)
 int K = *I;
+  for (T::iterator I = begin(Tt), E = end(Other);  I != E; ++I)
+int K = *I;
 
   for (T::iterator I = Other.begin(), E = Tt.end();  I != E; ++I)
 int K = *I;
@@ -214,6 +239,24 @@
 MutableVal K = *I;
 }
 
+void mixedMemberAndADL() {
+  for (T::iterator I = Tt.begin(), E = end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = Tt.end();  I != E; ++I)
+int K = *I;
+  for (T::iterator I = std::begin(Tt), E = end(Tt);  I != E; ++I)
+int K = *I;
+}
+
+void nonADLOrStdCalls() {
+  for (T::iterator I = SomeClass::begin(Tt), E = SomeClass::end(Tt);  I != E; ++I)
+int K = *I;
+  for (T::iterator I = somenamespace::begin(Tt), E = somenamespace::end(Tt);  I != E; ++I)
+int K = *I;
+}
+
 void wrongIterators() {
   T::iterator Other;
   for (T::iterator I = Tt.begin(), E = Tt.end(); I != Other; ++I)
@@ -379,6 +422,13 @@
 Sum += V[I];
 }
 
+void nonADLOrStdCalls() {
+  for (int I = 0, E = somenamespace::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+  for (int I = 0, E = SomeClass::size(V); E != I; ++I)
+printf("Fibonacci number is %d\n", V[I]);
+}
+
 // Checks to see that non-const member functions are not called on the container
 // object.
 // These could be conceivably allowed with a lower required confidence level.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,41 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s0 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s0 has value %d\n", It.X);
+
+  for (S::iterator It = std::begin(Ss), E = std::end(Ss); It != E; ++It) {
+printf("s1 has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s1 has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps), E = end(*Ps); It != E; ++I

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.
Herald added a subscriber: PiotrZSL.

@Febbe - checking in. is this still on your radar? I would love to see this 
merged!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

What about classes that doesn't have begin/end method but got cbegin/cend ? 
I thing there is open issue for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D146929: [clang-tidy] Ignore unevaluated exprs in rvalue-reference-param-not-moved

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL 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/D146929/new/

https://reviews.llvm.org/D146929

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+  if (isa(Node) || isa(Node))

move this matcher to some utils/...
It may be needed by other checks.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+  }
+};
+

ccotter wrote:
> PiotrZSL wrote:
> > add here that thing about UnlessSpelledInSource
> Sorry, I think I'm missing some context here. Would you mind clarifying your 
> comment?
I mean:
```std::optional getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}```
just to specify this explicitly..




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst:7
+Warns when a forwarding reference parameter is not forwarded inside the
+function body.
+

Add link to cpp guidelines, mention rule name like in other checks.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:189
`cppcoreguidelines-avoid-reference-coroutine-parameters 
`_,
+   `cppcoreguidelines-forwarding-reference-param-not-forwarded 
`_, "Yes"
`cppcoreguidelines-init-variables 
`_, "Yes"

no fixes



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases

what about when someone uses std::move instead of std::format ?
maybe some "note" for such issue ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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