date:20230409

[PATCH] D147121: [hwasan] remove requirment for PIE

2023-04-09 Thread Mingjie Xu via Phabricator via cfe-commits

Enna1 added a comment.

gentle ping, @eugenis PTAL, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147121

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

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-04-09 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

Note: I did not rebase this on top of D146941 
, because I'd like to suggest that this fix 
be backported to the 16.x branch (and presumably that's not the case for 
D146941 ); however, I'm happy to rebase 
D146941  on top of this if that would be 
helpful.

I'd like to add test coverage for this to `system-include-extractor.test`, just 
wanted to post the patch for feedback in the meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147905

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

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-04-09 Thread Nathan Ridge via Phabricator via cfe-commits

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147905

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


Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -333,6 +333,13 @@
   else if (Arg.startswith("-x"))
 Lang = Arg.drop_front(2).trim();
 }
+// Downgrade objective-c++-header (used in clangd's fallback flags for .h
+// files) to c++-header, as some drivers may fail to run the extraction
+// command if it contains `-xobjective-c++-header` and objective-c++ 
support
+// is not installed.
+if (Lang == "objective-c++-header") {
+  Lang = "c++-header";
+}
 if (Lang.empty()) {
   llvm::StringRef Ext = llvm::sys::path::extension(File).trim('.');
   auto Type = driver::types::lookupTypeForExtension(Ext);


Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -333,6 +333,13 @@
   else if (Arg.startswith("-x"))
 Lang = Arg.drop_front(2).trim();
 }
+// Downgrade objective-c++-header (used in clangd's fallback flags for .h
+// files) to c++-header, as some drivers may fail to run the extraction
+// command if it contains `-xobjective-c++-header` and objective-c++ support
+// is not installed.
+if (Lang == "objective-c++-header") {
+  Lang = "c++-header";
+}
 if (Lang.empty()) {
   llvm::StringRef Ext = llvm::sys::path::extension(File).trim('.');
   auto Type = driver::types::lookupTypeForExtension(Ext);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147893: [clang-tidy] avoid colon in namespace cause false positve

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:175
 
+  llvm::errs() << Namespaces.size() << " " << ND.getName() << " clear\n";
   Namespaces.clear();

debug log...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147893

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 512073.
junaire added a comment.

fix some nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/Mangle.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Interpreter/Value.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
@@ -33,6 +34,10 @@
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
 #endif
 
+int Global = 42;
+int getGlobal() { return Global; }
+void setGlobal(int val) { Global = val; }
+
 namespace {
 using Args = std::vector;
 static std::unique_ptr
@@ -276,8 +281,7 @@
   std::vector Args = {"-fno-delayed-template-parsing"};
   std::unique_ptr Interp = createInterpreter(Args);
 
-  llvm::cantFail(Interp->Parse("void* operator new(__SIZE_TYPE__, void* __p);"
-   "extern \"C\" int printf(const char*,...);"
+  llvm::cantFail(Interp->Parse("extern \"C\" int printf(const char*,...);"
"class A {};"
"struct B {"
"  template"
@@ -314,4 +318,55 @@
   free(NewA);
 }
 
+TEST(InterpreterTest, Value) {
+  std::unique_ptr Interp = createInterpreter();
+
+  Value V1;
+  llvm::cantFail(Interp->ParseAndExecute("int x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("x", ));
+  EXPECT_TRUE(V1.isValid());
+  EXPECT_EQ(V1.getInt(), 42);
+  EXPECT_TRUE(V1.getType()->isIntegerType());
+  EXPECT_EQ(V1.getKind(), Value::K_Int);
+  EXPECT_FALSE(V1.isManuallyAlloc());
+  EXPECT_FALSE(V1.isPointerOrObjectType());
+
+  Value V2;
+  llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
+  llvm::cantFail(Interp->ParseAndExecute("y", ));
+  EXPECT_TRUE(V2.isValid());
+  EXPECT_EQ(V2.getDouble(), 3.14);
+  EXPECT_TRUE(V2.getType()->isFloatingType());
+  EXPECT_EQ(V2.getKind(), Value::K_Double);
+  EXPECT_FALSE(V2.isManuallyAlloc());
+  EXPECT_FALSE(V2.isPointerOrObjectType());
+
+  Value V3;
+  llvm::cantFail(Interp->ParseAndExecute(
+  "struct S { int* p; S() { p = new int(42); } ~S() { delete p; }};"));
+  llvm::cantFail(Interp->ParseAndExecute("S{}", ));
+  EXPECT_TRUE(V3.isValid());
+  EXPECT_TRUE(V3.getType()->isRecordType());
+  EXPECT_EQ(V3.getKind(), Value::K_PtrOrObj);
+  EXPECT_TRUE(V3.isManuallyAlloc());
+  EXPECT_TRUE(V3.isPointerOrObjectType());
+
+  Value V4;
+  llvm::cantFail(Interp->ParseAndExecute("int getGlobal();"));
+  llvm::cantFail(Interp->ParseAndExecute("void setGlobal(int);"));
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V4.getInt(), 42);
+  EXPECT_TRUE(V4.getType()->isIntegerType());
+
+  Value V5;
+  // Change the global from the compiled code.
+  setGlobal(43);
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V5.getInt(), 43);
+  EXPECT_TRUE(V5.getType()->isIntegerType());
+
+  // Change the global from the interpreted code.
+  llvm::cantFail(Interp->ParseAndExecute("setGlobal(44);"));
+  EXPECT_EQ(getGlobal(), 44);
+}
 } // end anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -22,3 +22,5 @@
 if(NOT WIN32)
   add_subdirectory(ExceptionTests)
 endif()
+
+export_executable_symbols(ClangReplInterpreterTests)
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Parse/Parser.cpp

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 512072.
junaire added a comment.

Address some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/Mangle.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Interpreter/Value.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
@@ -33,6 +34,10 @@
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
 #endif
 
+int Global = 42;
+int getGlobal() { return Global; }
+void setGlobal(int val) { Global = val; }
+
 namespace {
 using Args = std::vector;
 static std::unique_ptr
@@ -276,8 +281,7 @@
   std::vector Args = {"-fno-delayed-template-parsing"};
   std::unique_ptr Interp = createInterpreter(Args);
 
-  llvm::cantFail(Interp->Parse("void* operator new(__SIZE_TYPE__, void* __p);"
-   "extern \"C\" int printf(const char*,...);"
+  llvm::cantFail(Interp->Parse("extern \"C\" int printf(const char*,...);"
"class A {};"
"struct B {"
"  template"
@@ -314,4 +318,55 @@
   free(NewA);
 }
 
+TEST(InterpreterTest, Value) {
+  std::unique_ptr Interp = createInterpreter();
+
+  Value V1;
+  llvm::cantFail(Interp->ParseAndExecute("int x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("x", ));
+  EXPECT_TRUE(V1.isValid());
+  EXPECT_EQ(V1.getInt(), 42);
+  EXPECT_TRUE(V1.getType()->isIntegerType());
+  EXPECT_EQ(V1.getKind(), Value::K_Int);
+  EXPECT_FALSE(V1.isManuallyAlloc());
+  EXPECT_FALSE(V1.isPointerOrObjectType());
+
+  Value V2;
+  llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
+  llvm::cantFail(Interp->ParseAndExecute("y", ));
+  EXPECT_TRUE(V2.isValid());
+  EXPECT_EQ(V2.getDouble(), 3.14);
+  EXPECT_TRUE(V2.getType()->isFloatingType());
+  EXPECT_EQ(V2.getKind(), Value::K_Double);
+  EXPECT_FALSE(V2.isManuallyAlloc());
+  EXPECT_FALSE(V2.isPointerOrObjectType());
+
+  Value V3;
+  llvm::cantFail(Interp->ParseAndExecute(
+  "struct S { int* p; S() { p = new int(42); } ~S() { delete p; }};"));
+  llvm::cantFail(Interp->ParseAndExecute("S{}", ));
+  EXPECT_TRUE(V3.isValid());
+  EXPECT_TRUE(V3.getType()->isRecordType());
+  EXPECT_EQ(V3.getKind(), Value::K_PtrOrObj);
+  EXPECT_TRUE(V3.isManuallyAlloc());
+  EXPECT_TRUE(V3.isPointerOrObjectType());
+
+  Value V4;
+  llvm::cantFail(Interp->ParseAndExecute("int getGlobal();"));
+  llvm::cantFail(Interp->ParseAndExecute("void setGlobal(int);"));
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V4.getInt(), 42);
+  EXPECT_TRUE(V4.getType()->isIntegerType());
+
+  Value V5;
+  // Change the global from the compiled code.
+  setGlobal(43);
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V5.getInt(), 43);
+  EXPECT_TRUE(V5.getType()->isIntegerType());
+
+  // Change the global from the interpreted code.
+  llvm::cantFail(Interp->ParseAndExecute("setGlobal(44);"));
+  EXPECT_EQ(getGlobal(), 44);
+}
 } // end anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -22,3 +22,5 @@
 if(NOT WIN32)
   add_subdirectory(ExceptionTests)
 endif()
+
+export_executable_symbols(ClangReplInterpreterTests)
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Parse/Parser.cpp

[PATCH] D147904: [Clang] Fix cast to BigIntType in hasUniqueObjectRepresentations

2023-04-09 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson created this revision.
royjacobson added reviewers: shafik, cjdb.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A bad QualType cast caused us not to detect _BigInt types if they were wrapped 
inside sugar types like SubstTemplateTypeParm.
Fix https://github.com/llvm/llvm-project/issues/62019


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147904

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2970,6 +2970,12 @@
   bool b = __has_unique_object_representations(T);
 };
 
+static_assert(!has_unique_object_representations<_BitInt(7)>::value, 
"BitInt:");
+static_assert(has_unique_object_representations<_BitInt(8)>::value, "BitInt:");
+static_assert(!has_unique_object_representations<_BitInt(127)>::value, 
"BitInt:");
+static_assert(has_unique_object_representations<_BitInt(128)>::value, 
"BitInt:");
+
+
 namespace PR46209 {
   // Foo has both a trivial assignment operator and a non-trivial one.
   struct Foo {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2832,7 +2832,7 @@
   // All integrals and enums are unique.
   if (Ty->isIntegralOrEnumerationType()) {
 // Except _BitInt types that have padding bits.
-if (const auto *BIT = dyn_cast(Ty))
+if (const auto *BIT = Ty->getAs())
   return getTypeSize(BIT) == BIT->getNumBits();
 
 return true;


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2970,6 +2970,12 @@
   bool b = __has_unique_object_representations(T);
 };
 
+static_assert(!has_unique_object_representations<_BitInt(7)>::value, "BitInt:");
+static_assert(has_unique_object_representations<_BitInt(8)>::value, "BitInt:");
+static_assert(!has_unique_object_representations<_BitInt(127)>::value, "BitInt:");
+static_assert(has_unique_object_representations<_BitInt(128)>::value, "BitInt:");
+
+
 namespace PR46209 {
   // Foo has both a trivial assignment operator and a non-trivial one.
   struct Foo {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2832,7 +2832,7 @@
   // All integrals and enums are unique.
   if (Ty->isIntegralOrEnumerationType()) {
 // Except _BitInt types that have padding bits.
-if (const auto *BIT = dyn_cast(Ty))
+if (const auto *BIT = Ty->getAs())
   return getTypeSize(BIT) == BIT->getNumBits();
 
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

2023-04-09 Thread kiwixz via Phabricator via cfe-commits

kiwixz added a comment.

I'm not sure how to test this, is there any check dependency we can rely on 
with hardcoded tests ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146520

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

[PATCH] D147901: [NFC][CLANG][API] Fix coverity remarks about large copies by values

2023-04-09 Thread Soumi Manna via Phabricator via cfe-commits

Manna added a comment.

x64 debian failures seem unrelated to my fix: 
https://lab.llvm.org/buildbot/#/builders/21/builds/67315/steps/6/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147901

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

[PATCH] D147901: [NFC][CLANG][API] Fix coverity remarks about large copies by values

2023-04-09 Thread Soumi Manna via Phabricator via cfe-commits

Manna created this revision.
Manna added reviewers: erichkeane, aaron.ballman.
Herald added a reviewer: ributzka.
Herald added a project: All.
Manna requested review of this revision.
Herald added a reviewer: dang.
Herald added a project: clang.

Reported by Coverity Static Analyzer Tool:

Big parameter passed by value (PASS_BY_VALUE)
Copying large values is inefficient, consider passing by reference; Low, 
medium, and high size thresholds for detection can be adjusted.

1. pass_by_value: Passing parameter Availabilities of type 
clang::extractapi::AvailabilitySet (size 320 bytes) by value, which exceeds the 
medium threshold of 256 bytes in "API.cpp" and "API.h" files.

This patch passes parameter as const AvailabilitySet   instead 
of AvailabilitySet Availabilities.

2. Inside "APIIgnoresList.h" file,  in 
clang::extractapi::APIIgnoresList::APIIgnoresList(llvm::SmallVector, llvm::SmallVector>, 13u>): A large function call 
parameter exceeding the medium threshold is passed by value.

pass_by_value: Passing parameter SymbolsToIgnore of type 
clang::extractapi::APIIgnoresList::SymbolNameList (size 268 bytes) by value, 
which exceeds the medium threshold of 256 bytes.

This patch passes parameter as const SymbolNameList  instead of 
SymbolNameList SymbolsToIgnore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147901

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/APIIgnoresList.h
  clang/lib/ExtractAPI/API.cpp

Index: clang/lib/ExtractAPI/API.cpp
===
--- clang/lib/ExtractAPI/API.cpp
+++ clang/lib/ExtractAPI/API.cpp
@@ -46,7 +46,7 @@
 
 GlobalVariableRecord *
 APISet::addGlobalVar(StringRef Name, StringRef USR, PresumedLoc Loc,
- AvailabilitySet Availabilities, LinkageInfo Linkage,
+ const AvailabilitySet , LinkageInfo Linkage,
  const DocComment , DeclarationFragments Fragments,
  DeclarationFragments SubHeading, bool IsFromSystemHeader) {
   return addTopLevelRecord(USRBasedLookupTable, GlobalVariables, USR, Name, Loc,
@@ -56,7 +56,7 @@
 
 GlobalFunctionRecord *APISet::addGlobalFunction(
 StringRef Name, StringRef USR, PresumedLoc Loc,
-AvailabilitySet Availabilities, LinkageInfo Linkage,
+const AvailabilitySet , LinkageInfo Linkage,
 const DocComment , DeclarationFragments Fragments,
 DeclarationFragments SubHeading, FunctionSignature Signature,
 bool IsFromSystemHeader) {
@@ -66,13 +66,11 @@
IsFromSystemHeader);
 }
 
-EnumConstantRecord *APISet::addEnumConstant(EnumRecord *Enum, StringRef Name,
-StringRef USR, PresumedLoc Loc,
-AvailabilitySet Availabilities,
-const DocComment ,
-DeclarationFragments Declaration,
-DeclarationFragments SubHeading,
-bool IsFromSystemHeader) {
+EnumConstantRecord *APISet::addEnumConstant(
+EnumRecord *Enum, StringRef Name, StringRef USR, PresumedLoc Loc,
+const AvailabilitySet , const DocComment ,
+DeclarationFragments Declaration, DeclarationFragments SubHeading,
+bool IsFromSystemHeader) {
   auto Record = std::make_unique(
   USR, Name, Loc, std::move(Availabilities), Comment, Declaration,
   SubHeading, IsFromSystemHeader);
@@ -83,7 +81,7 @@
 }
 
 EnumRecord *APISet::addEnum(StringRef Name, StringRef USR, PresumedLoc Loc,
-AvailabilitySet Availabilities,
+const AvailabilitySet ,
 const DocComment ,
 DeclarationFragments Declaration,
 DeclarationFragments SubHeading,
@@ -95,7 +93,7 @@
 
 StructFieldRecord *APISet::addStructField(StructRecord *Struct, StringRef Name,
   StringRef USR, PresumedLoc Loc,
-  AvailabilitySet Availabilities,
+  const AvailabilitySet ,
   const DocComment ,
   DeclarationFragments Declaration,
   DeclarationFragments SubHeading,
@@ -110,7 +108,7 @@
 }
 
 StructRecord *APISet::addStruct(StringRef Name, StringRef USR, PresumedLoc Loc,
-AvailabilitySet Availabilities,
+const AvailabilitySet ,
 const DocComment ,
 DeclarationFragments Declaration,
 DeclarationFragments SubHeading,
@@ -122,7 +120,7 @@
 
 ObjCCategoryRecord *APISet::addObjCCategory(
 StringRef Name, StringRef USR,

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file

2023-04-09 Thread Nathan James via Phabricator via cfe-commits

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

From an architectural point of view, is there any reason we don't change the 
backend to treat checks internally as a vector?

`clang-tools-extra/docs/clang-tidy/index.rst` also needs updating to specify 
this new behaviour.




Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:73
 for (const auto  : OptionMap)
-  Options.emplace_back(std::string(KeyValue.getKey()), 
KeyValue.getValue().Value);
+  Options.emplace_back(std::string(KeyValue.getKey()),
+   KeyValue.getValue().Value);

Any reason for this line changing, this formatting change looks unrelated.



Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:129-141
+// Special case for reading from YAML
+// Must support reading from both a string or a list
+Input  = reinterpret_cast(IO);
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();
+  yamlize(IO, *Checks.AsString, true, Ctx);

All this code can just be inlined into the function below and this function can 
just be removed



Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:132-133
+Input  = reinterpret_cast(IO);
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();





Comment at: clang-tools-extra/docs/ReleaseNotes.rst:304
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro 
between 
+  ` when using macro 
between
   namespace declarations could result incorrect fix.

This change is unrelated and should be pushed as an NFC commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

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

[PATCH] D93240: [clang-format] Add SpaceBeforeCaseColon option

2023-04-09 Thread sstwcw via Phabricator via cfe-commits

sstwcw added a comment.
Herald added a project: All.
Herald added reviewers: rymiel, owenpan.

A goto label isn't affected by this option.  Is it intentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93240

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

[PATCH] D147895: [clang-format] Handle Verilog assertions and loops

2023-04-09 Thread sstwcw via Phabricator via cfe-commits

sstwcw created this revision.
sstwcw added reviewers: HazardyKnusperkeks, MyDeveloperDay, owenpan, rymiel.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
sstwcw requested review of this revision.

Assert statements in Verilog can optionally have an else part.  We
handle them like for `if` statements, except that an `if` statement in
the else part of an `assert` statement doesn't get merged with the
`else` keyword.  Like this:

  assert (x)
$info();
  else
if (y)
  $info();
else if (z)
  $info();
else
  $info();

`foreach` and `repeat` are now handled like for or while loops.

We used the type `TT_ConditionLParen` to mark the condition part so
they are handled in the same way as the condition part of an `if`
statement.  When the code being formatted is not in Verilog, it is
only set for `if` statements, not loops.  It's because loop conditions
are currently handled slightly differently, and existing behavior is
not supposed to change.  We formatted all files ending in `.cpp` and
`.h` in the repository with and without this change.  It showed that
setting the type for `if` statements doesn't change existing behavior.

And we noticed that we forgot to make the program print the list of
tokens when the number is not correct in `TokenAnnotatorTest`.  It's
fixed now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147895

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -841,7 +841,7 @@
 "  [[nodiscard]] constexpr operator T() const { "
 "return number_zero_v; }\n"
 "};");
-  ASSERT_EQ(Tokens.size(), 44u);
+  ASSERT_EQ(Tokens.size(), 44u) << Tokens;
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[14], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_RequiresExpressionLBrace);
@@ -1605,7 +1605,7 @@
   // Test for block label colons.
   Tokens = Annotate("begin : x\n"
 "end : x");
-  ASSERT_EQ(Tokens.size(), 7u);
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::colon, TT_VerilogBlockLabelColon);
   EXPECT_TOKEN(Tokens[4], tok::colon, TT_VerilogBlockLabelColon);
   // Test that the dimension colon is annotated correctly.
@@ -1632,15 +1632,15 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GotoLabelColon);
   // Non-blocking assignments.
   Tokens = Annotate("a <= b;");
-  ASSERT_EQ(Tokens.size(), 5u);
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::lessequal, TT_BinaryOperator);
   EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
   Tokens = Annotate("if (a <= b) break;");
-  ASSERT_EQ(Tokens.size(), 9u);
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
   EXPECT_TOKEN_PRECEDENCE(Tokens[3], prec::Relational);
   Tokens = Annotate("a <= b <= a;");
-  ASSERT_EQ(Tokens.size(), 7u);
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::lessequal, TT_BinaryOperator);
   EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
   EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
@@ -1648,15 +1648,35 @@
 
   // Port lists in module instantiation.
   Tokens = Annotate("module_x instance_1(port_1), instance_2(port_2);");
-  ASSERT_EQ(Tokens.size(), 12u);
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
   EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_VerilogInstancePortLParen);
   Tokens = Annotate("module_x #(parameter) instance_1(port_1), "
 "instance_2(port_2);");
-  ASSERT_EQ(Tokens.size(), 16u);
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogInstancePortLParen);
   EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_VerilogInstancePortLParen);
+
+  // Condition parentheses.
+  Tokens = Annotate("assert (x);");
+  ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+  Tokens = Annotate("assert #0 (x);");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_ConditionLParen);
+  Tokens = Annotate("assert final (x);");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+  Tokens = Annotate("foreach (x[x]) continue;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+

[PATCH] D147894: [clang-format] SortIncludes documentation: remove contradiction in its description

2023-04-09 Thread Mike Matthews via Phabricator via cfe-commits

michael-g-matthews updated this revision to Diff 512042.
michael-g-matthews added a comment.

Amended commit to include change to `clang/include/clang/Format/Format.h`


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

https://reviews.llvm.org/D147894

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3568,11 +3568,6 @@
   };
 
   /// Controls if and how clang-format will sort ``#includes``.
-  /// If ``Never``, includes are never sorted.
-  /// If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  /// insensitive fashion.
-  /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  /// sensitive fashion.
   /// \version 3.8
   SortIncludesOptions SortIncludes;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4526,11 +4526,6 @@
 
 **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` 
:ref:`¶ `
   Controls if and how clang-format will sort ``#includes``.
-  If ``Never``, includes are never sorted.
-  If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  insensitive fashion.
-  If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  sensitive fashion.
 
   Possible values:
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3568,11 +3568,6 @@
   };
 
   /// Controls if and how clang-format will sort ``#includes``.
-  /// If ``Never``, includes are never sorted.
-  /// If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  /// insensitive fashion.
-  /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  /// sensitive fashion.
   /// \version 3.8
   SortIncludesOptions SortIncludes;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4526,11 +4526,6 @@
 
 **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` :ref:`Â¶ `
   Controls if and how clang-format will sort ``#includes``.
-  If ``Never``, includes are never sorted.
-  If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  insensitive fashion.
-  If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  sensitive fashion.
 
   Possible values:
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147894: [clang-format] SortIncludes documentation: remove contradiction in its description

2023-04-09 Thread Mike Matthews via Phabricator via cfe-commits

michael-g-matthews created this revision.
michael-g-matthews added reviewers: MyDeveloperDay, owenpan.
michael-g-matthews added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks.
Herald added a comment.
michael-g-matthews requested review of this revision.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to ClangFormatStyleOptions.rst but not a change 
to clang/include/clang/Format/Format.h

ClangFormatStyleOptions.rst is auto generated from Format.h via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


The style option SortIncludes contains a contradiction between its initial 
description and the list of possible values.

Currently the description incorrectly states:

  If CaseInsensitive, includes are sorted in an ASCIIbetical or case 
insensitive fashion. If CaseSensitive, includes are sorted in an alphabetical 
or case sensitive fashion

And the possible values section correctly states:

  SI_CaseSensitive (in configuration: CaseSensitive) Includes are sorted in an 
ASCIIbetical or case sensitive fashion.
  SI_CaseInsensitive (in configuration: CaseInsensitive) Includes are sorted in 
an alphabetical or case insensitive fashion

This patch shortens the initial description to read:

  Controls if and how clang-format will sort #includes.

This removes the contradiction and ensures there is only a singular explanation 
for how this style option works.


https://reviews.llvm.org/D147894

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4526,11 +4526,6 @@
 
 **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` 
:ref:`¶ `
   Controls if and how clang-format will sort ``#includes``.
-  If ``Never``, includes are never sorted.
-  If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  insensitive fashion.
-  If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  sensitive fashion.
 
   Possible values:
 


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4526,11 +4526,6 @@
 
 **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` :ref:`Â¶ `
   Controls if and how clang-format will sort ``#includes``.
-  If ``Never``, includes are never sorted.
-  If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
-  insensitive fashion.
-  If ``CaseSensitive``, includes are sorted in an alphabetical or case
-  sensitive fashion.
 
   Possible values:
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file

2023-04-09 Thread Eugene Zelenko via Phabricator via cfe-commits

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

Looks OK for me but will be good idea to wait for other reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

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

[PATCH] D147893: [clang-tidy] avoid colon in namespace cause false positve

2023-04-09 Thread Congcong Cai via Phabricator via cfe-commits

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

Refactor the Namespaces with NamespaceDecl[][].
First level stores non nested namepsace.
Second level stores nested namespace.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147893

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -214,6 +214,18 @@
 // CHECK-FIXES: namespace avoid_change_close_comment::inner {
 // CHECK-FIXES-NOT: } // namespace avoid_add_close_comment::inner
 
+namespace /*::*/ comment_colon_1 {
+void foo() {}
+} // namespace comment_colon_1
+// CHECK-FIXES: namespace /*::*/ comment_colon_1 {
+
+// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+namespace /*::*/ comment_colon_2 {
+namespace comment_colon_2 {
+void foo() {}
+} // namespace comment_colon_2
+} // namespace comment_colon_2
+
 int main() {
   n26::n27::n28::n29::n30::t();
 #ifdef IEXIST
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -15,6 +15,20 @@
 
 namespace clang::tidy::modernize {
 
+using NamespaceName = llvm::SmallString<40>;
+
+class NS : public llvm::SmallVector {
+public:
+  std::optional
+  getCleanedNamespaceFrontRange(const SourceManager ,
+const LangOptions ) const;
+  SourceRange getReplacedNamespaceFrontRange() const;
+  SourceRange getNamespaceBackRange(const SourceManager ,
+const LangOptions ) const;
+  SourceRange getDefaultNamespaceBackRange() const;
+  NamespaceName getName() const;
+};
+
 class ConcatNestedNamespacesCheck : public ClangTidyCheck {
 public:
   ConcatNestedNamespacesCheck(StringRef Name, ClangTidyContext *Context)
@@ -26,12 +40,10 @@
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
 
 private:
-  using NamespaceContextVec = llvm::SmallVector;
-  using NamespaceString = llvm::SmallString<40>;
+  using NamespaceContextVec = llvm::SmallVector;
 
   void reportDiagnostic(const SourceManager ,
 const LangOptions );
-  NamespaceString concatNamespaces();
   NamespaceContextVec Namespaces;
 };
 } // namespace clang::tidy::modernize
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -12,7 +12,6 @@
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceLocation.h"
-#include "llvm/ADT/STLExtras.h"
 #include 
 #include 
 
@@ -45,22 +44,23 @@
   return ChildNamespace && !unsupportedNamespace(*ChildNamespace);
 }
 
-static bool alreadyConcatenated(std::size_t NumCandidates,
-const SourceRange ,
-const SourceManager ,
-const LangOptions ) {
-  // FIXME: This logic breaks when there is a comment with ':'s in the middle.
-  return getRawStringRef(ReplacementRange, Sources, LangOpts).count(':') ==
- (NumCandidates - 1) * 2;
+template 
+static void concatNamespace(NamespaceName , R &,
+F &) {
+  for (auto const  : Range) {
+ConcatNameSpace.append(Stringify(V));
+if (V != Range.back())
+  ConcatNameSpace.append("::");
+  }
 }
 
-static std::optional
-getCleanedNamespaceFrontRange(const NamespaceDecl *ND, const SourceManager ,
-  const LangOptions ) {
+std::optional
+NS::getCleanedNamespaceFrontRange(const SourceManager ,
+  const LangOptions ) const {
   // Front from namespace tp '{'
   std::optional Tok =
   ::clang::tidy::utils::lexer::findNextTokenSkippingComments(
-  ND->getLocation(), SM, LangOpts);
+  back()->getLocation(), SM, LangOpts);
   if (!Tok)
 return std::nullopt;
   while (Tok->getKind() !=

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickly using clang-tidy to run a handful of checks. However it

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 512038.
carlosgalvezp retitled this revision from "[clang-tidy] Support introducing 
checks as a list in the config file

Specifying checks as a string is convenient for quickly using
clang-tidy to run a handful of checks. However it is not 
suitable for projects that have a long list of enabled or..." to "[clang-tidy] 
Support introducing checks as a list in the config file

Specifying checks as a string is convenient for quickly using 
clang-tidy to run a handful of checks. However it is not 
suitable for projects that have a long list of enabled...".
carlosgalvezp edited the summary of this revision.
carlosgalvezp added a comment.

Fix commit message title

Updating D147876: [clang-tidy] Support introducing checks as a list in the 
config file
==

Specifying checks as a string is convenient for quickly using 
clang-tidy to run a handful of checks. However it is not 
suitable for projects that have a long list of enabled...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
  clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
@@ -6,3 +6,15 @@
 // CHECK-SPACES-NEXT: hicpp-use-auto
 // CHECK-SPACES-NEXT: hicpp-use-emplace
 // CHECK-SPACES-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-dash --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-DASH
+// CHECK-LIST-DASH: Enabled checks:
+// CHECK-LIST-DASH-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-DASH-NEXT: hicpp-use-auto
+// CHECK-LIST-DASH-NEXT: hicpp-use-emplace
+// CHECK-LIST-DASH-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-bracket --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-BRACKET
+// CHECK-LIST-BRACKET: Enabled checks:
+// CHECK-LIST-BRACKET-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-auto
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-emplace
+// CHECK-LIST-BRACKET-EMPTY:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
@@ -0,0 +1,5 @@
+Checks:
+  - "-*"
+  - "hicpp-uppercase-literal-suffix"
+  - "hicpp-use-auto"
+  - "hicpp-use-emplace"
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
@@ -0,0 +1,6 @@
+Checks: [
+  "-*",
+  "hicpp-uppercase-literal-suffix",
+  "hicpp-use-auto",
+  "hicpp-use-emplace",
+]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -259,7 +259,9 @@
options. Example:
  CheckOptions:
some-check.SomeOption: 'some value'
-Checks   - Same as '--checks'.
+Checks   - Same as '--checks'. Additionally, the list of
+   globs can be specified as a list instead of a
+   string.
 ExtraArgs- Same as '--extra-args'.
 ExtraArgsBefore  - Same as '--extra-args-before'.
 FormatStyle  - Same as '--format-style'.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,9 @@
   `ImplementationFileExtensions`, replacing the check-local options of the
   same name.
 
+- Support specifying `Checks` as a YAML list in the `.clang-tidy` configuration
+  file.
+
 New checks
 ^^
 
@@ -298,7 +301,7 @@
   ``DISABLED_`` in the test suite name.
 
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickly using clang-tidy to run a handful of checks. However it i

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 512037.
carlosgalvezp retitled this revision from "[WIP][clang-tidy] Support specifying 
Checks as a list in the config file" to "[clang-tidy] Support introducing 
checks as a list in the config file

Specifying checks as a string is convenient for quickly using
clang-tidy to run a handful of checks. However it is not 
suitable for projects that have a long list of enabled or...".
carlosgalvezp edited the summary of this revision.
carlosgalvezp added a comment.

Update commit message

Updating D147876: [clang-tidy] Support introducing checks as a list in the 
config file
==

Specifying checks as a string is convenient for quickly using
clang-tidy to run a handful of checks. However it is not 
suitable for projects that have a long list of enabled or...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
  clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
@@ -6,3 +6,15 @@
 // CHECK-SPACES-NEXT: hicpp-use-auto
 // CHECK-SPACES-NEXT: hicpp-use-emplace
 // CHECK-SPACES-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-dash --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-DASH
+// CHECK-LIST-DASH: Enabled checks:
+// CHECK-LIST-DASH-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-DASH-NEXT: hicpp-use-auto
+// CHECK-LIST-DASH-NEXT: hicpp-use-emplace
+// CHECK-LIST-DASH-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-bracket --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-BRACKET
+// CHECK-LIST-BRACKET: Enabled checks:
+// CHECK-LIST-BRACKET-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-auto
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-emplace
+// CHECK-LIST-BRACKET-EMPTY:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
@@ -0,0 +1,5 @@
+Checks:
+  - "-*"
+  - "hicpp-uppercase-literal-suffix"
+  - "hicpp-use-auto"
+  - "hicpp-use-emplace"
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
@@ -0,0 +1,6 @@
+Checks: [
+  "-*",
+  "hicpp-uppercase-literal-suffix",
+  "hicpp-use-auto",
+  "hicpp-use-emplace",
+]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -259,7 +259,9 @@
options. Example:
  CheckOptions:
some-check.SomeOption: 'some value'
-Checks   - Same as '--checks'.
+Checks   - Same as '--checks'. Additionally, the list of
+   globs can be specified as a list instead of a
+   string.
 ExtraArgs- Same as '--extra-args'.
 ExtraArgsBefore  - Same as '--extra-args-before'.
 FormatStyle  - Same as '--format-style'.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,9 @@
   `ImplementationFileExtensions`, replacing the check-local options of the
   same name.
 
+- Support specifying `Checks` as a YAML list in the `.clang-tidy` configuration
+  file.
+
 New checks
 ^^
 
@@ -298,7 +301,7 @@
   ``DISABLED_`` in the test suite name.
 
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro between 
+  ` when using macro between
   namespace declarations could result incorrect fix.
 
 - Fixed a false positive in

[PATCH] D147876: [WIP][clang-tidy] Support specifying Checks as a list in the config file

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 512036.
carlosgalvezp added a comment.
Herald added a subscriber: arphaman.

- Simplify code with llvm:join.
- Add documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
  clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
@@ -6,3 +6,15 @@
 // CHECK-SPACES-NEXT: hicpp-use-auto
 // CHECK-SPACES-NEXT: hicpp-use-emplace
 // CHECK-SPACES-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-dash --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-DASH
+// CHECK-LIST-DASH: Enabled checks:
+// CHECK-LIST-DASH-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-DASH-NEXT: hicpp-use-auto
+// CHECK-LIST-DASH-NEXT: hicpp-use-emplace
+// CHECK-LIST-DASH-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-bracket --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-BRACKET
+// CHECK-LIST-BRACKET: Enabled checks:
+// CHECK-LIST-BRACKET-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-auto
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-emplace
+// CHECK-LIST-BRACKET-EMPTY:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
@@ -0,0 +1,5 @@
+Checks:
+  - "-*"
+  - "hicpp-uppercase-literal-suffix"
+  - "hicpp-use-auto"
+  - "hicpp-use-emplace"
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
@@ -0,0 +1,6 @@
+Checks: [
+  "-*",
+  "hicpp-uppercase-literal-suffix",
+  "hicpp-use-auto",
+  "hicpp-use-emplace",
+]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -259,7 +259,9 @@
options. Example:
  CheckOptions:
some-check.SomeOption: 'some value'
-Checks   - Same as '--checks'.
+Checks   - Same as '--checks'. Additionally, the list of
+   globs can be specified as a list instead of a
+   string.
 ExtraArgs- Same as '--extra-args'.
 ExtraArgsBefore  - Same as '--extra-args-before'.
 FormatStyle  - Same as '--format-style'.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,9 @@
   `ImplementationFileExtensions`, replacing the check-local options of the
   same name.
 
+- Support specifying `Checks` as a YAML list in the `.clang-tidy` configuration
+  file.
+
 New checks
 ^^
 
@@ -298,7 +301,7 @@
   ``DISABLED_`` in the test suite name.
 
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro between 
+  ` when using macro between
   namespace declarations could result incorrect fix.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -52,7 +52,9 @@
  options. Example:
CheckOptions:
  some-check.SomeOption: 'some value'
-  Checks   - Same as '--checks'.
+  Checks   - Same as '--checks'. Additionally, the list of
+ globs can be specified as a list instead of a
+ string.
   ExtraArgs

[clang] 3c83480 - [clang][AST] Fix `-Wuninitialized`. NFC

2023-04-09 Thread Michael Liao via cfe-commits


Author: Michael Liao
Date: 2023-04-09T15:58:10-04:00
New Revision: 3c83480ae95dde9b5d45b6fd7cdb1c64332531d7

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

LOG: [clang][AST] Fix `-Wuninitialized`. NFC

- Adjust the declaration order as non-static member are initialized in
  order of declaration in the class definition.

Added: 


Modified: 
clang/lib/AST/MicrosoftMangle.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftMangle.cpp 
b/clang/lib/AST/MicrosoftMangle.cpp
index d8c837bcade02..e0fd8abe5e3b8 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -327,8 +327,8 @@ class MicrosoftCXXNameMangler {
 
   typedef llvm::DenseMap TemplateArgStringMap;
   TemplateArgStringMap TemplateArgStrings;
-  llvm::StringSaver TemplateArgStringStorage;
   llvm::BumpPtrAllocator TemplateArgStringStorageAlloc;
+  llvm::StringSaver TemplateArgStringStorage;
 
   typedef std::set> PassObjectSizeArgsSet;
   PassObjectSizeArgsSet PassObjectSizeArgs;



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

[PATCH] D147889: [clang-tidy] Improve bugprone-branch-clone with support for fallthrough attribute

2023-04-09 Thread 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.

Ignore duplicated switch cases with [[fallthrough]] attribute to reduce false 
positives.

Fixes: #47588


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147889

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- -- -std=c++17
+
+void handle(int);
+
+void testSwitchFallthroughAttribute(int value) {
+  switch(value) {
+case 1: [[fallthrough]];
+case 2: [[fallthrough]];
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchFallthroughAttributeAndBraces(int value) {
+  switch(value) {
+case 1: { [[fallthrough]]; }
+case 2: { [[fallthrough]]; }
+case 3: {
+  handle(value);
+  break;
+}
+default: {
+  break;
+}
+  }
+}
+
+void testSwitchWithFallthroughAttributeAndCode(int value) {
+  switch(value) {
+case 1: value += 1; [[fallthrough]];
+case 2: value += 1; [[fallthrough]];
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchWithFallthroughAndCode(int value) {
+  switch(value) {
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: switch has 2 consecutive identical branches [bugprone-branch-clone]
+case 1: value += 1;
+case 2: value += 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:23: note: last of these clones ends here
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchFallthroughAttributeIntoDefault(int value) {
+  switch(value) {
+case 1: [[fallthrough]];
+case 2: [[fallthrough]];
+default:
+  handle(value);
+  break;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
@@ -77,6 +77,8 @@
 we want to preserve that ``'a'`` is before ``'b'`` and ``default:`` is the last
 branch.
 
+Switch cases marked with the ``[[fallthrough]]`` attribute are ignored.
+
 Finally, the check also examines conditional operators and reports code like:
 
 .. code-block:: c++
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -165,6 +165,11 @@
 
 Changes in existing checks
 ^^
+
+- Fixed false-positive in :doc:`bugprone-branch-clone
+  ` check by ignoring duplicated
+  switch cases marked with the ``[[fallthrough]]`` attribute.
+
 - Improved :doc:`readability-redundant-string-cstr
   ` check to recognise
   unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "BranchCloneCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/CloneDetection.h"
 #include "clang/Lex/Lexer.h"
@@ -45,8 +46,8 @@
 /// Determines if the bodies of two branches in a switch statements are Type I
 /// clones of each other. This function only examines the body of the branch
 /// and ignores the `case X:` or `default:` at the start of the branch.
-static bool areSwitchBranchesIdentical(const SwitchBranch LHS,
-   const SwitchBranch RHS,
+static bool areSwitchBranchesIdentical(const SwitchBranch ,
+   const SwitchBranch ,
const ASTContext ) {
   if (LHS.size() != RHS.size())
 return false;
@@ -64,6 +65,55 @@
   return true;
 }
 
+static bool isFallthroughSwitchBranch(const SwitchBranch ) {
+  struct SwitchCaseVisitor : RecursiveASTVisitor {
+using RecursiveASTVisitor::DataRecursionQueue;
+
+bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+  // Ignore lambdas

[PATCH] D147888: Update declaration message of extern linkage

2023-04-09 Thread Krishna Narayanan via Phabricator via cfe-commits

Krishna-13-cyber created this revision.
Krishna-13-cyber added reviewers: tbaeder, cjdb, aaron.ballman.
Herald added a project: All.
Krishna-13-cyber requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch aims to improve the diagnostic to much better precision of 
indicating extern declaration being followed after static declaration rather 
than just generic non-static declaration.

**Example Testcase:** https://godbolt.org/z/zMTda6MGG


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147888

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/extern_static.cpp


Index: clang/test/SemaCXX/extern_static.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/extern_static.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wabstract-vbase-init
+void f(void)
+{
+static int x; // expected-note {{previous definition is here}}
+extern int x; // expected-error {{extern declaration of 'x' follows static 
declaration}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4637,6 +4637,15 @@
   //   the prior declaration. If no prior declaration is visible, or
   //   if the prior declaration specifies no linkage, then the
   //   identifier has external linkage.
+
+  if (New->hasExternalStorage() &&
+  Old->getCanonicalDecl()->getStorageClass() == SC_Static &&
+  Old->isLocalVarDeclOrParm()) {
+Diag(New->getLocation(), diag::err_extern_static) << New->getDeclName();
+Diag(OldLocation, PrevDiag);
+return New->setInvalidDecl();
+  }
+
   if (New->hasExternalStorage() && Old->hasLinkage())
 /* Okay */;
   else if (New->getCanonicalDecl()->getStorageClass() != SC_Static &&
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5799,6 +5799,8 @@
   InGroup;
 def err_non_static_static : Error<
   "non-static declaration of %0 follows static declaration">;
+def err_extern_static : Error<
+  "extern declaration of %0 follows static declaration">;
 def err_extern_non_extern : Error<
   "extern declaration of %0 follows non-extern declaration">;
 def err_non_extern_extern : Error<


Index: clang/test/SemaCXX/extern_static.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/extern_static.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wabstract-vbase-init
+void f(void)
+{
+static int x; // expected-note {{previous definition is here}}
+extern int x; // expected-error {{extern declaration of 'x' follows static declaration}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4637,6 +4637,15 @@
   //   the prior declaration. If no prior declaration is visible, or
   //   if the prior declaration specifies no linkage, then the
   //   identifier has external linkage.
+
+  if (New->hasExternalStorage() &&
+  Old->getCanonicalDecl()->getStorageClass() == SC_Static &&
+  Old->isLocalVarDeclOrParm()) {
+Diag(New->getLocation(), diag::err_extern_static) << New->getDeclName();
+Diag(OldLocation, PrevDiag);
+return New->setInvalidDecl();
+  }
+
   if (New->hasExternalStorage() && Old->hasLinkage())
 /* Okay */;
   else if (New->getCanonicalDecl()->getStorageClass() != SC_Static &&
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5799,6 +5799,8 @@
   InGroup;
 def err_non_static_static : Error<
   "non-static declaration of %0 follows static declaration">;
+def err_extern_static : Error<
+  "extern declaration of %0 follows static declaration">;
 def err_extern_non_extern : Error<
   "extern declaration of %0 follows non-extern declaration">;
 def err_non_extern_extern : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

2023-04-09 Thread Mark de Wever via Phabricator via cfe-commits

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



Comment at: 
libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/parse.pass.cpp:49
+  std::same_as auto it = 
formatter.parse(parse_ctx);
+  assert(it == fmt.end() - (!fmt.empty() && fmt.back() == '}'));
+}

ldionne wrote:
> This seems a bit clever to me. Can we pass the expected end position as a 
> parameter to the function explicitly? So this would look like `it == 
> fmt.begin() + offset`. If you have this pattern in other places, I'm OK to 
> land the patch as-is but we could have a simple patch to change all of them.
See D147885


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144331

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

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

2023-04-09 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 512024.
tbaeder edited the summary of this revision.

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

https://reviews.llvm.org/D147875

Files:
  clang/include/clang/Frontend/TextDiagnostic.h
  clang/lib/Frontend/TextDiagnostic.cpp

Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -1121,6 +1121,24 @@
   return FixItInsertionLine;
 }
 
+static unsigned getNumDisplayWidth(unsigned N) {
+  if (N < 10)
+return 1;
+  if (N < 100)
+return 2;
+  if (N < 1'000)
+return 3;
+  if (N < 10'000)
+return 4;
+  if (N < 100'000)
+return 5;
+  if (N < 1'000'000)
+return 6;
+  if (N < 10'000'000)
+return 7;
+  return 0;
+}
+
 /// Emit a code snippet and caret line.
 ///
 /// This routine emits a single line's code snippet and caret line..
@@ -1174,6 +1192,10 @@
 if (auto OptionalRange = findLinesForRange(*I, FID, SM))
   Lines = maybeAddRange(Lines, *OptionalRange, MaxLines);
 
+  unsigned MaxLineNoDisplayWidth = std::max(getNumDisplayWidth(Lines.first),
+getNumDisplayWidth(Lines.second));
+  unsigned HighlightIndent = MaxLineNoDisplayWidth + 1;
+
   for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1; ++LineNo) {
 const char *BufStart = BufData.data();
 const char *BufEnd = BufStart + BufData.size();
@@ -1249,9 +1271,13 @@
   CaretLine.erase(CaretLine.end() - 1);
 
 // Emit what we have computed.
-emitSnippet(SourceLine);
+emitSnippet(SourceLine, MaxLineNoDisplayWidth, LineNo);
 
 if (!CaretLine.empty()) {
+  // Indent for line numbers
+  for (unsigned I = 0; I != HighlightIndent; ++I)
+OS << ' ';
+  OS << " | ";
   if (DiagOpts->ShowColors)
 OS.changeColor(caretColor, true);
   OS << CaretLine << '\n';
@@ -1260,6 +1286,9 @@
 }
 
 if (!FixItInsertionLine.empty()) {
+  for (unsigned I = 0; I != HighlightIndent; ++I)
+OS << ' ';
+  OS << " | ";
   if (DiagOpts->ShowColors)
 // Print fixit line in color
 OS.changeColor(fixitColor, false);
@@ -1275,7 +1304,8 @@
   emitParseableFixits(Hints, SM);
 }
 
-void TextDiagnostic::emitSnippet(StringRef line) {
+void TextDiagnostic::emitSnippet(StringRef line, unsigned MaxLineNoDisplayWidth,
+ unsigned LineNo) {
   if (line.empty())
 return;
 
@@ -1284,6 +1314,16 @@
   std::string to_print;
   bool print_reversed = false;
 
+  // Emit line number.
+  {
+unsigned LineNoDisplayWidth = getNumDisplayWidth(LineNo);
+OS << ' ';
+for (unsigned I = LineNoDisplayWidth; I < MaxLineNoDisplayWidth; ++I)
+  OS << ' ';
+OS << LineNo;
+OS << " | ";
+  }
+
   while (i,bool> res
 = printableTextForNextCharacter(line, , DiagOpts->TabStop);
Index: clang/include/clang/Frontend/TextDiagnostic.h
===
--- clang/include/clang/Frontend/TextDiagnostic.h
+++ clang/include/clang/Frontend/TextDiagnostic.h
@@ -103,7 +103,8 @@
SmallVectorImpl ,
ArrayRef Hints);
 
-  void emitSnippet(StringRef SourceLine);
+  void emitSnippet(StringRef SourceLine, unsigned MaxLineNoDisplayWidth,
+   unsigned LineNo);
 
   void emitParseableFixits(ArrayRef Hints, const SourceManager );
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:74
+"rend","crend",  "size"};
+static const std::set StdNames{
+"std::begin", "std::cbegin", "std::rbegin", "std::crbegin", "std::end",

consider: #include "llvm/ADT/StringSet.h" & llvm::StringSet



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:302
+ hasType(RecordWithBeginEnd,
+  callExpr(argumentCountIs(1), callee(functionDecl(hasAnyName("size"))),
+   usesADL()),

hasAnyName -> hasName, there is only 1 argument.



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:305
+  callExpr(argumentCountIs(1),
+   callee(functionDecl(hasAnyName("::std::size"));
 

hasAnyName -> hasName, there is only 1 argument.



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:346
+// The returned Expr* is nullptr if any of the assumptions are not met.
+static std::tuple
+getContainerExpr(const Expr *Call) {

change this tuple into some struct, it's hard to find out what those arguments 
are...



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:354
+CallKind = ICK_Member;
+if (const auto *Member = dyn_cast(TheCall->getCallee())) {
+  return std::make_tuple(TheCall->getImplicitObjectArgument(),

STYLE: no need for `if (xx) { y1 } else { y2 }`, just `if (xx) y1 else y2;` 
should be sufficient



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:363
+}
+  } else if (const auto *TheCall = dyn_cast_or_null(Dug)) {
+if (TheCall->getNumArgs() != 1)

STYLE: this else is not needed, there are returns in previous anyway
simply if will be sufficient



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:368
+if (TheCall->usesADL()) {
+  if (!ADLNames.count(TheCall->getDirectCallee()->getName()))
+return {};

count -> contains



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:372
+} else {
+  if (!StdNames.count(
+  TheCall->getDirectCallee()->getQualifiedNameAsString()))

count -> contains



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:390
 /// is called via . or ->.
-static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
-bool *IsArrow, bool IsReverse) 
{
+static std::tuple
+getContainerFromBeginEndCall(const Expr *Init, bool IsBegin, bool *IsArrow,

no need for tuple for 2 argments, pair would be sufficient.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:201-204
+- Improved :doc:`modernize-loop-convert
+  ` to refactor container based
+  for loops that initialize iterators with free function calls to ``begin``,
+  ``end``, or ``size``.

maybe just "Enhanced XYZ check to support for-loops with iterators initialized 
by free function calls like begin, end, or size."
I got lost on this part "to refactor container based for loops"



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:477
+
+  for (S::const_iterator It = cbegin(Ss), E = cend(Ss); It != E; ++It) {
+printf("s4 has value %d\n", (*It).X);

what if begin,end is missing, and there is only cbegin, cend then it will fail 
with:
"error: invalid range expression of type 'const A'; no viable 'begin' function 
available|




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:481
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s4 has value %d\n", It.X);

shouldnt it be `const& It` ?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:737
+  // CHECK-FIXES: for (auto & Val : Vals)
+  // CHECK-FIXES-NEXT: Sum += Val.X;
 }

add some tests for C++20 with rbegin, rend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-09 Thread Douglas Chen via Phabricator via cfe-commits

dougpuob updated this revision to Diff 512021.
dougpuob added a comment.

- Improved suggestions in code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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-hungarian-notation-c-language.c
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *custpcustu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *custpcustucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **custpcustpcustucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* custpcustvVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
@@ -283,6 +283,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -240,6 +240,10 @@
   behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix`
   option to `i` instead of using `EnumConstantHungarianPrefix`.
 
+- Fixed a hungarian notation issue in :doc:`readability-identifier-naming
+  ` which failed to indicate
+  the number of asterisks.
+
 - Fixed a false positive in

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

2023-04-09 Thread Chris Cotter via Phabricator via cfe-commits

ccotter added a comment.

bump - any other feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+

junaire wrote:
> v.g.vassilev wrote:
> > junaire wrote:
> > > v.g.vassilev wrote:
> > > > This can probably go in the RuntimeInterfaceBuilder class.
> > > We need it. See:
> > > 
> > > ```
> > > class RuntimeInterfaceBuilder
> > > : public TypeVisitor > > Interpreter::InterfaceKind> {
> > >...
> > > }
> > > ```
> > Can't this be an enum which is file local?
> You can't put this enum inside RuntimeInterfaceBuilder because its 
> declaration needs it. If you do so, then the above line will report an error 
> since you use the enum before defined it.
Here is what I had in mind: https://godbolt.org/z/av54aMbG6


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D147857: [clang-tidy] fix false positve for namespace with attrs in modernize-concat-nested-namespaces

2023-04-09 Thread Congcong Cai via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbdf7fd8297bc: [clang-tidy] fix false positve for namespace 
with attrs in modernize-concat… (authored by HerrCai0907).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147857

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -21,11 +21,17 @@
 } // namespace n2
 
 namespace n5 {
-inline namespace n6 {
+inline namespace inline_ns {
 void t();
-}
+} // namespace inline_ns
 } // namespace n5
 
+namespace n6 {
+namespace [[deprecated]] attr_ns {
+void t();
+} // namespace attr_ns
+} // namespace n6
+
 namespace n7 {
 void t();
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,9 +301,10 @@
   ` 
when using
   ``DISABLED_`` in the test suite name.
 
-- Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro 
between
-  namespace declarations could result incorrect fix.
+- Improved :doc:`modernize-concat-nested-namespaces
+  ` to fix incorrect 
fixes when 
+  using macro between namespace declarations and false positive when using 
namespace 
+  with attributes.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   ` when warning would be
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -31,8 +31,9 @@
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool anonymousOrInlineNamespace(const NamespaceDecl ) {
-  return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+static bool unsupportedNamespace(const NamespaceDecl ) {
+  return ND.isAnonymousNamespace() || ND.isInlineNamespace() ||
+ !ND.attrs().empty();
 }
 
 static bool singleNamedNamespaceChild(const NamespaceDecl ) {
@@ -41,7 +42,7 @@
 return false;
 
   const auto *ChildNamespace = dyn_cast(*Decls.begin());
-  return ChildNamespace && !anonymousOrInlineNamespace(*ChildNamespace);
+  return ChildNamespace && !unsupportedNamespace(*ChildNamespace);
 }
 
 static bool alreadyConcatenated(std::size_t NumCandidates,
@@ -166,7 +167,7 @@
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
 return;
 
-  if (anonymousOrInlineNamespace(ND))
+  if (unsupportedNamespace(ND))
 return;
 
   Namespaces.push_back();


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -21,11 +21,17 @@
 } // namespace n2
 
 namespace n5 {
-inline namespace n6 {
+inline namespace inline_ns {
 void t();
-}
+} // namespace inline_ns
 } // namespace n5
 
+namespace n6 {
+namespace [[deprecated]] attr_ns {
+void t();
+} // namespace attr_ns
+} // namespace n6
+
 namespace n7 {
 void t();
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,9 +301,10 @@
   ` when using
   ``DISABLED_`` in the test suite name.
 
-- Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro between
-  namespace declarations could result incorrect fix.
+- Improved :doc:`modernize-concat-nested-namespaces
+  ` to fix incorrect fixes when 
+  using macro between namespace declarations and false positive when using namespace 
+  with attributes.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   ` when warning would be
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -31,8 +31,9 @@
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool

[clang-tools-extra] bdf7fd8 - [clang-tidy] fix false positve for namespace with attrs in modernize-concat-nested-namespaces

2023-04-09 Thread Congcong Cai via cfe-commits


Author: Congcong Cai
Date: 2023-04-09T16:23:29+02:00
New Revision: bdf7fd8297bcbcddc9c184a40c954c1f1b0b8340

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

LOG: [clang-tidy] fix false positve for namespace with attrs in 
modernize-concat-nested-namespaces

Fixed https://github.com/llvm/llvm-project/issues/57530
Add pre check to avoid false positive for namespace with attributes like
```
namespace [[deprecated]] ns {}
```

Reviewed By: PiotrZSL

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
index e18dcc2b3e591..a0b004f851ee0 100644
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -31,8 +31,9 @@ static StringRef getRawStringRef(const SourceRange ,
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool anonymousOrInlineNamespace(const NamespaceDecl ) {
-  return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+static bool unsupportedNamespace(const NamespaceDecl ) {
+  return ND.isAnonymousNamespace() || ND.isInlineNamespace() ||
+ !ND.attrs().empty();
 }
 
 static bool singleNamedNamespaceChild(const NamespaceDecl ) {
@@ -41,7 +42,7 @@ static bool singleNamedNamespaceChild(const NamespaceDecl 
) {
 return false;
 
   const auto *ChildNamespace = dyn_cast(*Decls.begin());
-  return ChildNamespace && !anonymousOrInlineNamespace(*ChildNamespace);
+  return ChildNamespace && !unsupportedNamespace(*ChildNamespace);
 }
 
 static bool alreadyConcatenated(std::size_t NumCandidates,
@@ -166,7 +167,7 @@ void ConcatNestedNamespacesCheck::check(
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
 return;
 
-  if (anonymousOrInlineNamespace(ND))
+  if (unsupportedNamespace(ND))
 return;
 
   Namespaces.push_back();

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 1ab7f4c79c7df..35cdcf7b207f2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,9 +301,10 @@ Changes in existing checks
   ` 
when using
   ``DISABLED_`` in the test suite name.
 
-- Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro 
between
-  namespace declarations could result incorrect fix.
+- Improved :doc:`modernize-concat-nested-namespaces
+  ` to fix incorrect 
fixes when 
+  using macro between namespace declarations and false positive when using 
namespace 
+  with attributes.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   ` when warning would be

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
index 7728ada6a18db..13d1f2c37d983 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -21,11 +21,17 @@ void t();
 } // namespace n2
 
 namespace n5 {
-inline namespace n6 {
+inline namespace inline_ns {
 void t();
-}
+} // namespace inline_ns
 } // namespace n5
 
+namespace n6 {
+namespace [[deprecated]] attr_ns {
+void t();
+} // namespace attr_ns
+} // namespace n6
+
 namespace n7 {
 void t();
 



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

[PATCH] D147857: [clang-tidy] fix false positve for namespace with attrs in modernize-concat-nested-namespaces

2023-04-09 Thread Congcong Cai via Phabricator via cfe-commits

HerrCai0907 updated this revision to Diff 512004.
HerrCai0907 edited the summary of this revision.
HerrCai0907 added a comment.

merge items in release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147857

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -21,11 +21,17 @@
 } // namespace n2
 
 namespace n5 {
-inline namespace n6 {
+inline namespace inline_ns {
 void t();
-}
+} // namespace inline_ns
 } // namespace n5
 
+namespace n6 {
+namespace [[deprecated]] attr_ns {
+void t();
+} // namespace attr_ns
+} // namespace n6
+
 namespace n7 {
 void t();
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,9 +301,10 @@
   ` 
when using
   ``DISABLED_`` in the test suite name.
 
-- Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro 
between
-  namespace declarations could result incorrect fix.
+- Improved :doc:`modernize-concat-nested-namespaces
+  ` to fix incorrect 
fixes when 
+  using macro between namespace declarations and false positive when using 
namespace 
+  with attributes.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   ` when warning would be
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -31,8 +31,9 @@
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool anonymousOrInlineNamespace(const NamespaceDecl ) {
-  return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+static bool unsupportedNamespace(const NamespaceDecl ) {
+  return ND.isAnonymousNamespace() || ND.isInlineNamespace() ||
+ !ND.attrs().empty();
 }
 
 static bool singleNamedNamespaceChild(const NamespaceDecl ) {
@@ -41,7 +42,7 @@
 return false;
 
   const auto *ChildNamespace = dyn_cast(*Decls.begin());
-  return ChildNamespace && !anonymousOrInlineNamespace(*ChildNamespace);
+  return ChildNamespace && !unsupportedNamespace(*ChildNamespace);
 }
 
 static bool alreadyConcatenated(std::size_t NumCandidates,
@@ -166,7 +167,7 @@
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
 return;
 
-  if (anonymousOrInlineNamespace(ND))
+  if (unsupportedNamespace(ND))
 return;
 
   Namespaces.push_back();


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -21,11 +21,17 @@
 } // namespace n2
 
 namespace n5 {
-inline namespace n6 {
+inline namespace inline_ns {
 void t();
-}
+} // namespace inline_ns
 } // namespace n5
 
+namespace n6 {
+namespace [[deprecated]] attr_ns {
+void t();
+} // namespace attr_ns
+} // namespace n6
+
 namespace n7 {
 void t();
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,9 +301,10 @@
   ` when using
   ``DISABLED_`` in the test suite name.
 
-- Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  ` when using macro between
-  namespace declarations could result incorrect fix.
+- Improved :doc:`modernize-concat-nested-namespaces
+  ` to fix incorrect fixes when 
+  using macro between namespace declarations and false positive when using namespace 
+  with attributes.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   ` when warning would be
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -31,8 +31,9 @@
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool anonymousOrInlineNamespace(const NamespaceDecl ) {
-  return ND.isAnonymousNamespace() || ND.isInlineNamespace();
+static

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+

v.g.vassilev wrote:
> junaire wrote:
> > v.g.vassilev wrote:
> > > This can probably go in the RuntimeInterfaceBuilder class.
> > We need it. See:
> > 
> > ```
> > class RuntimeInterfaceBuilder
> > : public TypeVisitor > Interpreter::InterfaceKind> {
> >...
> > }
> > ```
> Can't this be an enum which is file local?
You can't put this enum inside RuntimeInterfaceBuilder because its declaration 
needs it. If you do so, then the above line will report an error since you use 
the enum before defined it.



Comment at: clang/lib/Interpreter/Interpreter.cpp:350
   std::list  = IncrParser->getPTUs();
-  if (N > PTUs.size())
+  if (N + InitPTUSize > PTUs.size())
 return llvm::make_error("Operation failed. "

v.g.vassilev wrote:
> junaire wrote:
> > v.g.vassilev wrote:
> > > v.g.vassilev wrote:
> > > > I'd propose `IncrParser->getPTUs()` to return the list starting from 
> > > > `InitPTUSize`. That should solve the issue you see.
> > > Ping.
> > I'm uncertain about how to do this. Can you elaborate?
> The `std::list PTUs;` in `IncrementalParser.h` stores 
> the all PTUs that we saw, including the Interpreter builtins which contain 
> declarations required by the Interpreter to run. I'd propose  to make `PTUs` 
> of type `std::vector` and `getPTUs()` to return an `ArrayRef` starting from 
> `PTUs.begin() + N` where `N` is the number of builtins that we must not undo.
OK, return ArrayRef sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+

junaire wrote:
> v.g.vassilev wrote:
> > This can probably go in the RuntimeInterfaceBuilder class.
> We need it. See:
> 
> ```
> class RuntimeInterfaceBuilder
> : public TypeVisitor 
> {
>...
> }
> ```
Can't this be an enum which is file local?



Comment at: clang/lib/Interpreter/Interpreter.cpp:350
   std::list  = IncrParser->getPTUs();
-  if (N > PTUs.size())
+  if (N + InitPTUSize > PTUs.size())
 return llvm::make_error("Operation failed. "

junaire wrote:
> v.g.vassilev wrote:
> > v.g.vassilev wrote:
> > > I'd propose `IncrParser->getPTUs()` to return the list starting from 
> > > `InitPTUSize`. That should solve the issue you see.
> > Ping.
> I'm uncertain about how to do this. Can you elaborate?
The `std::list PTUs;` in `IncrementalParser.h` stores 
the all PTUs that we saw, including the Interpreter builtins which contain 
declarations required by the Interpreter to run. I'd propose  to make `PTUs` of 
type `std::vector` and `getPTUs()` to return an `ArrayRef` starting from 
`PTUs.begin() + N` where `N` is the number of builtins that we must not undo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 511999.
junaire added a comment.

Address some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/Mangle.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Interpreter/Value.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
@@ -33,6 +34,10 @@
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
 #endif
 
+int Global = 42;
+int getGlobal() { return Global; }
+void setGlobal(int val) { Global = val; }
+
 namespace {
 using Args = std::vector;
 static std::unique_ptr
@@ -276,8 +281,7 @@
   std::vector Args = {"-fno-delayed-template-parsing"};
   std::unique_ptr Interp = createInterpreter(Args);
 
-  llvm::cantFail(Interp->Parse("void* operator new(__SIZE_TYPE__, void* __p);"
-   "extern \"C\" int printf(const char*,...);"
+  llvm::cantFail(Interp->Parse("extern \"C\" int printf(const char*,...);"
"class A {};"
"struct B {"
"  template"
@@ -314,4 +318,55 @@
   free(NewA);
 }
 
+TEST(InterpreterTest, Value) {
+  std::unique_ptr Interp = createInterpreter();
+
+  Value V1;
+  llvm::cantFail(Interp->ParseAndExecute("int x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("x", ));
+  EXPECT_TRUE(V1.isValid());
+  EXPECT_EQ(V1.getInt(), 42);
+  EXPECT_TRUE(V1.getType()->isIntegerType());
+  EXPECT_EQ(V1.getKind(), Value::K_Int);
+  EXPECT_FALSE(V1.isManuallyAlloc());
+  EXPECT_FALSE(V1.isPointerOrObjectType());
+
+  Value V2;
+  llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
+  llvm::cantFail(Interp->ParseAndExecute("y", ));
+  EXPECT_TRUE(V2.isValid());
+  EXPECT_EQ(V2.getDouble(), 3.14);
+  EXPECT_TRUE(V2.getType()->isFloatingType());
+  EXPECT_EQ(V2.getKind(), Value::K_Double);
+  EXPECT_FALSE(V2.isManuallyAlloc());
+  EXPECT_FALSE(V2.isPointerOrObjectType());
+
+  Value V3;
+  llvm::cantFail(Interp->ParseAndExecute(
+  "struct S { int* p; S() { p = new int(42); } ~S() { delete p; }};"));
+  llvm::cantFail(Interp->ParseAndExecute("S{}", ));
+  EXPECT_TRUE(V3.isValid());
+  EXPECT_TRUE(V3.getType()->isRecordType());
+  EXPECT_EQ(V3.getKind(), Value::K_PtrOrObj);
+  EXPECT_TRUE(V3.isManuallyAlloc());
+  EXPECT_TRUE(V3.isPointerOrObjectType());
+
+  Value V4;
+  llvm::cantFail(Interp->ParseAndExecute("int getGlobal();"));
+  llvm::cantFail(Interp->ParseAndExecute("void setGlobal(int);"));
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V4.getInt(), 42);
+  EXPECT_TRUE(V4.getType()->isIntegerType());
+
+  Value V5;
+  // Change the global from the compiled code.
+  setGlobal(43);
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V5.getInt(), 43);
+  EXPECT_TRUE(V5.getType()->isIntegerType());
+
+  // Change the global from the interpreted code.
+  llvm::cantFail(Interp->ParseAndExecute("setGlobal(44);"));
+  EXPECT_EQ(getGlobal(), 44);
+}
 } // end anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -22,3 +22,5 @@
 if(NOT WIN32)
   add_subdirectory(ExceptionTests)
 endif()
+
+export_executable_symbols(ClangReplInterpreterTests)
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Parse/Parser.cpp

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+

v.g.vassilev wrote:
> This can probably go in the RuntimeInterfaceBuilder class.
We need it. See:

```
class RuntimeInterfaceBuilder
: public TypeVisitor {
   ...
}
```



Comment at: clang/lib/Interpreter/Interpreter.cpp:350
   std::list  = IncrParser->getPTUs();
-  if (N > PTUs.size())
+  if (N + InitPTUSize > PTUs.size())
 return llvm::make_error("Operation failed. "

v.g.vassilev wrote:
> v.g.vassilev wrote:
> > I'd propose `IncrParser->getPTUs()` to return the list starting from 
> > `InitPTUSize`. That should solve the issue you see.
> Ping.
I'm uncertain about how to do this. Can you elaborate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added inline comments.



Comment at: clang/lib/Interpreter/Interpreter.cpp:383
+
+static std::string MangleName(ASTContext , GlobalDecl GD) {
+  std::unique_ptr MangleC(C.createMangleContext());

I suspect we could use `IncrementalParser::GetMangledName` which in turn uses 
codegen's caching. In that case we could avoid having a dtor cache altogether 
and move all of the logic inside the RuntimeInterfaceBuilder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added inline comments.



Comment at: clang/include/clang/AST/Decl.h:4345
+  }
+  bool isValuePrinting() const { return MissingSemi; }
+  void setValuePrinting(bool Missing = true) { MissingSemi = Missing; }

We should rename these to `isMissingSemi` or similar.



Comment at: clang/include/clang/Interpreter/Interpreter.h:37
 
+class Value;
 class CompilerInstance;

This seems to be redundant.



Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+

This can probably go in the RuntimeInterfaceBuilder class.



Comment at: clang/lib/Interpreter/Interpreter.cpp:350
   std::list  = IncrParser->getPTUs();
-  if (N > PTUs.size())
+  if (N + InitPTUSize > PTUs.size())
 return llvm::make_error("Operation failed. "

v.g.vassilev wrote:
> I'd propose `IncrParser->getPTUs()` to return the list starting from 
> `InitPTUSize`. That should solve the issue you see.
Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D144218: [Clang] [AVR] Fix USHRT_MAX for 16-bit int.

2023-04-09 Thread Sergei Barannikov via Phabricator via cfe-commits

barannikov88 added a comment.

There is something wrong with the `#if`s, isn't it? They all look no-op.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144218

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-04-09 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 511984.
junaire added a comment.

Refactor how we compile a destructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/Mangle.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Interpreter/Value.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
@@ -33,6 +34,10 @@
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
 #endif
 
+int Global = 42;
+int getGlobal() { return Global; }
+void setGlobal(int val) { Global = val; }
+
 namespace {
 using Args = std::vector;
 static std::unique_ptr
@@ -276,8 +281,7 @@
   std::vector Args = {"-fno-delayed-template-parsing"};
   std::unique_ptr Interp = createInterpreter(Args);
 
-  llvm::cantFail(Interp->Parse("void* operator new(__SIZE_TYPE__, void* __p);"
-   "extern \"C\" int printf(const char*,...);"
+  llvm::cantFail(Interp->Parse("extern \"C\" int printf(const char*,...);"
"class A {};"
"struct B {"
"  template"
@@ -314,4 +318,55 @@
   free(NewA);
 }
 
+TEST(InterpreterTest, Value) {
+  std::unique_ptr Interp = createInterpreter();
+
+  Value V1;
+  llvm::cantFail(Interp->ParseAndExecute("int x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("x", ));
+  EXPECT_TRUE(V1.isValid());
+  EXPECT_EQ(V1.getInt(), 42);
+  EXPECT_TRUE(V1.getType()->isIntegerType());
+  EXPECT_EQ(V1.getKind(), Value::K_Int);
+  EXPECT_FALSE(V1.isManuallyAlloc());
+  EXPECT_FALSE(V1.isPointerOrObjectType());
+
+  Value V2;
+  llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
+  llvm::cantFail(Interp->ParseAndExecute("y", ));
+  EXPECT_TRUE(V2.isValid());
+  EXPECT_EQ(V2.getDouble(), 3.14);
+  EXPECT_TRUE(V2.getType()->isFloatingType());
+  EXPECT_EQ(V2.getKind(), Value::K_Double);
+  EXPECT_FALSE(V2.isManuallyAlloc());
+  EXPECT_FALSE(V2.isPointerOrObjectType());
+
+  Value V3;
+  llvm::cantFail(Interp->ParseAndExecute(
+  "struct S { int* p; S() { p = new int(42); } ~S() { delete p; }};"));
+  llvm::cantFail(Interp->ParseAndExecute("S{}", ));
+  EXPECT_TRUE(V3.isValid());
+  EXPECT_TRUE(V3.getType()->isRecordType());
+  EXPECT_EQ(V3.getKind(), Value::K_PtrOrObj);
+  EXPECT_TRUE(V3.isManuallyAlloc());
+  EXPECT_TRUE(V3.isPointerOrObjectType());
+
+  Value V4;
+  llvm::cantFail(Interp->ParseAndExecute("int getGlobal();"));
+  llvm::cantFail(Interp->ParseAndExecute("void setGlobal(int);"));
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V4.getInt(), 42);
+  EXPECT_TRUE(V4.getType()->isIntegerType());
+
+  Value V5;
+  // Change the global from the compiled code.
+  setGlobal(43);
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V5.getInt(), 43);
+  EXPECT_TRUE(V5.getType()->isIntegerType());
+
+  // Change the global from the interpreted code.
+  llvm::cantFail(Interp->ParseAndExecute("setGlobal(44);"));
+  EXPECT_EQ(getGlobal(), 44);
+}
 } // end anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -22,3 +22,5 @@
 if(NOT WIN32)
   add_subdirectory(ExceptionTests)
 endif()
+
+export_executable_symbols(ClangReplInterpreterTests)
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index:

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

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

LGTM, except pointed out issues with order of fixes in release note and missing 
const.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:706
+size_t IdentifierNamingCheck::HungarianNotation::getAsteriskCount(
+std::string , const NamedDecl *ND) const {
+  size_t PtrCount = 0;

const std::string&



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:294-296
+- Fix hungarian notation issue in :doc:`readability-identifier-naming
+  ` which failed to indicate
+  the number of asterisks.

this list should by sorted by a full check name, also there are already fixes 
for readability-identifier-naming, check if you could merge this to them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[clang] 4f1954e - Revert (3) "[Assignment Tracking] Enable by default"

2023-04-09 Thread via cfe-commits


Author: OCHyams
Date: 2023-04-09T11:21:55+01:00
New Revision: 4f1954ed67c12aca3fd3e67151185c64dc941768

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

LOG: Revert (3) "[Assignment Tracking] Enable by default"

Revert D146987.

This reverts commit f11e1475c97c1ba6b418838f3592de930677c3d0 which
reapplies D146987.

Buildbot: https://lab.llvm.org/buildbot/#/builders/70/builds/36103

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/test/CodeGen/assignment-tracking/flag.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 0e3c7b708071f..5e008fc9b26ee 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5815,7 +5815,7 @@ def experimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignmen
   Group, CodeGenOpts<"EnableAssignmentTracking">,
   NormalizedValuesScope<"CodeGenOptions::AssignmentTrackingOpts">,
   Values<"disabled,enabled,forced">, 
NormalizedValues<["Disabled","Enabled","Forced"]>,
-  MarshallingInfoEnum, "Enabled">;
+  MarshallingInfoEnum, "Disabled">;
 
 } // let Flags = [CC1Option, NoDriverOption]
 

diff  --git a/clang/test/CodeGen/assignment-tracking/flag.cpp 
b/clang/test/CodeGen/assignment-tracking/flag.cpp
index 3bd974fe07c6c..aa1f054dae4d7 100644
--- a/clang/test/CodeGen/assignment-tracking/flag.cpp
+++ b/clang/test/CodeGen/assignment-tracking/flag.cpp
@@ -8,10 +8,10 @@
 // RUN: -emit-llvm  %s -o - -fexperimental-assignment-tracking=disabled 
-O1\
 // RUN: | FileCheck %s --check-prefixes=DISABLE
 
- Enabled by default:
+ Disabled by default:
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\
 // RUN: -emit-llvm  %s -o - -O1
\
-// RUN: | FileCheck %s --check-prefixes=ENABLE
+// RUN: | FileCheck %s --check-prefixes=DISABLE
 
  Disabled at O0 unless forced.
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\



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

[PATCH] D147876: [WIP][clang-tidy] Support specifying Checks as a list in the config file

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 511972.
carlosgalvezp retitled this revision from "[WIP][clang-tidy] Support 
introducing checks as a list in the config file" to "[WIP][clang-tidy] Support 
specifying Checks as a list in the config file".
carlosgalvezp added a comment.

Update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
  clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
@@ -6,3 +6,15 @@
 // CHECK-SPACES-NEXT: hicpp-use-auto
 // CHECK-SPACES-NEXT: hicpp-use-emplace
 // CHECK-SPACES-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-dash --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-DASH
+// CHECK-LIST-DASH: Enabled checks:
+// CHECK-LIST-DASH-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-DASH-NEXT: hicpp-use-auto
+// CHECK-LIST-DASH-NEXT: hicpp-use-emplace
+// CHECK-LIST-DASH-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-bracket --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-BRACKET
+// CHECK-LIST-BRACKET: Enabled checks:
+// CHECK-LIST-BRACKET-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-auto
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-emplace
+// CHECK-LIST-BRACKET-EMPTY:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
@@ -0,0 +1,5 @@
+Checks:
+  - "-*"
+  - "hicpp-uppercase-literal-suffix"
+  - "hicpp-use-auto"
+  - "hicpp-use-emplace"
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
@@ -0,0 +1,6 @@
+Checks: [
+  "-*",
+  "hicpp-uppercase-literal-suffix",
+  "hicpp-use-auto",
+  "hicpp-use-emplace",
+]
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -70,7 +70,8 @@
   NOptionMap(IO &, const ClangTidyOptions::OptionMap ) {
 Options.reserve(OptionMap.size());
 for (const auto  : OptionMap)
-  Options.emplace_back(std::string(KeyValue.getKey()), KeyValue.getValue().Value);
+  Options.emplace_back(std::string(KeyValue.getKey()),
+   KeyValue.getValue().Value);
   }
   ClangTidyOptions::OptionMap denormalize(IO &) {
 ClangTidyOptions::OptionMap Map;
@@ -117,10 +118,58 @@
   }
 }
 
+struct ChecksVariant {
+  std::optional AsString;
+  std::optional> AsVector;
+};
+
+template <>
+void yamlize(IO , ChecksVariant , bool, EmptyContext ) {
+  if (!IO.outputting()) {
+// Special case for reading from YAML
+// Must support reading from both a string or a list
+Input  = reinterpret_cast(IO);
+
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();
+  yamlize(IO, *Checks.AsString, true, Ctx);
+} else if (isa(I.getCurrentNode())) {
+  Checks.AsVector = std::vector();
+  yamlize(IO, *Checks.AsVector, true, Ctx);
+} else {
+  IO.setError("expected string or sequence");
+}
+  }
+}
+
+static void mapChecks(IO , std::optional ) {
+  if (IO.outputting()) {
+// Output as a string
+IO.mapOptional("Checks", Checks);
+  } else {
+// Input as either a string or a list
+ChecksVariant ChecksAsVariant;
+IO.mapOptional("Checks", ChecksAsVariant);
+
+// If we got the input as string, assign directly
+if (ChecksAsVariant.AsString) {
+  Checks = ChecksAsVariant.AsString;
+}
+// If we got the input as a list, concatenate the items to create string
+else if (ChecksAsVariant.AsVector) {
+  Checks = std::string();
+  for (std::string const  : *ChecksAsVariant.AsVector) {
+*Checks += check + ",";
+  }
+}
+  }
+}
+
 template <> struct MappingTraits {
   static void mapping(IO , ClangTidyOptions ) {
 bool Ignored = false;
-IO.mapOptional("Checks", Options.Checks);
+mapChecks(IO, Options.Checks);

[PATCH] D147876: [WIP][clang-tidy] Support introducing checks as a list in the config file

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

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

Add tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
  clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp
@@ -6,3 +6,15 @@
 // CHECK-SPACES-NEXT: hicpp-use-auto
 // CHECK-SPACES-NEXT: hicpp-use-emplace
 // CHECK-SPACES-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-dash --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-DASH
+// CHECK-LIST-DASH: Enabled checks:
+// CHECK-LIST-DASH-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-DASH-NEXT: hicpp-use-auto
+// CHECK-LIST-DASH-NEXT: hicpp-use-emplace
+// CHECK-LIST-DASH-EMPTY:
+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file-list-bracket --list-checks -- | FileCheck %s -check-prefix=CHECK-LIST-BRACKET
+// CHECK-LIST-BRACKET: Enabled checks:
+// CHECK-LIST-BRACKET-NEXT: hicpp-uppercase-literal-suffix
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-auto
+// CHECK-LIST-BRACKET-NEXT: hicpp-use-emplace
+// CHECK-LIST-BRACKET-EMPTY:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-dash
@@ -0,0 +1,5 @@
+Checks:
+  - "-*"
+  - "hicpp-uppercase-literal-suffix"
+  - "hicpp-use-auto"
+  - "hicpp-use-emplace"
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file-list-bracket
@@ -0,0 +1,6 @@
+Checks: [
+  "-*",
+  "hicpp-uppercase-literal-suffix",
+  "hicpp-use-auto",
+  "hicpp-use-emplace",
+]
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -70,7 +70,8 @@
   NOptionMap(IO &, const ClangTidyOptions::OptionMap ) {
 Options.reserve(OptionMap.size());
 for (const auto  : OptionMap)
-  Options.emplace_back(std::string(KeyValue.getKey()), KeyValue.getValue().Value);
+  Options.emplace_back(std::string(KeyValue.getKey()),
+   KeyValue.getValue().Value);
   }
   ClangTidyOptions::OptionMap denormalize(IO &) {
 ClangTidyOptions::OptionMap Map;
@@ -117,10 +118,58 @@
   }
 }
 
+struct ChecksVariant {
+  std::optional AsString;
+  std::optional> AsVector;
+};
+
+template <>
+void yamlize(IO , ChecksVariant , bool, EmptyContext ) {
+  if (!IO.outputting()) {
+// Special case for reading from YAML
+// Must support reading from both a string or a list
+Input  = reinterpret_cast(IO);
+
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();
+  yamlize(IO, *Checks.AsString, true, Ctx);
+} else if (isa(I.getCurrentNode())) {
+  Checks.AsVector = std::vector();
+  yamlize(IO, *Checks.AsVector, true, Ctx);
+} else {
+  IO.setError("expected string or sequence");
+}
+  }
+}
+
+static void mapChecks(IO , std::optional ) {
+  if (IO.outputting()) {
+// Output as a string
+IO.mapOptional("Checks", Checks);
+  } else {
+// Input as either a string or a list
+ChecksVariant ChecksAsVariant;
+IO.mapOptional("Checks", ChecksAsVariant);
+
+// If we got the input as string, assign directly
+if (ChecksAsVariant.AsString) {
+  Checks = ChecksAsVariant.AsString;
+}
+// If we got the input as a list, concatenate the items to create string
+else if (ChecksAsVariant.AsVector) {
+  Checks = std::string();
+  for (std::string const  : *ChecksAsVariant.AsVector) {
+*Checks += check + ",";
+  }
+}
+  }
+}
+
 template <> struct MappingTraits {
   static void mapping(IO , ClangTidyOptions ) {
 bool Ignored = false;
-IO.mapOptional("Checks", Options.Checks);
+mapChecks(IO, Options.Checks);
 IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
 IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
 IO.mapOptional("ImplementationFileExtensions",
___

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-09 Thread Douglas Chen via Phabricator via cfe-commits

dougpuob updated this revision to Diff 511969.
dougpuob marked 2 inline comments as done.
dougpuob added a comment.

- Extract method
- Remove template parameters in the getDeclTypeName() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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-hungarian-notation-c-language.c
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *custpcustu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *custpcustucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **custpcustpcustucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* custpcustvVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
@@ -283,6 +283,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -291,6 +291,10 @@
   ` when warning would be
   emitted for a const local variable to which NRVO is applied.
 
+- Fix hungarian notation issue in :doc:`readability-identifier-naming
+  ` which failed to indicate
+  the number of asterisks.
+
 Removed checks

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-09 Thread Douglas Chen via Phabricator via cfe-commits

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

In D147779#4251081 , @PiotrZSL wrote:

> Missing release notes.

Thank you for reminding me.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:367-379
+if (const auto *TD = dyn_cast(ND)) {
+  QualType QT = TD->getType();
+
+  size_t PtrCount = 0;
+  if (QT->isPointerType()) {
+PtrCount = getAsteriskCount(Type);
+  }

PiotrZSL wrote:
> this function is already big, extract this to have something like:
> ```
> if (removeReudnantAsterisks(Type, ND))
> {
> RedundantRemoved = true;
> break;
> }
> ```
Good idea, thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:696-702
+const std::string ) const {
+  size_t Pos = TypeName.find('*');
+  size_t Count = 0;
+  for (; Pos < TypeName.length(); Pos++, Count++) {
+if ('*' != TypeName[Pos])
+  break;
+  }

PiotrZSL wrote:
> what if type will be like std::optional, will this function 
> also be executed ?
Yes, this function is also executed in this case, but the 
`std::optional` is not in the supported list so the case will 
be ignored. Even though it is still weird, I will add some code to remove 
template parameters in the `getDeclTypeName()` function, making it clean.

- OLD: `std::optional`  --> `std::optional`  --> `std::optional` (removed template 
parameters)

Thank you for reminding me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D147876: [WIP][clang-tidy] Support introducing checks as a list in the config file

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147876

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp


Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -70,7 +70,8 @@
   NOptionMap(IO &, const ClangTidyOptions::OptionMap ) {
 Options.reserve(OptionMap.size());
 for (const auto  : OptionMap)
-  Options.emplace_back(std::string(KeyValue.getKey()), 
KeyValue.getValue().Value);
+  Options.emplace_back(std::string(KeyValue.getKey()),
+   KeyValue.getValue().Value);
   }
   ClangTidyOptions::OptionMap denormalize(IO &) {
 ClangTidyOptions::OptionMap Map;
@@ -117,10 +118,58 @@
   }
 }
 
+struct ChecksVariant {
+  std::optional AsString;
+  std::optional> AsVector;
+};
+
+template <>
+void yamlize(IO , ChecksVariant , bool, EmptyContext ) {
+  if (!IO.outputting()) {
+// Special case for reading from YAML
+// Must support reading from both a string or a list
+Input  = reinterpret_cast(IO);
+
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();
+  yamlize(IO, *Checks.AsString, true, Ctx);
+} else if (isa(I.getCurrentNode())) {
+  Checks.AsVector = std::vector();
+  yamlize(IO, *Checks.AsVector, true, Ctx);
+} else {
+  IO.setError("expected string or sequence");
+}
+  }
+}
+
+static void mapChecks(IO , std::optional ) {
+  if (IO.outputting()) {
+// Output as a string
+IO.mapOptional("Checks", Checks);
+  } else {
+// Input as either a string or a list
+ChecksVariant ChecksAsVariant;
+IO.mapOptional("Checks", ChecksAsVariant);
+
+// If we got the input as string, assign directly
+if (ChecksAsVariant.AsString) {
+  Checks = ChecksAsVariant.AsString;
+}
+// If we got the input as a list, concatenate the items to create string
+else if (ChecksAsVariant.AsVector) {
+  Checks = std::string();
+  for (std::string const  : *ChecksAsVariant.AsVector) {
+*Checks += check + ",";
+  }
+}
+  }
+}
+
 template <> struct MappingTraits {
   static void mapping(IO , ClangTidyOptions ) {
 bool Ignored = false;
-IO.mapOptional("Checks", Options.Checks);
+mapChecks(IO, Options.Checks);
 IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
 IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
 IO.mapOptional("ImplementationFileExtensions",


Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -70,7 +70,8 @@
   NOptionMap(IO &, const ClangTidyOptions::OptionMap ) {
 Options.reserve(OptionMap.size());
 for (const auto  : OptionMap)
-  Options.emplace_back(std::string(KeyValue.getKey()), KeyValue.getValue().Value);
+  Options.emplace_back(std::string(KeyValue.getKey()),
+   KeyValue.getValue().Value);
   }
   ClangTidyOptions::OptionMap denormalize(IO &) {
 ClangTidyOptions::OptionMap Map;
@@ -117,10 +118,58 @@
   }
 }
 
+struct ChecksVariant {
+  std::optional AsString;
+  std::optional> AsVector;
+};
+
+template <>
+void yamlize(IO , ChecksVariant , bool, EmptyContext ) {
+  if (!IO.outputting()) {
+// Special case for reading from YAML
+// Must support reading from both a string or a list
+Input  = reinterpret_cast(IO);
+
+if (isa(I.getCurrentNode()) ||
+isa(I.getCurrentNode())) {
+  Checks.AsString = std::string();
+  yamlize(IO, *Checks.AsString, true, Ctx);
+} else if (isa(I.getCurrentNode())) {
+  Checks.AsVector = std::vector();
+  yamlize(IO, *Checks.AsVector, true, Ctx);
+} else {
+  IO.setError("expected string or sequence");
+}
+  }
+}
+
+static void mapChecks(IO , std::optional ) {
+  if (IO.outputting()) {
+// Output as a string
+IO.mapOptional("Checks", Checks);
+  } else {
+// Input as either a string or a list
+ChecksVariant ChecksAsVariant;
+IO.mapOptional("Checks", ChecksAsVariant);
+
+// If we got the input as string, assign directly
+if (ChecksAsVariant.AsString) {
+  Checks = ChecksAsVariant.AsString;
+}
+// If we got the input as a list, concatenate the items to create string
+else if (ChecksAsVariant.AsVector) {
+  Checks = std::string();
+  for (std::string const  :

[clang] f11e147 - Reapply (2) "[Assignment Tracking] Enable by default"

2023-04-09 Thread via cfe-commits


Author: OCHyams
Date: 2023-04-09T09:59:26+01:00
New Revision: f11e1475c97c1ba6b418838f3592de930677c3d0

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

LOG: Reapply (2) "[Assignment Tracking] Enable by default"

Re-land D146987.

This reverts commit 64fba207a683a355d9059cd57adb8a139c2f5dda
which reverts D146987.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/test/CodeGen/assignment-tracking/flag.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 5e008fc9b26ee..0e3c7b708071f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5815,7 +5815,7 @@ def experimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignmen
   Group, CodeGenOpts<"EnableAssignmentTracking">,
   NormalizedValuesScope<"CodeGenOptions::AssignmentTrackingOpts">,
   Values<"disabled,enabled,forced">, 
NormalizedValues<["Disabled","Enabled","Forced"]>,
-  MarshallingInfoEnum, "Disabled">;
+  MarshallingInfoEnum, "Enabled">;
 
 } // let Flags = [CC1Option, NoDriverOption]
 

diff  --git a/clang/test/CodeGen/assignment-tracking/flag.cpp 
b/clang/test/CodeGen/assignment-tracking/flag.cpp
index aa1f054dae4d7..3bd974fe07c6c 100644
--- a/clang/test/CodeGen/assignment-tracking/flag.cpp
+++ b/clang/test/CodeGen/assignment-tracking/flag.cpp
@@ -8,10 +8,10 @@
 // RUN: -emit-llvm  %s -o - -fexperimental-assignment-tracking=disabled 
-O1\
 // RUN: | FileCheck %s --check-prefixes=DISABLE
 
- Disabled by default:
+ Enabled by default:
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\
 // RUN: -emit-llvm  %s -o - -O1
\
-// RUN: | FileCheck %s --check-prefixes=DISABLE
+// RUN: | FileCheck %s --check-prefixes=ENABLE
 
  Disabled at O0 unless forced.
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\



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

[PATCH] D147874: [clang-tidy] Fix AST Library documentation link

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f5951f11a0e: [clang-tidy] Fix AST Library documentation 
link (authored by sousajo, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147874

Files:
  clang-tools-extra/docs/clang-tidy/Contributing.rst


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -278,7 +278,7 @@
   for information about tokens, lexing (transforming characters into tokens) 
and the
   preprocessor.
 - `The AST Library
-  
`_
+  `_
   for information about how C++ source statements are represented as an 
abstract syntax
   tree (AST).
 


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -278,7 +278,7 @@
   for information about tokens, lexing (transforming characters into tokens) and the
   preprocessor.
 - `The AST Library
-  `_
+  `_
   for information about how C++ source statements are represented as an abstract syntax
   tree (AST).
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 9f5951f - [clang-tidy] Fix AST Library documentation link

2023-04-09 Thread Piotr Zegar via cfe-commits


Author: Jorge Pinto Sousa
Date: 2023-04-09T08:47:54Z
New Revision: 9f5951f11a0ef3227077d5a47b62e8842655fbd8

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

LOG: [clang-tidy] Fix AST Library documentation link

The link was pointing to the 'Lexer and Preprocessor Library'
instead.

Reviewed By: PiotrZSL

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/Contributing.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst 
b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 0014bb23aee4f..c4b93927eae23 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -278,7 +278,7 @@ are:
   for information about tokens, lexing (transforming characters into tokens) 
and the
   preprocessor.
 - `The AST Library
-  
`_
+  `_
   for information about how C++ source statements are represented as an 
abstract syntax
   tree (AST).
 



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

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

2023-04-09 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

@fhahn do you plan to continue with this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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

[PATCH] D147874: [clang-tidy] Fix AST Library documentation link

2023-04-09 Thread Jorge Pinto Sousa via Phabricator via cfe-commits

sousajo added a comment.

I dont have commit access, can someone land it for me?

Name: Jorge Pinto Sousa
Email: 


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

https://reviews.llvm.org/D147874

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

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

2023-04-09 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added inline comments.



Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1141
+}
+
 /// Emit a code snippet and caret line.

Wrote this just to get things going but I hope there's something better than 
this...?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147875

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

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

2023-04-09 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, cjdb.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Just putting this here because it's a pretty simple patch as it turns out.

Caveats:

1. WIP, so I haven't fixed up or added any tests yet
2. We (probably) need a cmdline option to disable line numbers
3. With the default for `-fcaret-diagnostics-max-lines` being `1`, the line 
numbers don't make that much sense
4. caret, underline and fixits probably need a ` | ` prefix as well

Sample output:

  ./array.cpp:98:1: error: static assertion failed due to requirement 'func()'
98 | static_assert(
 ^
99 | // I wonder if this works.
   100 | // Also, what if it doesn't?
   101 | func());
 ~~
  1 error generated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147875

Files:
  clang/include/clang/Frontend/TextDiagnostic.h
  clang/lib/Frontend/TextDiagnostic.cpp

Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -1121,6 +1121,24 @@
   return FixItInsertionLine;
 }
 
+static unsigned getNumDisplayWidth(unsigned N) {
+  if (N < 10)
+return 1;
+  if (N < 100)
+return 2;
+  if (N < 1'000)
+return 3;
+  if (N < 10'000)
+return 4;
+  if (N < 100'000)
+return 5;
+  if (N < 1'000'000)
+return 6;
+  if (N < 10'000'000)
+return 7;
+  return 0;
+}
+
 /// Emit a code snippet and caret line.
 ///
 /// This routine emits a single line's code snippet and caret line..
@@ -1174,6 +1192,10 @@
 if (auto OptionalRange = findLinesForRange(*I, FID, SM))
   Lines = maybeAddRange(Lines, *OptionalRange, MaxLines);
 
+  unsigned MaxLineNoDisplayWidth = std::max(getNumDisplayWidth(Lines.first),
+getNumDisplayWidth(Lines.second));
+  unsigned HighlightIndent = MaxLineNoDisplayWidth + 4;
+
   for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1; ++LineNo) {
 const char *BufStart = BufData.data();
 const char *BufEnd = BufStart + BufData.size();
@@ -1249,9 +1271,12 @@
   CaretLine.erase(CaretLine.end() - 1);
 
 // Emit what we have computed.
-emitSnippet(SourceLine);
+emitSnippet(SourceLine, MaxLineNoDisplayWidth, LineNo);
 
 if (!CaretLine.empty()) {
+  // Indent for line numbers
+  for (unsigned I = 0; I != HighlightIndent; ++I)
+OS << ' ';
   if (DiagOpts->ShowColors)
 OS.changeColor(caretColor, true);
   OS << CaretLine << '\n';
@@ -1260,6 +1285,8 @@
 }
 
 if (!FixItInsertionLine.empty()) {
+  for (unsigned I = 0; I != HighlightIndent; ++I)
+OS << ' ';
   if (DiagOpts->ShowColors)
 // Print fixit line in color
 OS.changeColor(fixitColor, false);
@@ -1275,7 +1302,8 @@
   emitParseableFixits(Hints, SM);
 }
 
-void TextDiagnostic::emitSnippet(StringRef line) {
+void TextDiagnostic::emitSnippet(StringRef line, unsigned MaxLineNoDisplayWidth,
+ unsigned LineNo) {
   if (line.empty())
 return;
 
@@ -1284,6 +1312,16 @@
   std::string to_print;
   bool print_reversed = false;
 
+  // Emit line number.
+  {
+unsigned LineNoDisplayWidth = getNumDisplayWidth(LineNo);
+OS << ' ';
+for (unsigned I = LineNoDisplayWidth; I < MaxLineNoDisplayWidth; ++I)
+  OS << ' ';
+OS << LineNo;
+OS << " | ";
+  }
+
   while (i,bool> res
 = printableTextForNextCharacter(line, , DiagOpts->TabStop);
Index: clang/include/clang/Frontend/TextDiagnostic.h
===
--- clang/include/clang/Frontend/TextDiagnostic.h
+++ clang/include/clang/Frontend/TextDiagnostic.h
@@ -103,7 +103,8 @@
SmallVectorImpl ,
ArrayRef Hints);
 
-  void emitSnippet(StringRef SourceLine);
+  void emitSnippet(StringRef SourceLine, unsigned MaxLineNoDisplayWidth,
+   unsigned LineNo);
 
   void emitParseableFixits(ArrayRef Hints, const SourceManager );
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-04-09 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni added inline comments.



Comment at: clang/test/Modules/gmodules-preferred-name-alias.cpp:6
+// RUN: -fmodules -mllvm -debug-only=pchcontainer -x c++ \
+// RUN: -I %S/Inputs %s &> %t.ll
+// RUN: cat %t.ll | FileCheck %s

I guess it made difficult to find out the actual error since it hid stderr.
In fact, I had to reproduce failures locally.

In addition, mixing stdout and stderr sometimes confuse line order due to 
buffering.

btw, did it really require stderr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D147874: [clang-tidy] Fix AST Library documentation link

2023-04-09 Thread Jorge Pinto Sousa via Phabricator via cfe-commits

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

The link was pointing to the 'Lexer and Preprocessor Library'
instead.


https://reviews.llvm.org/D147874

Files:
  clang-tools-extra/docs/clang-tidy/Contributing.rst


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -278,7 +278,7 @@
   for information about tokens, lexing (transforming characters into tokens) 
and the
   preprocessor.
 - `The AST Library
-  
`_
+  `_
   for information about how C++ source statements are represented as an 
abstract syntax
   tree (AST).
 


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -278,7 +278,7 @@
   for information about tokens, lexing (transforming characters into tokens) and the
   preprocessor.
 - `The AST Library
-  `_
+  `_
   for information about how C++ source statements are represented as an abstract syntax
   tree (AST).
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

2023-04-09 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp added a comment.

It would still be good to add some test that ensures this does not happen again 
in the future, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146520

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

[PATCH] D147857: [clang-tidy] fix false positve for namespace with attrs in modernize-concat-nested-namespaces

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

Reference issue in commit message, so it could auto-close, or close it manualy 
later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147857

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

[PATCH] D147857: [clang-tidy] fix false positve for namespace with attrs in modernize-concat-nested-namespaces

2023-04-09 Thread Piotr Zegar via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:304-310
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
   ` when using macro 
between
   namespace declarations could result incorrect fix.
 
+- Fixed an false positive in :doc:`modernize-concat-nested-namespaces
+  ` when using namespace 
with 
+  attributes.

Merge those 2 into one if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147857

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

[PATCH] D143974: [clangd] Inactive regions support via dedicated protocol

2023-04-09 Thread Nathan Ridge via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:977
+  return CB(InpAST.takeError());
+// Include inactive regions in semantic highlighting tokens only if the
+// client doesn't support a dedicated protocol for being informed about

hokein wrote:
> As discussed in the GitHub thread (the experiment of highlighting the 
> inactive regions as comment is considered as a failure),  we should always 
> not include the inactive region in the semantic highlighting, this will also 
> simplify the existing code in semantic highlighting (e.g. 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/SemanticHighlighting.cpp#L464-L513).
>  I think we can do it in a separated patch. 
I think it might make sense to keep this support around for a while, so that 
users whose clients do not yet support this extension still have //some// 
indication of which preprocessor branches are inactive.

However, we could make it optional, since some users (for example those who 
have to work on large sections of code in inactive preprocessor branches) may 
prefer to see the client-side colors over having it all highlighted as one 
color.



Comment at: clang-tools-extra/clangd/ClangdServer.h:91
+virtual void onInactiveRegionsReady(PathRef File,
+std::vector InactiveRegions) {}
   };

hokein wrote:
> it would be nice to have a unittest for this, I think it should not be to 
> hard to add one in `ClangdTests.cpp` (with a customize Callbacks passing to 
> the `ClangdServer`).
Good idea -- added!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143974

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

[PATCH] D143974: [clangd] Inactive regions support via dedicated protocol

2023-04-09 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 511957.
nridge marked 3 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143974

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -89,7 +89,8 @@
   for (auto File : AdditionalFiles)
 TU.AdditionalFiles.insert({File.first, std::string(File.second)});
   auto AST = TU.build();
-  auto Actual = getSemanticHighlightings(AST);
+  auto Actual =
+  getSemanticHighlightings(AST, /*IncludeInactiveRegionTokens=*/true);
   for (auto  : Actual)
 Token.Modifiers &= ModifierMask;
 
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -1310,6 +1310,50 @@
   });
   N.wait();
 }
+
+TEST(ClangdServer, InactiveRegions) {
+  struct InactiveRegionsCallback : ClangdServer::Callbacks {
+std::vector> FoundInactiveRegions;
+
+void onInactiveRegionsReady(PathRef FIle,
+std::vector InactiveRegions) override {
+  FoundInactiveRegions.push_back(std::move(InactiveRegions));
+}
+  };
+
+  MockFS FS;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags.push_back("-DCMDMACRO");
+  auto Opts = ClangdServer::optsForTest();
+  Opts.PublishInactiveRegions = true;
+  InactiveRegionsCallback Callback;
+  ClangdServer Server(CDB, FS, Opts, );
+  Annotations Source(R"cpp(
+#define PREAMBLEMACRO 42
+#if PREAMBLEMACRO > 40
+  #define ACTIVE
+$inactive1[[#else
+  #define INACTIVE
+#endif]]
+int endPreamble;
+$inactive2[[#ifndef CMDMACRO
+int inactiveInt;
+#endif]]
+#undef CMDMACRO
+$inactive3[[#ifdef CMDMACRO
+  int inactiveInt2;
+#else]]
+  int activeInt;
+#endif
+  )cpp");
+  Server.addDocument(testPath("foo.cpp"), Source.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(Callback.FoundInactiveRegions,
+  ElementsAre(ElementsAre(Source.range("inactive1"),
+  Source.range("inactive2"),
+  Source.range("inactive3";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -344,7 +344,8 @@
 
   void buildSemanticHighlighting(std::optional LineRange) {
 log("Building semantic highlighting");
-auto Highlights = getSemanticHighlightings(*AST);
+auto Highlights =
+getSemanticHighlightings(*AST, /*IncludeInactiveRegionTokens=*/true);
 for (const auto HL : Highlights)
   if (!LineRange || LineRange->contains(HL.R))
 vlog(" {0} {1} {2}", HL.R, HL.Kind, HL.Modifiers);
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -47,14 +47,16 @@
 // Now we hit the TUDecl case where commonAncestor() returns null
 // intendedly. We only annotate tokens in the main file, so use the default
 // traversal scope (which is the top level decls of the main file).
-HighlightingTokens = getSemanticHighlightings(*Inputs.AST);
+HighlightingTokens = getSemanticHighlightings(
+*Inputs.AST, /*IncludeInactiveRegionTokens=*/true);
   } else {
 // Store the existing scopes.
 const auto  = Inputs.AST->getASTContext().getTraversalScope();
 // Narrow the traversal scope to the selected node.
 Inputs.AST->getASTContext().setTraversalScope(
 {const_cast(CommonDecl)});
-HighlightingTokens = getSemanticHighlightings(*Inputs.AST);
+HighlightingTokens = getSemanticHighlightings(
+*Inputs.AST, /*IncludeInactiveRegionTokens=*/true);
 // Restore the traversal scope.

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

2023-04-09 Thread Yi Zhang via Phabricator via cfe-commits

zhangyi1357 added a comment.

I dont have commit access. Could you help committing the change for me? @hokein




Comment at: clang-tools-extra/clangd/Config.h:151
+// Limit the length of type names in inlay hints.
+size_t TypeNameLimit = 32;
   } InlayHints;

hokein wrote:
> zhangyi1357 wrote:
> > hokein wrote:
> > > I would extend it a bit more -- 0 means no limit. 
> > > 
> > > Can you also add a unittest in `TEST(TypeHints, LongTypeName)` in 
> > > `InlayHintTests.cpp`? 
> > > 0 means no limit.
> > This is quite a good idea. I've done it.
> > 
> > For unittest, there is already `TEST(TypeHints, LongTypeName)` in 
> > `InlayHintTests.cpp`. Do you mean add more tests in the same `TEST` or 
> > another `TEST` with TypeNameLimit configured?
> > 
> I mean adding one more test in the same `TEST(TypeHints, LongTypeName)`.
> 
> This test verifies the the long type name is shown when the limit is set to 
> 0, something like
> 
> ```
> TEST(TypeHints, LongTypeName) {
>   assertTypeHints(R"cpp(
> template 
> struct A {};
> struct MultipleWords {};
> A foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
>   )cpp");
> 
> Config cfg; 
>  ... // set the limit to 0
> 
>assertTypeHints(R"cpp(
> template 
> struct A {};
> struct MultipleWords {};
> A foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
>   )cpp", ExpectedHint...);
> }
> ```
Thanks for your example! I've added that.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:80
   C.InlayHints.Designators = false;
+  C.InlayHints.TypeNameLimit = 1;
   return C;

hokein wrote:
> why do we need this change? I think it should be fine without it.
Yes, without this line the tests will not fail. But I am a little confused 
about the code below without 'C.InlayHints.TypeNameLimit = 1;'.
```
Config noHintsConfig() {
  Config C;
  C.InlayHints.Parameters = false;
  C.InlayHints.DeducedTypes = false;
  C.InlayHints.Designators = false;
  // C.InlayHints.TypeNameLimit = 1;
  return C;
}

template 
void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
 ExpectedHints... Expected) {
  Annotations Source(AnnotatedSource);
  TestTU TU = TestTU::withCode(Source.code());
  TU.ExtraArgs.push_back("-std=c++20");
  auto AST = TU.build();

  EXPECT_THAT(hintsOfKind(AST, Kind),
  ElementsAre(HintMatcher(Expected, Source)...));
  // Sneak in a cross-cutting check that hints are disabled by config.
  // We'll hit an assertion failure if addInlayHint still gets called.
  WithContextValue WithCfg(Config::Key, noHintsConfig());
  EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty());  // Why does this 
succeed with TypeNameLimit = 32 ?
}

TEST(TypeHints, Smoke) {
  assertTypeHints(R"cpp(
auto $waldo[[waldo]] = 42;
  )cpp",
  ExpectedHint{": int", "waldo"});
}
```
The dault TypeNameLimit is 32, why does the second `EXPECT_THAT` succeed with 
TypeNameLimit = 32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147395

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

63 matches

Mail list logo