[PATCH] D146370: [Clang][OpenMP]Solved the the always truth condition in Arm64

[Samuel Maina via Phabricator via cfe-commits]

samuelmaina updated this revision to Diff 506345.
samuelmaina added a comment.



Updating D146370: [Clang][OpenMP]Solved the the always truth condition in Arm64
===

Reformatted the file
Reformatted the CGOpenMPRuntime.cpp to conform to clang standards


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146370

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp

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

[PATCH] D146371: [Clang][OpenMP]Solved the the always truth condition in Arm64

[Samuel Maina via Phabricator via cfe-commits]

samuelmaina created this revision.
Herald added subscribers: sunshaoce, guansong, kristof.beyls, yaxunl.
Herald added a project: All.
samuelmaina requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146371

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp

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

[PATCH] D146370: [Clang][OpenMP]Solved the the always truth condition in Arm64

[Samuel Maina via Phabricator via cfe-commits]

samuelmaina created this revision.
Herald added subscribers: sunshaoce, guansong, kristof.beyls, yaxunl.
Herald added a project: All.
samuelmaina requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146370

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp

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

[PATCH] D141892: Implement modernize-use-constraints

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 506339.
ccotter added a comment.
Herald added a subscriber: PiotrZSL.

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& 

[PATCH] D145435: Choose style (file) from within code for use in IDEs

[bers via Phabricator via cfe-commits]

bersbersbers updated this revision to Diff 506336.
bersbersbers added a comment.

Rebased patch against main branch (previous patch was against 15.0.7 tag)


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

https://reviews.llvm.org/D145435

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp

Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -4292,6 +4292,10 @@
   if (CommentPragmasRegex.match(IndentContent))
 return false;
 
+  // If comment is a style comment, treat as separate section.
+  if (IndentContent.ltrim().startswith(clang::format::StyleComment))
+return false;
+
   // If Line starts with a line comment, then FormatTok continues the comment
   // section if its original column is greater or equal to the original start
   // column of the line.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -3741,6 +3741,29 @@
   if (!getPredefinedStyle(FallbackStyleName, Style.Language, ))
 return make_string_error("Invalid fallback style \"" + FallbackStyleName);
 
+  if (StyleName == "file") {
+// Read style from first style comment at start of line in the code.
+std::string Prefix{std::string("// ") + StyleComment};
+size_t PrefixPos = StringRef::npos;
+while ((PrefixPos = Code.find(Prefix, PrefixPos + 1)) != StringRef::npos) {
+  // For each prefix, locate respective start of line (SoL) and check text
+  // between prefix and start of line for non-whitespace (comments not at
+  // start of line up to whitespace may be quoted, commented out, ...).
+  if (const size_t SoL = Code.substr(0, PrefixPos).find_last_of("\r\n") + 1;
+  !Code.substr(SoL, PrefixPos - SoL).ltrim().empty()) {
+continue;
+  }
+
+  // Use remainder of line as `StyleName` as if passed by `-style=...`;
+  // if no end of line (EoL) found, use remainder of file.
+  const size_t StylePos = PrefixPos + Prefix.size();
+  const size_t EoL = Code.find_first_of("\r\n", StylePos);
+  const size_t StyleLen = EoL - (EoL == StringRef::npos ? 0 : StylePos);
+  StyleName = Code.substr(StylePos, StyleLen);
+  break;
+}
+  }
+
   llvm::SmallVector, 1>
   ChildFormatTextToApply;
 
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -2192,7 +2192,7 @@
 }();
 if (!Style.ReflowComments ||
 CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
-switchesFormatting(Current) || !RegularComments) {
+setsStyle(Current) || switchesFormatting(Current) || !RegularComments) {
   return nullptr;
 }
 return std::make_unique(
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -27,6 +27,10 @@
 namespace clang {
 namespace format {
 
+/// Checks if \p Token sets style, basically, // clang-format style=
+/// \p Token must be a line comment.
+bool setsStyle(const FormatToken );
+
 /// Checks if \p Token switches formatting, like /* clang-format off */.
 /// \p Token must be a comment.
 bool switchesFormatting(const FormatToken );
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -216,6 +216,12 @@
   return BreakableToken::Split(StringRef::npos, 0);
 }
 
+bool setsStyle(const FormatToken ) {
+  assert(Token.is(TT_LineComment) && "style is set by line-comment token");
+  StringRef Content = Token.TokenText.substr(2).ltrim();
+  return Content.startswith(StyleComment);
+}
+
 bool switchesFormatting(const FormatToken ) {
   assert((Token.is(TT_BlockComment) || Token.is(TT_LineComment)) &&
  "formatting regions are switched by comment tokens");
@@ -339,7 +345,7 @@
   // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
   bool hasSpecialMeaningPrefix = false;
   for (StringRef Prefix :
-   {"@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* "}) {
+   {"@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* ", StyleComment}) {
 if (Content.startswith(Prefix)) {
   hasSpecialMeaningPrefix = true;
   break;
@@ -743,6 +749,7 @@
 IndentContent = Lines[LineIndex].ltrim(Blanks).substr(1);
   return LineIndex > 0 && 

[PATCH] D144912: [clang-tidy] readability-identifier-naming: fix hungarian enum prefix in C

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau added a comment.

In D144912#4204088 , @carlosgalvezp 
wrote:

> Looks great, approved! Thanks for fixing :)

Thanks for the review :)

I don't have push access, so please can you (or anyone else) land it for me ?
(as `Alexis Murzeau ` (phabricator mail and handle is the 
same as github))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144912

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

There are 2 issues found in llvm repository:

- llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp:721:22: warning: reference variable 
'S' extends the lifetime of a just-constructed temporary object 'const 
StringRef', consider changing reference to value 
[readability-reference-to-constructed-temporary]
- clang/lib/Sema/SemaChecking.cpp:10096:36: warning: reference variable 'AT2' 
extends the lifetime of a just-constructed temporary object 'const 
analyze_printf::ArgType', consider changing reference to value 
[readability-reference-to-constructed-temporary]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

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

Detects code where a temporary object is directly constructed by calling
a constructor or using an initializer list and immediately assigned to a
reference variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146368

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-reference-to-constructed-temporary %t
+
+struct WithConstructor
+{
+WithConstructor(int, int);
+};
+
+struct WithoutConstructor
+{
+int a, b;
+};
+
+void test()
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: reference variable 'tmp1' extends the lifetime of a just-constructed temporary object 'const WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithConstructor& tmp1{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: reference variable 'tmp2' extends the lifetime of a just-constructed temporary object 'const WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithoutConstructor& tmp2{1,2};
+
+
+// CHECK-MESSAGES: :[[@LINE+1]]:22: warning: reference variable 'tmp3' extends the lifetime of a just-constructed temporary object 'WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithConstructor&& tmp3{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: reference variable 'tmp4' extends the lifetime of a just-constructed temporary object 'WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithoutConstructor&& tmp4{1,2};
+
+   WithConstructor tmp5{1,2};
+   WithoutConstructor tmp6{1,2};
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-reference-to-constructed-temporary
+
+readability-reference-to-constructed-temporary
+==
+
+Detects C++ code where a reference variable is used to extend the lifetime of
+a temporary object that has just been constructed.
+
+This construction is often the result of multiple code refactorings or a lack
+of developer knowledge, leading to confusion or subtle bugs. In most cases,
+what the developer really wanted to do is create a new variable rather than
+extending the lifetime of a temporary object.
+
+Examples of problematic code include:
+
+.. code-block:: c++
+
+   const std::string& value("hello");
+
+   struct Point { int x; int y; };
+   const Point& p = { 1, 2 };
+
+In the first example, a ``const std::string&`` reference variable ``str`` is
+assigned a temporary object created by the ``std::string("hello")``
+constructor. In the second example, a ``const Point&`` reference variable ``p``
+is assigned an object that is constructed from an initializer list ``{ 1, 2 }``.
+Both of these examples extend the lifetime of the temporary object to the
+lifetime of the reference variable, which can make it difficult to reason about
+and may lead to subtle bugs or misunderstanding.
+
+To avoid these issues, it is recommended to change the reference variable to a
+(``const``) value variable.
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
@@ -363,6 +363,7 @@
`readability-redundant-smartptr-get `_, "Yes"
`readability-redundant-string-cstr `_, "Yes"
`readability-redundant-string-init `_, "Yes"
+   `readability-reference-to-constructed-temporary `_,

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/Interp.cpp:373
+   const FieldDecl *SubObjDecl) {
+  S.FFDiag(SI, diag::note_constexpr_uninitialized) << SubObjDecl;
+  S.Note(SubObjDecl->getLocation(),

shafik wrote:
> Do we need to check that `SubObjDecl` is not `nullptr` ever?
If we are sure it can never be `nullptr` then we should `assert(SubObjDecl)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146358

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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Thank you for submitting this fix.




Comment at: clang/lib/AST/ExprConstant.cpp:2270
+ Info, MTE->getExprLoc(), TempType, *V, Kind,
+ nullptr, CheckedTemps))
 return false;

To conform to [clang-tidy bugprone-argument-comment 
format](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)



Comment at: clang/lib/AST/ExprConstant.cpp:2399
Value.getStructBase(BaseIndex), Kind,
-   BS.getBeginLoc(), CheckedTemps))
+   nullptr, CheckedTemps))
   return false;

same here.



Comment at: clang/lib/AST/ExprConstant.cpp:2443
   return CheckEvaluationResult(CheckEvaluationResultKind::ConstantExpression,
-   Info, DiagLoc, Type, Value, Kind,
-   SourceLocation(), CheckedTemps);
+   Info, DiagLoc, Type, Value, Kind, nullptr,
+   CheckedTemps);

Also here



Comment at: clang/lib/AST/ExprConstant.cpp:2454
+   Info, DiagLoc, Type, Value,
+   ConstantExprKind::Normal, nullptr, 
CheckedTemps);
 }

and here



Comment at: clang/lib/AST/Interp/Interp.cpp:373
+   const FieldDecl *SubObjDecl) {
+  S.FFDiag(SI, diag::note_constexpr_uninitialized) << SubObjDecl;
+  S.Note(SubObjDecl->getLocation(),

Do we need to check that `SubObjDecl` is not `nullptr` ever?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146358

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

[PATCH] D146364: [Driver] Fix naming conflicts of getStatsFileName when using LTO

[Yingwei Zheng via Phabricator via cfe-commits]

dtcxzyw created this revision.
dtcxzyw added reviewers: liaolucy, StephenFan.
Herald added a subscriber: inglorion.
Herald added a project: All.
dtcxzyw requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

When using `clang` with `-flto=thin -save-stats=obj` to compile a multi-file 
program, `clang` will save internal statistics during the link time code 
generation to `/.stats` because `BaseName` is empty. When multiple 
binaries are placed in the same directory, conflicts will be caused by 
identical filenames for statistics. This patch uses the output filename as the 
base name instead of the input when `-save-stats=obj`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146364

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1774,14 +1774,13 @@
 StringRef SaveStats = A->getValue();
 if (SaveStats == "obj" && Output.isFilename()) {
   StatsFile.assign(Output.getFilename());
-  llvm::sys::path::remove_filename(StatsFile);
-} else if (SaveStats != "cwd") {
+} else if (SaveStats == "cwd") {
+  StatsFile.assign(Input.getBaseInput());
+} else {
   D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
   return {};
 }
 
-StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
-llvm::sys::path::append(StatsFile, BaseName);
 llvm::sys::path::replace_extension(StatsFile, "stats");
   } else {
 assert(D.CCPrintInternalStats);


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1774,14 +1774,13 @@
 StringRef SaveStats = A->getValue();
 if (SaveStats == "obj" && Output.isFilename()) {
   StatsFile.assign(Output.getFilename());
-  llvm::sys::path::remove_filename(StatsFile);
-} else if (SaveStats != "cwd") {
+} else if (SaveStats == "cwd") {
+  StatsFile.assign(Input.getBaseInput());
+} else {
   D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
   return {};
 }
 
-StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
-llvm::sys::path::append(StatsFile, BaseName);
 llvm::sys::path::replace_extension(StatsFile, "stats");
   } else {
 assert(D.CCPrintInternalStats);
___
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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 506310.
PiotrZSL added a comment.

Change name of isStatic* to isImmutable*


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,75 @@
+.. title:: clang-tidy - readability-avoid-unconditional-preprocessor-if
+
+readability-avoid-unconditional-preprocessor-if
+===
+
+Check flags always enabled or disabled code blocks in preprocessor ``#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
+
+Preprocessor directives are a powerful feature in C and C++ that allow code to
+be modified before it is compiled. These directives can be used to include or
+exclude code based on conditions, or to define macros that can be used
+throughout the code. However, the misuse of preprocessor directives can create
+several issues that can impact the readability 

[PATCH] D146354: [clang][ExtractAPI] Add semicolons for enum, typedef, struct declaration fragments

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 506309.
chaitanyav added a comment.

- Update reference output in underscored.c
- combine return and append semicolon fragment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146354

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef_chain.c
===
--- clang/test/ExtractAPI/typedef_chain.c
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -68,6 +68,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -128,6 +132,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -188,6 +196,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -75,6 +75,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -170,6 +174,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -223,6 +231,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -283,6 +295,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -343,6 +359,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +423,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef.c
===
--- clang/test/ExtractAPI/typedef.c
+++ clang/test/ExtractAPI/typedef.c
@@ -66,6 +66,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -89,6 +89,10 @@
 {
   "kind": "identifier",
   "spelling": "Color"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/enum.c
===
--- clang/test/ExtractAPI/enum.c
+++ clang/test/ExtractAPI/enum.c
@@ -153,6 +153,10 @@
   "kind": "typeIdentifier",
   "preciseIdentifier": 

[PATCH] D146354: [clang][ExtractAPI] Add semicolons for enum, typedef, struct declaration fragments

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 506308.
chaitanyav added a comment.

- Update reference output in underscored.c
- combine return and append semicolon fragment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146354

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/underscored.c


Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -470,8 +470,7 @@
 getFragmentsForType(IntegerType, EnumDecl->getASTContext(), After))
 .append(std::move(After));
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -495,8 +494,7 @@
 Fragments.appendSpace().append(
 Record->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -746,8 +744,7 @@
   .appendSpace()
   .append(Decl->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 template 


Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -470,8 +470,7 @@
 getFragmentsForType(IntegerType, EnumDecl->getASTContext(), After))
 .append(std::move(After));
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -495,8 +494,7 @@
 Fragments.appendSpace().append(
 Record->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -746,8 +744,7 @@
   .appendSpace()
   .append(Decl->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 template 
___
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 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp:41
+
+  bool isStatic(SourceManager , const LangOptions ,
+SourceRange ConditionRange) {

"static" is an overloaded C++ keyword which can lead to confusion in this 
context (you don't intend to detect whether a token is the `static` keyword). 
Would it be possible to give this a different name? Same for `isStaticToken`



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:67-69
+#if 10 > DDD
+// some code
+#endif

PiotrZSL wrote:
> carlosgalvezp wrote:
> > Add case for if 10 < DDD ?
> > 
> > Also, add test case for comparing 2 macros? If people follow the "no magic 
> > numbers" policy they'll likely have defines for both sides of the 
> > comparison.
> I don't support currently macros, would need to check if they defined 
> unconditionally in same file, and I didn't want to mess with this.
Yes I didn't mean "function-like macros", just a plain define like you just 
added. Great, 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] D146363: Update reference output in underscored.c

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav created this revision.
Herald added a reviewer: ributzka.
Herald added a project: All.
chaitanyav requested review of this revision.
Herald added a reviewer: dang.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

update reference output in underscored.c, combine return and append semicolon 
fragment


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146363

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/underscored.c


Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -470,8 +470,7 @@
 getFragmentsForType(IntegerType, EnumDecl->getASTContext(), After))
 .append(std::move(After));
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -495,8 +494,7 @@
 Fragments.appendSpace().append(
 Record->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -746,8 +744,7 @@
   .appendSpace()
   .append(Decl->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 template 


Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -470,8 +470,7 @@
 getFragmentsForType(IntegerType, EnumDecl->getASTContext(), After))
 .append(std::move(After));
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -495,8 +494,7 @@
 Fragments.appendSpace().append(
 Record->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments
@@ -746,8 +744,7 @@
   .appendSpace()
   .append(Decl->getName(), DeclarationFragments::FragmentKind::Identifier);
 
-  Fragments.append(";", DeclarationFragments::FragmentKind::Text);
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 template 
___
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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:67-69
+#if 10 > DDD
+// some code
+#endif

carlosgalvezp wrote:
> Add case for if 10 < DDD ?
> 
> Also, add test case for comparing 2 macros? If people follow the "no magic 
> numbers" policy they'll likely have defines for both sides of the comparison.
I don't support currently macros, would need to check if they defined 
unconditionally in same file, and I didn't want to mess with this.


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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 506304.
PiotrZSL marked 3 inline comments as done.
PiotrZSL added a comment.

Removed some whitespace & added tests


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,75 @@
+.. title:: clang-tidy - readability-avoid-unconditional-preprocessor-if
+
+readability-avoid-unconditional-preprocessor-if
+===
+
+Check flags always enabled or disabled code blocks in preprocessor ``#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
+
+Preprocessor directives are a powerful feature in C and C++ that allow code to
+be modified before it is compiled. These directives can be used to include or
+exclude code based on conditions, or to define macros that can be used
+throughout the code. However, the misuse of preprocessor directives can create
+several 

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:16
+To configure the check to enforce the traditional token representation, you can
+set the `BinaryOperators` and `OverloadedOperators` options to ``&&,||,!``,
+respectively. This will cause the check to warn on any occurrences of ``and``,

carlosgalvezp wrote:
> Is there any reason why this distinction is needed? The usage of the operator 
> happens in client code - it's unlikely one would want different style 
> depending on whether the operator is overloaded or not?
> 
> ```
> bool x = some_bool and some_other_bool;
> bool y = some_object && some_other_object;
> ```
Consider code like this:

```
template
void archive(T&& a, std::optional& v)
{
a & v;
}

and

bool test(unsigned value, unsigned mask)
{
return value & mask;
}
```

Those are 2 different use-cases, probably you don't want to use bit_and, but 
you could use bit_and for value & mask.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:28-29
+
+Alternative Token Representation
+
+

carlosgalvezp wrote:
> Personally I find this to be a matter of style, and arguments could be found 
> for either style. As such I don't think this check should promote the style 
> preference of the author of the check.
> 
> It's like having a written discussion in the clang-format option 
> `ColumnLimit` explaining why N characters is a better choice than M 
> characters. In the end this is a decision each project should take, and the 
> check should not have a bias towards a preferred choice.
Following this way of thinking none of value is correct. Always will be group 
that is for it or against it. Initially this check had to be only to enforce 
alternative tokens but then made it configurable and change name of check. This 
check is for "readability" and alternative tokens are more readable. If someone 
don't like them, their choose, they can change config to their needs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144522

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

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:28-29
+
+Alternative Token Representation
+
+

carlosgalvezp wrote:
> Personally I find this to be a matter of style, and arguments could be found 
> for either style. As such I don't think this check should promote the style 
> preference of the author of the check.
> 
> It's like having a written discussion in the clang-format option 
> `ColumnLimit` explaining why N characters is a better choice than M 
> characters. In the end this is a decision each project should take, and the 
> check should not have a bias towards a preferred choice.
FYI @njames93 , do you have an opinion on the matter as Code Owner?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144522

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

[PATCH] D144912: [clang-tidy] readability-identifier-naming: fix hungarian enum prefix in C

[Carlos Galvez via Phabricator via cfe-commits]

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

> Without this patch, the suggested fix in a C file by clang-tidy is iRevValid 
> instead of rtRevValid.

Ah there it was, thanks for the clarification! It was not highlighted in the 
diff.

> Would the new C test file would have been better in a separate patch too 
> (thus keep only the main change in the checker) ?

That would definitely have helped reviewing! Coming into the patch I was 
expecting the diff to look like either fixing an existing test, or adding a new 
test (in the same file). You did add a new test, but it happened to be an 
almost exact copy of another one, so Phabricator only displayed the diff 
against the copy, not highlighting the issue the patch is fixing.

Looks great, approved! Thanks for fixing :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144912

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

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:16
+To configure the check to enforce the traditional token representation, you can
+set the `BinaryOperators` and `OverloadedOperators` options to ``&&,||,!``,
+respectively. This will cause the check to warn on any occurrences of ``and``,

Is there any reason why this distinction is needed? The usage of the operator 
happens in client code - it's unlikely one would want different style depending 
on whether the operator is overloaded or not?

```
bool x = some_bool and some_other_bool;
bool y = some_object && some_other_object;
```



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:28-29
+
+Alternative Token Representation
+
+

Personally I find this to be a matter of style, and arguments could be found 
for either style. As such I don't think this check should promote the style 
preference of the author of the check.

It's like having a written discussion in the clang-format option `ColumnLimit` 
explaining why N characters is a better choice than M characters. In the end 
this is a decision each project should take, and the check should not have a 
bias towards a preferred choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144522

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

Looks good, minor comments!




Comment at: 
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h:16
+
+///  Check flags always enabled or disabled code blocks in preprocessor `#if`
+///  conditions, such as `#if 0` and `#if 1`.

Ultra nit: remove extra space



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:17-31
+// 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 
'true', consider removing condition but leaving its contents 
[readability-avoid-unconditional-preprocessor-if]
+#if 10 > 5

For completeness, would it make sense to add the same test but with the "is 
always 'false'" case?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:67-69
+#if 10 > DDD
+// some code
+#endif

Add case for if 10 < DDD ?

Also, add test case for comparing 2 macros? If people follow the "no magic 
numbers" policy they'll likely have defines for both sides of the comparison.


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] D144912: [clang-tidy] readability-identifier-naming: fix hungarian enum prefix in C

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau added a comment.

In D144912#4203920 , @carlosgalvezp 
wrote:

> Looks good but I fail to understand what exactly the patch fixes, can you 
> point me to an example in the tests?

The patch fixes these enum tests in C language mode:

  enum REV_TYPE { RevValid };
  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for enum 
constant 'RevValid' [readability-identifier-naming]
  // CHECK-FIXES: {{^}}enum REV_TYPE { rtRevValid };
  
  enum EnumConstantCase { OneByte, TwoByte };
  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for enum 
constant 'OneByte' [readability-identifier-naming]
  // CHECK-MESSAGES: :[[@LINE-2]]:34: warning: invalid case style for enum 
constant 'TwoByte' [readability-identifier-naming]
  // CHECK-FIXES: {{^}}enum EnumConstantCase { eccOneByte, eccTwoByte };

Without this patch, the suggested fix in a C file by clang-tidy is `iRevValid` 
instead of `rtRevValid`.
This patch makes the behavior on C files the same as C++ files.

In D144912#4203920 , @carlosgalvezp 
wrote:

> It would be easier to review if the NFC changes had been done in a separate 
> patch.

Ok sorry, will do that next time.
Would the new C test file would have been better in a separate patch too (thus 
keep only the main change in the checker) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144912

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

[PATCH] D129070: [clang-tidy] Fixed an issue in readability-identifier-naming not using options specified.

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ed40ac90ce8: [clang-tidy] Fixed an issue in 
readability-identifier-naming not using options… (authored by njames93, 
committed by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D129070?vs=504434=506298#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129070

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: { \
+// RUN: readability-identifier-naming.GlobalConstantPrefix: "", \
+// RUN: readability-identifier-naming.GlobalVariablePrefix: g_ }}'
+
+int BadGlobalVariable;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global 
variable 'BadGlobalVariable' [readability-identifier-naming]
+// CHECK-FIXES: int g_BadGlobalVariable;
+int g_GoodGlobalVariable;
+
+const int GoodGlobalConstant = 0;
+const int g_IgnoreGlobalConstant = 0;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -223,6 +223,11 @@
   ` to understand that there is a
   sequence point between designated initializers.
 
+- Fixed an issue in :doc:`readability-identifier-naming
+  ` when specifying an empty
+  string for ``Prefix`` or ``Suffix`` options could result in the style not
+  being used.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -68,8 +68,8 @@
   struct NamingStyle {
 NamingStyle() = default;
 
-NamingStyle(std::optional Case, const std::string ,
-const std::string , const std::string ,
+NamingStyle(std::optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr,
 HungarianPrefixType HPType);
 NamingStyle(const NamingStyle ) = delete;
 NamingStyle =(NamingStyle &) = default;
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -230,9 +230,8 @@
 // clang-format on
 
 IdentifierNamingCheck::NamingStyle::NamingStyle(
-std::optional Case,
-const std::string , const std::string ,
-const std::string , HungarianPrefixType HPType)
+std::optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType)
 : Case(Case), Prefix(Prefix), Suffix(Suffix),
   IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) {
   if (!IgnoredRegexpStr.empty()) {
@@ -265,22 +264,21 @@
 
 memcpy([StyleSize], "IgnoredRegexp", 13);
 StyleString.truncate(StyleSize + 13);
-StringRef IgnoredRegexpStr = Options.get(StyleString, "");
+std::optional IgnoredRegexpStr = Options.get(StyleString);
 memcpy([StyleSize], "Prefix", 6);
 StyleString.truncate(StyleSize + 6);
-std::string Prefix(Options.get(StyleString, ""));
+std::optional Prefix(Options.get(StyleString));
 // Fast replacement of [Pre]fix -> [Suf]fix.
 memcpy([StyleSize], "Suf", 3);
-std::string Postfix(Options.get(StyleString, ""));
+std::optional Postfix(Options.get(StyleString));
 memcpy([StyleSize], "Case", 4);
 StyleString.pop_back_n(2);
-auto CaseOptional =
+std::optional CaseOptional =
 Options.get(StyleString);
 
-if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
-!IgnoredRegexpStr.empty() || HPTOpt)
-  Styles[I].emplace(std::move(CaseOptional), std::move(Prefix),
-std::move(Postfix), IgnoredRegexpStr.str(),
+if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt)
+  Styles[I].emplace(std::move(CaseOptional), Prefix.value_or(""),
+Postfix.value_or(""), IgnoredRegexpStr.value_or(""),
 HPTOpt.value_or(IdentifierNamingCheck::HPT_Off));
   }
   bool IgnoreMainLike = 

[clang-tools-extra] 3ed40ac - [clang-tidy] Fixed an issue in readability-identifier-naming not using options specified.

[Piotr Zegar via cfe-commits]

Author: Nathan James
Date: 2023-03-18T15:54:25Z
New Revision: 3ed40ac90ce8a91f0990fe2d847feb6663606f93

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

LOG: [clang-tidy] Fixed an issue in readability-identifier-naming not using 
options specified.

Fixed an issue where specifying empty strings for prefix or suffix would be 
ignored preventing using those to override a different style.

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

Reviewed By: carlosgalvezp

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 61421a13d73ba..9b2b9f7672f4d 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -230,9 +230,8 @@ static StringRef const HungarainNotationUserDefinedTypes[] 
= {
 // clang-format on
 
 IdentifierNamingCheck::NamingStyle::NamingStyle(
-std::optional Case,
-const std::string , const std::string ,
-const std::string , HungarianPrefixType HPType)
+std::optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType)
 : Case(Case), Prefix(Prefix), Suffix(Suffix),
   IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) {
   if (!IgnoredRegexpStr.empty()) {
@@ -265,22 +264,21 @@ IdentifierNamingCheck::FileStyle 
IdentifierNamingCheck::getFileStyleFromOptions(
 
 memcpy([StyleSize], "IgnoredRegexp", 13);
 StyleString.truncate(StyleSize + 13);
-StringRef IgnoredRegexpStr = Options.get(StyleString, "");
+std::optional IgnoredRegexpStr = Options.get(StyleString);
 memcpy([StyleSize], "Prefix", 6);
 StyleString.truncate(StyleSize + 6);
-std::string Prefix(Options.get(StyleString, ""));
+std::optional Prefix(Options.get(StyleString));
 // Fast replacement of [Pre]fix -> [Suf]fix.
 memcpy([StyleSize], "Suf", 3);
-std::string Postfix(Options.get(StyleString, ""));
+std::optional Postfix(Options.get(StyleString));
 memcpy([StyleSize], "Case", 4);
 StyleString.pop_back_n(2);
-auto CaseOptional =
+std::optional CaseOptional =
 Options.get(StyleString);
 
-if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
-!IgnoredRegexpStr.empty() || HPTOpt)
-  Styles[I].emplace(std::move(CaseOptional), std::move(Prefix),
-std::move(Postfix), IgnoredRegexpStr.str(),
+if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt)
+  Styles[I].emplace(std::move(CaseOptional), Prefix.value_or(""),
+Postfix.value_or(""), IgnoredRegexpStr.value_or(""),
 HPTOpt.value_or(IdentifierNamingCheck::HPT_Off));
   }
   bool IgnoreMainLike = Options.get("IgnoreMainLikeFunctions", false);

diff  --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
index cbf0653a280b6..b1db919902e22 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -68,8 +68,8 @@ class IdentifierNamingCheck final : public 
RenamerClangTidyCheck {
   struct NamingStyle {
 NamingStyle() = default;
 
-NamingStyle(std::optional Case, const std::string ,
-const std::string , const std::string ,
+NamingStyle(std::optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr,
 HungarianPrefixType HPType);
 NamingStyle(const NamingStyle ) = delete;
 NamingStyle =(NamingStyle &) = default;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index b53516a742e2c..e1bbe6776f1e8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -223,6 +223,11 @@ Changes in existing checks
   ` to understand that there is a
   sequence point between designated initializers.
 
+- Fixed an issue in :doc:`readability-identifier-naming
+  ` when specifying an empty
+  string for ``Prefix`` or ``Suffix`` options could result in the style not
+  being used.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
 

[PATCH] D145721: [HIP] clang should pass `-mno-amdgpu-ieee` to -cc1

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 506296.
yaxunl added a comment.

fix tests


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

https://reviews.llvm.org/D145721

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -126,3 +126,15 @@
 // RUN:   --offload-arch=gfx906 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=KANNEG %s
 // KANNEG-NOT: "-fhip-kernel-arg-name"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mno-amdgpu-ieee -mamdgpu-ieee \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=IEEE-ON %s
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-m{{(no-)?}}amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF 
%s
+// IEEE-OFF: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mno-amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefixes=IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mamdgpu-ieee"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7181,6 +7181,8 @@
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
+Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
+   options::OPT_mno_amdgpu_ieee);
   }
 
   // For all the host OpenMP offloading compile jobs we need to pass the 
targets


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -126,3 +126,15 @@
 // RUN:   --offload-arch=gfx906 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=KANNEG %s
 // KANNEG-NOT: "-fhip-kernel-arg-name"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mno-amdgpu-ieee -mamdgpu-ieee \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=IEEE-ON %s
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-m{{(no-)?}}amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee -ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF %s
+// IEEE-OFF: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mno-amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee -ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mamdgpu-ieee"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7181,6 +7181,8 @@
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
+Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
+   options::OPT_mno_amdgpu_ieee);
   }
 
   // For all the host OpenMP offloading compile jobs we need to pass the targets
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146168: [Sema] Stop stripping CV quals from *this captures in lambdas

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 506295.
royjacobson added a comment.

Add the test case from the GH issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146168

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
@@ -308,3 +303,22 @@
 z(id,3);
 }
 } // namespace PR45881
+
+
+namespace GH50866 {
+struct S;
+
+void f(S *) = delete; // expected-note {{would lose const qualifier}}
+void f(const S *) = delete; // expected-note {{candidate function has been explicitly deleted}}
+
+struct S {
+  void g() const {
+[*this]() mutable { f(this); }(); // expected-error {{call to deleted function}}
+  }
+};
+
+void g() {
+  S s{};
+  s.g();
+}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,8 +1135,7 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
 }
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // The type of the corresponding data member (not a 'this' pointer if 'by
 // copy').
-QualType CaptureType = ThisTy;
-if (ByCopy) {
-  // If we are capturing the object referred to by '*this' by copy, ignore
-  // any cv qualifiers inherited from the type of the member function for
-  // the type of the closure-type's corresponding data member and any use
-  // of 'this'.
-  CaptureType = ThisTy->getPointeeType();
-  CaptureType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-}
+QualType CaptureType = ByCopy ? 

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet created this revision.
hazohelet added reviewers: aaron.ballman, tbaeder, shafik.
Herald added a project: All.
hazohelet requested review of this revision.
Herald added a project: clang.

This patch improves the diagnostic on uninitialized subobjects in constexpr 
variables by modifying the diagnostic message to display the subobject's name 
instead of its type.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146358

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/test/AST/Interp/cxx20.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -761,7 +761,7 @@
 };
 
 S s1; // expected-error {{call to consteval function 'NamespaceScopeConsteval::S::S' is not a constant expression}} \
- expected-note {{subobject of type 'int' is not initialized}}
+ expected-note {{subobject named 'Val' is not initialized}}
 
 template 
 struct T {
@@ -770,7 +770,7 @@
 };
 
 T t; // expected-error {{call to consteval function 'NamespaceScopeConsteval::T::T' is not a constant expression}} \
- expected-note {{subobject of type 'int' is not initialized}}
+ expected-note {{subobject named 'Val' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
 
Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -409,12 +409,12 @@
 b.a.x = 2;
 return b;
   }
-  constexpr B uninit = return_uninit(); // expected-error {{constant expression}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr B uninit = return_uninit(); // expected-error {{constant expression}} expected-note {{subobject named 'y' is not initialized}}
   static_assert(return_uninit().a.x == 2);
   constexpr A return_uninit_struct() {
 B b = {.b = 1};
 b.a.x = 2;
-return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note {{subobject of type 'int' is not initialized}}
+return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note {{subobject named 'y' is not initialized}}
   }
   // Note that this is rejected even though return_uninit() is accepted, and
   // return_uninit() copies the same stuff wrapped in a union.
@@ -558,7 +558,7 @@
 }
   };
   constinit X x1(true);
-  constinit X x2(false); // expected-error {{constant initializer}} expected-note {{constinit}} expected-note {{subobject of type 'int' is not initialized}}
+  constinit X x2(false); // expected-error {{constant initializer}} expected-note {{constinit}} expected-note {{subobject named 'n' is not initialized}}
 
   struct Y {
 struct Z { int n; }; // expected-note {{here}}
@@ -577,7 +577,7 @@
   };
   // FIXME: This is working around clang not implementing DR2026. With that
   // fixed, we should be able to test this without the injected copy.
-  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} expected-note {{subobject named 'n' is not initialized}}
   constexpr Y y1 = copy(Y());
   static_assert(y1.z1.n == 1 && y1.z2.n == 2 && y1.z3.n == 3);
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -359,7 +359,7 @@
   // The constructor is still 'constexpr' here, but the result is not intended
   // to be a constant expression. The standard is not clear on how this should
   // work.
-  constexpr V v; // expected-error {{constant expression}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr V v; // expected-error {{constant expression}} expected-note {{subobject named 'y' is not initialized}}
 
   constexpr int k = V().x; // FIXME: ok?
 }
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -143,9 +143,9 @@
 constexpr A() {}
   };
   constexpr A a; // expected-error {{must be initialized by a constant expression}} \
- // expected-note {{subobject of type 'int' is not initialized}} \
+ // expected-note {{subobject named 'a' is not initialized}} \
  // ref-error {{must be initialized by a constant expression}} \
-   

[PATCH] D145958: [clang-tidy] Fix minor bug in bugprone-too-small-loop-variable

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG113f0190dd38: [clang-tidy] Fix minor bug in 
bugprone-too-small-loop-variable (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145958

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -373,3 +373,20 @@
   }
 }
 
+void goodForLoopWithBitfieldOnUpperBoundOnly() {
+  struct S {
+int x : 4;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
+
+void goodForLoopWithIntegersOnUpperBoundOnly() {
+  struct S {
+short x;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -34,6 +34,11 @@
   bool operator<(const MagnitudeBits ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) const noexcept {
+return WidthWithoutSignBit != Other.WidthWithoutSignBit ||
+   BitFieldWidth != Other.BitFieldWidth;
+  }
 };
 
 } // namespace
@@ -184,13 +189,19 @@
   if (LoopVar->getType() != LoopIncrement->getType())
 return;
 
-  const QualType LoopVarType = LoopVar->getType();
-  const QualType UpperBoundType = UpperBound->getType();
-
   ASTContext  = *Result.Context;
 
+  const QualType LoopVarType = LoopVar->getType();
   const MagnitudeBits LoopVarMagnitudeBits =
   calcMagnitudeBits(Context, LoopVarType, LoopVar);
+
+  const MagnitudeBits LoopIncrementMagnitudeBits =
+  calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement);
+  // We matched the loop variable incorrectly.
+  if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits)
+return;
+
+  const QualType UpperBoundType = UpperBound->getType();
   const MagnitudeBits UpperBoundMagnitudeBits =
   calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
 


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -373,3 +373,20 @@
   }
 }
 
+void goodForLoopWithBitfieldOnUpperBoundOnly() {
+  struct S {
+int x : 4;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
+
+void goodForLoopWithIntegersOnUpperBoundOnly() {
+  struct S {
+short x;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -34,6 +34,11 @@
   bool operator<(const MagnitudeBits ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) const noexcept {
+return WidthWithoutSignBit != Other.WidthWithoutSignBit ||
+   BitFieldWidth != Other.BitFieldWidth;
+  }
 };
 
 } // namespace
@@ -184,13 +189,19 @@
   if (LoopVar->getType() != LoopIncrement->getType())
 return;
 
-  const QualType LoopVarType = LoopVar->getType();
-  const QualType UpperBoundType = UpperBound->getType();
-
   ASTContext  = *Result.Context;
 
+  const QualType LoopVarType = LoopVar->getType();
   const MagnitudeBits LoopVarMagnitudeBits =
   calcMagnitudeBits(Context, LoopVarType, LoopVar);
+
+  const MagnitudeBits LoopIncrementMagnitudeBits =
+  calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement);
+  // We matched the loop variable incorrectly.
+  if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits)
+return;
+
+  const QualType UpperBoundType = UpperBound->getType();
   const MagnitudeBits UpperBoundMagnitudeBits =
   calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 113f019 - [clang-tidy] Fix minor bug in bugprone-too-small-loop-variable

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-18T10:44:13Z
New Revision: 113f0190dd386d93b42d781b975aa2cca5c88914

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

LOG: [clang-tidy] Fix minor bug in bugprone-too-small-loop-variable

Correct issue when incorrectly matched bitfield loop
variable would still be considered valid and equal to
base type, because check didnt compare size of bitfield.

Fixes issue introduced in: D142587

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 133fcabd78392..8ba8b893e03a6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -34,6 +34,11 @@ struct MagnitudeBits {
   bool operator<(const MagnitudeBits ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) const noexcept {
+return WidthWithoutSignBit != Other.WidthWithoutSignBit ||
+   BitFieldWidth != Other.BitFieldWidth;
+  }
 };
 
 } // namespace
@@ -184,13 +189,19 @@ void TooSmallLoopVariableCheck::check(const 
MatchFinder::MatchResult ) {
   if (LoopVar->getType() != LoopIncrement->getType())
 return;
 
-  const QualType LoopVarType = LoopVar->getType();
-  const QualType UpperBoundType = UpperBound->getType();
-
   ASTContext  = *Result.Context;
 
+  const QualType LoopVarType = LoopVar->getType();
   const MagnitudeBits LoopVarMagnitudeBits =
   calcMagnitudeBits(Context, LoopVarType, LoopVar);
+
+  const MagnitudeBits LoopIncrementMagnitudeBits =
+  calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement);
+  // We matched the loop variable incorrectly.
+  if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits)
+return;
+
+  const QualType UpperBoundType = UpperBound->getType();
   const MagnitudeBits UpperBoundMagnitudeBits =
   calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 1505dbfef519e..7821c4144ccfe 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -373,3 +373,20 @@ void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
   }
 }
 
+void goodForLoopWithBitfieldOnUpperBoundOnly() {
+  struct S {
+int x : 4;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
+
+void goodForLoopWithIntegersOnUpperBoundOnly() {
+  struct S {
+short x;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}



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

[PATCH] D146288: clang-tidy: Detect use-after-move in CXXCtorInitializer

[Piotr Zegar via Phabricator via cfe-commits]

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

Requesting changes due to lack of support for base class initializes & got some 
concerns related lambdas used in initializers.




Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:400
 void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
   auto CallMoveMatcher =
+  callExpr(

This is correct but consider this:

```
auto TryEmplaceMatcher = 
cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace";

auto CallMoveMatcher =
callExpr(argumentCountIs(1), // because matching this will be faster than 
checking name.
 callee(functionDecl(hasName("::std::move"))),
 unless(inDecltypeOrTemplateArg()),
 expr().bind("call-move"),
 unless(hasParent(TryEmplaceMatcher)),
 
anyOf(hasAncestor(compoundStmt(hasParent(lambdaExpr().bind("containing-lambda",

hasAncestor(functionDecl(anyOf(cxxConstructorDecl(hasAnyConstructorInitializer(

   withInitializer(expr(

  anyOf(equalsBounNode("call-move"), 

 
hasDescendant(equalsBounNode("call-move")))

 ).bind("containing-ctor-init-stmt")

  ))

 ).bind("containing-ctor"),
 
functionDecl().bind("containing-func")
);
```
  



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:474
+  for (CXXCtorInitializer *Init : ContainingCtor->inits()) {
+if (Init->getInit() == ContainingCtorInitStmt) {
+  ContainingCtorField = Init->getMember();

consider using here `->IgnoreImplicit() == 
ContainingCtorInitStmt->IgnoreImplicit()`



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:479-488
+  if (ContainingCtorField) {
+// Collect the constructor initializer expressions.
+for (CXXCtorInitializer *Init : ContainingCtor->inits()) {
+  if (Init->getMember() && Init->getMember()->getFieldIndex() >=
+   ContainingCtorField->getFieldIndex()) {
+if (Init->getInit())
+  CodeBlocks.push_back(Init->getInit());

This won't work for std::move used to initialize base class. Assuming that 
Inits are in specific order, then you just should include all init's after one 
found, or use things like `SourceManager::isBeforeInTranslationUnit`, but still 
probably using something like:

```
bool PastMove = false;
for (CXXCtorInitializer *Init : ContainingCtor->inits()) {
if (!PastMove  && Init->getInit()->IgnoreImplicit() == 
ContainingCtorInitStmt->IgnoreImplicit()) {
PastMove  = true;
}

if (PastMove && Init->getInit())
{
CodeBlocks.push_back(Init->getInit());
}
```

would be better, as it should work for both field inits and base constructor 
calls




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1431
   std::string val_;
 };

Missing tests:
- Test with A derive from B, C, D, and argument is moved into C, but D 
initialization also uses it.
- Test with lambda call inside init, and in lambda std::move of captured by 
reference constructor parameter, and lambda could be mutable, and lambda could 
be saved into member (or passed to base class), or called instantly (better 
both tests).
- Test with lambda that takes argument into capture by move

In short:
```
struct Obj {};
struct D
{
D(Obj b);
};

struct C
{
 C(Obj b);
};

struct B
{
 B(Obj& b);
}

struct A : B, C, D
{
A(Obj b)
  : B(b), C(std::move(b)), D(b)
{
}
};

and second:

struct Base
{
Base(Obj b) : bb(std::move(b)) {}
 template
 Base(T&& b) : bb(b()) {};
   
 Obj bb;
};

struct Derived : Base, C
{
 Derived(Obj b) 
   : Base([&] mutable { return std::move(b); }()), C(b)
   {
   }
};

struct Derived2 : Base, C
{
 Derived2(Obj b) 
   : Base([&] mutable { return std::move(b); }), C(b)
   {
   }
};

struct Derived3 : Base, C
{
 Derived3(Obj b) 
   : Base([c = std::move(b)] mutable { return std::move(c); }), C(b)
   {
   }
};

[PATCH] D144912: [clang-tidy] readability-identifier-naming: fix hungarian enum prefix in C

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Looks good but I fail to understand what exactly the patch fixes, can you point 
me to an example in the tests? It would be easier to review if the NFC changes 
had been done in a separate patch.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c:4
+// clang-format off
+typedef signed char int8_t; // NOLINT
+typedef short   int16_t;// NOLINT

PiotrZSL wrote:
> those typedefs looks to be duplicated already in 
> identifier-naming-hungarian-notation.cpp and 
> identifier-naming-hungarian-notation-cfgfile.cpp, move them to separate 
> header file
Next time please apply this change in a separate NFC patch, for easier review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144912

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

[PATCH] D143287: [Clang][X86] Change X86 cast intrinsics to use __builtin_nondeterministic_value

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

In D143287#4202174 , @ManuelJBrito 
wrote:

> Implementing the 128 to 512 casts by filling the rest of the vector with the 
> same definition of a nondeterministic_value is not correct because  :
>
> a = freeze poison
> v = 
>
> is not the same as
>
> v = freeze poison
>
> The only solution I'm seeing ,using the shufflevector, is doing the 
> conversion in two steps:
>
> - build a 256 vector with the upper half being undefined( freeze poison)
> - build a 512 vector where the lower half is the previous 256 vector and the 
> upper half being undefined
>
> I think this would require two shuffles which is unfortunate.
>
> This would ensure no miscompilations due to multiple uses of the same freeze 
> undef/poison but would probably require some backend work to ensure the 
> pattern is recognized to emit efficient assembly.

Semantically that is correct.
But the backend may require some tweaks to recognize this pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143287

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

[PATCH] D146354: [clang][ExtractAPI] Add semicolons for enum, typedef, struct declaration fragments

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav created this revision.
chaitanyav added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
chaitanyav requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146354

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c

Index: clang/test/ExtractAPI/typedef_chain.c
===
--- clang/test/ExtractAPI/typedef_chain.c
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -68,6 +68,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -128,6 +132,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -188,6 +196,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -75,6 +75,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -170,6 +174,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -223,6 +231,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -283,6 +295,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -343,6 +359,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +423,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef.c
===
--- clang/test/ExtractAPI/typedef.c
+++ clang/test/ExtractAPI/typedef.c
@@ -66,6 +66,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -89,6 +89,10 @@
 {
   "kind": "identifier",
   "spelling": "Color"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/enum.c
===
--- clang/test/ExtractAPI/enum.c
+++ clang/test/ExtractAPI/enum.c
@@ -153,6 +153,10 @@
   "kind": "typeIdentifier",
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "docComment": {
@@ -461,6 +465,10 @@
   "kind": "typeIdentifier",
   "preciseIdentifier": "c:c",
   "spelling": "unsigned char"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -684,6 +692,10 @@
   "kind": "typeIdentifier",
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -772,6 +784,10 @@
   "kind": "typeIdentifier",
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
+},
+{
+