date:20230715

[PATCH] D155358: [clang-format] Correctly annotate overloaded operator function name

2023-07-15 Thread Owen Pan via Phabricator via cfe-commits

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

The `operator` keyword preceded by a template closer should be annotated as 
`TT_FunctionDeclarationName`.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155358

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -717,6 +717,16 @@
   EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
   EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("std::vector operator()(Foo &foo);");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[5], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[6], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3175,7 +3175,7 @@
 !Previous->isOneOf(tok::kw_return, tok::kw_co_return)) {
   return true;
 }
-if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp, 
TT_TemplateCloser))
   return false;
 Next = skipOperatorName(Next);
   } else {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -717,6 +717,16 @@
   EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
   EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("std::vector operator()(Foo &foo);");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[5], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[6], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3175,7 +3175,7 @@
 !Previous->isOneOf(tok::kw_return, tok::kw_co_return)) {
   return true;
 }
-if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp, TT_TemplateCloser))
   return false;
 Next = skipOperatorName(Next);
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154616: [RISCV] Use unsigned instead of signed types for Zk* and Zb* builtins.

2023-07-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper updated this revision to Diff 540651.
craig.topper added a comment.

Rebase. I'll commit tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154616

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkx.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb-error.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkx.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c

Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
@@ -10,7 +10,7 @@
 // RV64ZKSH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sm3p0.i64(i64 [[TMP0]])
 // RV64ZKSH-NEXT:ret i64 [[TMP1]]
 //
-long sm3p0(long rs1) {
+unsigned long sm3p0(unsigned long rs1) {
   return __builtin_riscv_sm3p0(rs1);
 }
 
@@ -23,6 +23,6 @@
 // RV64ZKSH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sm3p1.i64(i64 [[TMP0]])
 // RV64ZKSH-NEXT:ret i64 [[TMP1]]
 //
-long sm3p1(long rs1) {
+unsigned long sm3p1(unsigned long rs1) {
   return __builtin_riscv_sm3p1(rs1);
 }
Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
@@ -13,7 +13,7 @@
 // RV64ZKSED-NEXT:[[TMP2:%.*]] = call i64 @llvm.riscv.sm4ks.i64(i64 [[TMP0]], i64 [[TMP1]], i32 0)
 // RV64ZKSED-NEXT:ret i64 [[TMP2]]
 //
-long sm4ks(long rs1, long rs2) {
+unsigned long sm4ks(unsigned long rs1, unsigned long rs2) {
   return __builtin_riscv_sm4ks(rs1, rs2, 0);
 }
 
@@ -28,6 +28,6 @@
 // RV64ZKSED-NEXT:[[TMP2:%.*]] = call i64 @llvm.riscv.sm4ed.i64(i64 [[TMP0]], i64 [[TMP1]], i32 0)
 // RV64ZKSED-NEXT:ret i64 [[TMP2]]
 //
-long sm4ed(long rs1, long rs2) {
+unsigned long sm4ed(unsigned long rs1, unsigned long rs2) {
   return __builtin_riscv_sm4ed(rs1, rs2, 0);
 }
Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple riscv64 -target-feature +zknh -emit-llvm %s -o - \
 // RUN: | FileCheck %s  -check-prefix=RV64ZKNH
 
+#include 
 
 // RV64ZKNH-LABEL: @sha512sig0(
 // RV64ZKNH-NEXT:  entry:
@@ -11,7 +12,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sig0(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sig0(long rs1) {
+uint64_t sha512sig0(uint64_t rs1) {
   return __builtin_riscv_sha512sig0_64(rs1);
 }
 
@@ -24,7 +25,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sig1(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sig1(long rs1) {
+uint64_t sha512sig1(uint64_t rs1) {
   return __builtin_riscv_sha512sig1_64(rs1);
 }
 
@@ -37,7 +38,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sum0(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sum0(long rs1) {
+uint64_t sha512sum0(uint64_t rs1) {
   return __builtin_riscv_sha512sum0_64(rs1);
 }
 
@@ -50,7 +51,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sum1(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sum1(long rs1) {
+uint64_t sha512sum1(uint64_t rs1) {
   return __builtin_riscv_sha512sum1_64(rs1);
 }
 
@@ -63,7 +64,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha256sig0.i64(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha256sig0(long rs1) {
+uint64_t sha256sig0(uint64_t rs1) {
   return __builtin_riscv_sha256sig0(rs1);
 }
 
@@ -75,7 +76,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha256sig1.i64(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha256sig1(long rs1) {
+uint64_t sha256sig1(uint64_t rs1) {
   return __builtin_riscv_sha256sig1(rs1);
 }
 
@@ -88,7 +89,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha256sum0.i64(i64 [[TMP0]])

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

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

The tooling APIs have a lot of extension points for customization:
e.g. Executor, FrontendActionFactory, FrontendAction, ASTConsumer.
This customization is often not needed, and makes the APIs unergonomic.

This change introduces some shortcuts to bypass them, while attempting
not to add more concepts/complexity to the APIs.

1. main() entrypoint

Today tools are expected to create an executor (which can fail), then
use it to execute their action (which can report errors).
Errors are reported with llvm::Error, which is not what main() needs
(exit codes) but must be handled.

This patch adds executeFromCommandLineArgs(), which wraps this in a
single function which returns an exit code.

(It also tweaks ToolExecutor's execute() API to avoid replicating several
overloads here).

2. Skipping over FrontendAction to ASTConsumer

Tools that override ASTConsumer must provide two factory indirections:

  FrontendActionFactory -> FrontendAction -> ASTConsumer.

These are sometimes meaningful, but often not.
newFrontendActionFactory() already lets you skip the first.

Now newFrontendActionFactory() lets you skip the second, too.

3. Implementing simple actions as lambdas.

A large fraction of tools simply want to consume the final AST and do
something with it. Even ASTConsumer is too heavyweight an abstraction.

This patch adds consumeASTs(), which is a FrontendActionFactory that
calls a function once for each parsed AST:

  Exec->execute(consumeASTs([&](ASTContext &Ctx) {
... process AST ...
  }));

Bonus: through lambda captures, state can be easily shared across TUs.
(This otherwise tends to require plumbing, or use of global variables)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155361

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/include/clang/Tooling/Execution.h
  clang/include/clang/Tooling/StandaloneExecution.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/Execution.cpp
  clang/lib/Tooling/StandaloneExecution.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ExecutionTest.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -511,6 +512,18 @@
 }
 #endif
 
+TEST(newFrontendActionFactory, ConsumeASTs) {
+  FixedCompilationDatabase Compilations(".", std::vector());
+  ClangTool Tool(Compilations, {"a.cc"});
+  Tool.mapVirtualFile("a.cc", "int x = 1;");
+  bool FoundX = false;
+  Tool.run(consumeASTs([&](ASTContext &Ctx) {
+ DeclarationName X(&Ctx.Idents.get("x"));
+ FoundX = Ctx.getTranslationUnitDecl()->lookup(X).isSingleResult();
+   }).get());
+  EXPECT_TRUE(FoundX);
+}
+
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
Index: clang/unittests/Tooling/ExecutionTest.cpp
===
--- clang/unittests/Tooling/ExecutionTest.cpp
+++ clang/unittests/Tooling/ExecutionTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -18,10 +19,12 @@
 #include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -97,9 +100,7 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
-  llvm::Error
-  execute(llvm::ArrayRef,
-   ArgumentsAdjuster>>) override {
+  llvm::Error execute(llvm::ArrayRef) override {
 return llvm::Error::success();
   }
 
@@ -181,14 +182,32 @@
   EXPECT_EQ(Executor->get()->getExecutorName(), TestToolExecutor::ExecutorName);
 }
 
+TEST(CreateToolExecutorTest, ExecuteFromCommandLine) {
+  llvm::unittest::TempFile Source1("test1", ".cpp", "int x = 0;", true);
+  llvm::unittest::TempFile Source2("test2", ".cpp", "int y = 0;", true);
+  std::string Path1 = Source1.path().str();
+  std::string Path2 = Source2.path().str();
+  std::vector argv =

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 540656.
sammccall edited the summary of this revision.
sammccall added a comment.

update description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155361

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/include/clang/Tooling/Execution.h
  clang/include/clang/Tooling/StandaloneExecution.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/Execution.cpp
  clang/lib/Tooling/StandaloneExecution.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ExecutionTest.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -511,6 +512,18 @@
 }
 #endif
 
+TEST(newFrontendActionFactory, ConsumeASTs) {
+  FixedCompilationDatabase Compilations(".", std::vector());
+  ClangTool Tool(Compilations, {"a.cc"});
+  Tool.mapVirtualFile("a.cc", "int x = 1;");
+  bool FoundX = false;
+  Tool.run(consumeASTs([&](ASTContext &Ctx) {
+ DeclarationName X(&Ctx.Idents.get("x"));
+ FoundX = Ctx.getTranslationUnitDecl()->lookup(X).isSingleResult();
+   }).get());
+  EXPECT_TRUE(FoundX);
+}
+
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
Index: clang/unittests/Tooling/ExecutionTest.cpp
===
--- clang/unittests/Tooling/ExecutionTest.cpp
+++ clang/unittests/Tooling/ExecutionTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -18,10 +19,12 @@
 #include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -97,9 +100,7 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
-  llvm::Error
-  execute(llvm::ArrayRef,
-   ArgumentsAdjuster>>) override {
+  llvm::Error execute(llvm::ArrayRef) override {
 return llvm::Error::success();
   }
 
@@ -181,14 +182,32 @@
   EXPECT_EQ(Executor->get()->getExecutorName(), TestToolExecutor::ExecutorName);
 }
 
+TEST(CreateToolExecutorTest, ExecuteFromCommandLine) {
+  llvm::unittest::TempFile Source1("test1", ".cpp", "int x = 0;", true);
+  llvm::unittest::TempFile Source2("test2", ".cpp", "int y = 0;", true);
+  std::string Path1 = Source1.path().str();
+  std::string Path2 = Source2.path().str();
+  std::vector argv = {"prog", Path1.c_str(), Path2.c_str()};
+
+  int argc = argv.size();
+  int YCount = 0;
+  int Result = executeFromCommandLineArgs(
+  argc, &argv[0], consumeASTs([&](ASTContext &Ctx) {
+DeclarationName Y(&Ctx.Idents.get("y"));
+YCount += Ctx.getTranslationUnitDecl()->lookup(Y).isSingleResult();
+  }));
+  EXPECT_EQ(Result, 0);
+  EXPECT_EQ(YCount, 1);
+}
+
 TEST(StandaloneToolTest, SynctaxOnlyActionOnSimpleCode) {
   FixedCompilationDatabase Compilations(".", std::vector());
   StandaloneToolExecutor Executor(Compilations,
   std::vector(1, "a.cc"));
   Executor.mapVirtualFile("a.cc", "int x = 0;");
 
-  auto Err = Executor.execute(newFrontendActionFactory(),
-  getClangSyntaxOnlyAdjuster());
+  auto Err = Executor.execute({newFrontendActionFactory(),
+   getClangSyntaxOnlyAdjuster()});
   ASSERT_TRUE(!Err);
 }
 
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "clang/Tooling/Tooling.h"
+#include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
@@ -27,6 +28,7 @@
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/Front

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 540657.
sammccall added a comment.

tweak comment, remove unused overload


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155361

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/include/clang/Tooling/Execution.h
  clang/include/clang/Tooling/StandaloneExecution.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/Execution.cpp
  clang/lib/Tooling/StandaloneExecution.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ExecutionTest.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -511,6 +512,18 @@
 }
 #endif
 
+TEST(newFrontendActionFactory, ConsumeASTs) {
+  FixedCompilationDatabase Compilations(".", std::vector());
+  ClangTool Tool(Compilations, {"a.cc"});
+  Tool.mapVirtualFile("a.cc", "int x = 1;");
+  bool FoundX = false;
+  Tool.run(consumeASTs([&](ASTContext &Ctx) {
+ DeclarationName X(&Ctx.Idents.get("x"));
+ FoundX = Ctx.getTranslationUnitDecl()->lookup(X).isSingleResult();
+   }).get());
+  EXPECT_TRUE(FoundX);
+}
+
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
Index: clang/unittests/Tooling/ExecutionTest.cpp
===
--- clang/unittests/Tooling/ExecutionTest.cpp
+++ clang/unittests/Tooling/ExecutionTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -18,10 +19,12 @@
 #include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -97,9 +100,7 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
-  llvm::Error
-  execute(llvm::ArrayRef,
-   ArgumentsAdjuster>>) override {
+  llvm::Error execute(llvm::ArrayRef) override {
 return llvm::Error::success();
   }
 
@@ -181,14 +182,32 @@
   EXPECT_EQ(Executor->get()->getExecutorName(), TestToolExecutor::ExecutorName);
 }
 
+TEST(CreateToolExecutorTest, ExecuteFromCommandLine) {
+  llvm::unittest::TempFile Source1("test1", ".cpp", "int x = 0;", true);
+  llvm::unittest::TempFile Source2("test2", ".cpp", "int y = 0;", true);
+  std::string Path1 = Source1.path().str();
+  std::string Path2 = Source2.path().str();
+  std::vector argv = {"prog", Path1.c_str(), Path2.c_str()};
+
+  int argc = argv.size();
+  int YCount = 0;
+  int Result = executeFromCommandLineArgs(
+  argc, &argv[0], consumeASTs([&](ASTContext &Ctx) {
+DeclarationName Y(&Ctx.Idents.get("y"));
+YCount += Ctx.getTranslationUnitDecl()->lookup(Y).isSingleResult();
+  }));
+  EXPECT_EQ(Result, 0);
+  EXPECT_EQ(YCount, 1);
+}
+
 TEST(StandaloneToolTest, SynctaxOnlyActionOnSimpleCode) {
   FixedCompilationDatabase Compilations(".", std::vector());
   StandaloneToolExecutor Executor(Compilations,
   std::vector(1, "a.cc"));
   Executor.mapVirtualFile("a.cc", "int x = 0;");
 
-  auto Err = Executor.execute(newFrontendActionFactory(),
-  getClangSyntaxOnlyAdjuster());
+  auto Err = Executor.execute({newFrontendActionFactory(),
+   getClangSyntaxOnlyAdjuster()});
   ASSERT_TRUE(!Err);
 }
 
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "clang/Tooling/Tooling.h"
+#include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
@@ -27,6 +28,7 @@
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 540659.
sammccall added a comment.

squash commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155361

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/include/clang/Tooling/Execution.h
  clang/include/clang/Tooling/StandaloneExecution.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/Execution.cpp
  clang/lib/Tooling/StandaloneExecution.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ExecutionTest.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -152,13 +153,21 @@
   EXPECT_EQ(1u, Consumer.NumDiagnosticsSeen);
 }
 
-TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromType) {
+TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromActionType) {
   std::unique_ptr Factory(
   newFrontendActionFactory());
   std::unique_ptr Action(Factory->create());
   EXPECT_TRUE(Action.get() != nullptr);
 }
 
+TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromConsumerType) {
+  class MyASTConsumer : public ASTConsumer {};
+  std::unique_ptr Factory(
+  newFrontendActionFactory());
+  std::unique_ptr Action(Factory->create());
+  EXPECT_TRUE(Action.get() != nullptr);
+}
+
 struct IndependentFrontendActionCreator {
   std::unique_ptr newASTConsumer() {
 return std::make_unique(nullptr);
@@ -503,6 +512,18 @@
 }
 #endif
 
+TEST(newFrontendActionFactory, ConsumeASTs) {
+  FixedCompilationDatabase Compilations(".", std::vector());
+  ClangTool Tool(Compilations, {"a.cc"});
+  Tool.mapVirtualFile("a.cc", "int x = 1;");
+  bool FoundX = false;
+  Tool.run(consumeASTs([&](ASTContext &Ctx) {
+ DeclarationName X(&Ctx.Idents.get("x"));
+ FoundX = Ctx.getTranslationUnitDecl()->lookup(X).isSingleResult();
+   }).get());
+  EXPECT_TRUE(FoundX);
+}
+
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
Index: clang/unittests/Tooling/ExecutionTest.cpp
===
--- clang/unittests/Tooling/ExecutionTest.cpp
+++ clang/unittests/Tooling/ExecutionTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -18,10 +19,12 @@
 #include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -97,9 +100,7 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
-  llvm::Error
-  execute(llvm::ArrayRef,
-   ArgumentsAdjuster>>) override {
+  llvm::Error execute(llvm::ArrayRef) override {
 return llvm::Error::success();
   }
 
@@ -181,14 +182,32 @@
   EXPECT_EQ(Executor->get()->getExecutorName(), TestToolExecutor::ExecutorName);
 }
 
+TEST(CreateToolExecutorTest, ExecuteFromCommandLine) {
+  llvm::unittest::TempFile Source1("test1", ".cpp", "int x = 0;", true);
+  llvm::unittest::TempFile Source2("test2", ".cpp", "int y = 0;", true);
+  std::string Path1 = Source1.path().str();
+  std::string Path2 = Source2.path().str();
+  std::vector argv = {"prog", Path1.c_str(), Path2.c_str()};
+
+  int argc = argv.size();
+  int YCount = 0;
+  int Result = executeFromCommandLineArgs(
+  argc, &argv[0], consumeASTs([&](ASTContext &Ctx) {
+DeclarationName Y(&Ctx.Idents.get("y"));
+YCount += Ctx.getTranslationUnitDecl()->lookup(Y).isSingleResult();
+  }));
+  EXPECT_EQ(Result, 0);
+  EXPECT_EQ(YCount, 1);
+}
+
 TEST(StandaloneToolTest, SynctaxOnlyActionOnSimpleCode) {
   FixedCompilationDatabase Compilations(".", std::vector());
   StandaloneToolExecutor Executor(Compilations,
   std::vector(1, "a.cc"));
   Executor.mapVirtualFile("a.cc", "int x = 0;");
 
-  auto Err = Executor.execute(newFrontendActionFactory(),
-  getClangSyntaxOnlyAdjuster());
+  auto Err = Executor.execute({newFrontendActionFactory(),
+   getClangSyntaxOnly

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 540660.
sammccall added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155361

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/include/clang/Tooling/Execution.h
  clang/include/clang/Tooling/StandaloneExecution.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/Execution.cpp
  clang/lib/Tooling/StandaloneExecution.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ExecutionTest.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -152,13 +153,21 @@
   EXPECT_EQ(1u, Consumer.NumDiagnosticsSeen);
 }
 
-TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromType) {
+TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromActionType) {
   std::unique_ptr Factory(
   newFrontendActionFactory());
   std::unique_ptr Action(Factory->create());
   EXPECT_TRUE(Action.get() != nullptr);
 }
 
+TEST(newFrontendActionFactory, CreatesFrontendActionFactoryFromConsumerType) {
+  class MyASTConsumer : public ASTConsumer {};
+  std::unique_ptr Factory(
+  newFrontendActionFactory());
+  std::unique_ptr Action(Factory->create());
+  EXPECT_TRUE(Action.get() != nullptr);
+}
+
 struct IndependentFrontendActionCreator {
   std::unique_ptr newASTConsumer() {
 return std::make_unique(nullptr);
@@ -503,6 +512,18 @@
 }
 #endif
 
+TEST(newFrontendActionFactory, ConsumeASTs) {
+  FixedCompilationDatabase Compilations(".", std::vector());
+  ClangTool Tool(Compilations, {"a.cc"});
+  Tool.mapVirtualFile("a.cc", "int x = 1;");
+  bool FoundX = false;
+  Tool.run(consumeASTs([&](ASTContext &Ctx) {
+ DeclarationName X(&Ctx.Idents.get("x"));
+ FoundX = Ctx.getTranslationUnitDecl()->lookup(X).isSingleResult();
+   }).get());
+  EXPECT_TRUE(FoundX);
+}
+
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
Index: clang/unittests/Tooling/ExecutionTest.cpp
===
--- clang/unittests/Tooling/ExecutionTest.cpp
+++ clang/unittests/Tooling/ExecutionTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -18,10 +19,12 @@
 #include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -97,9 +100,7 @@
 
   StringRef getExecutorName() const override { return ExecutorName; }
 
-  llvm::Error
-  execute(llvm::ArrayRef,
-   ArgumentsAdjuster>>) override {
+  llvm::Error execute(llvm::ArrayRef) override {
 return llvm::Error::success();
   }
 
@@ -181,14 +182,32 @@
   EXPECT_EQ(Executor->get()->getExecutorName(), TestToolExecutor::ExecutorName);
 }
 
+TEST(CreateToolExecutorTest, ExecuteFromCommandLine) {
+  llvm::unittest::TempFile Source1("test1", ".cpp", "int x = 0;", true);
+  llvm::unittest::TempFile Source2("test2", ".cpp", "int y = 0;", true);
+  std::string Path1 = Source1.path().str();
+  std::string Path2 = Source2.path().str();
+  std::vector argv = {"prog", Path1.c_str(), Path2.c_str()};
+
+  int argc = argv.size();
+  int YCount = 0;
+  int Result = executeFromCommandLineArgs(
+  argc, &argv[0], consumeASTs([&](ASTContext &Ctx) {
+DeclarationName Y(&Ctx.Idents.get("y"));
+YCount += Ctx.getTranslationUnitDecl()->lookup(Y).isSingleResult();
+  }));
+  EXPECT_EQ(Result, 0);
+  EXPECT_EQ(YCount, 1);
+}
+
 TEST(StandaloneToolTest, SynctaxOnlyActionOnSimpleCode) {
   FixedCompilationDatabase Compilations(".", std::vector());
   StandaloneToolExecutor Executor(Compilations,
   std::vector(1, "a.cc"));
   Executor.mapVirtualFile("a.cc", "int x = 0;");
 
-  auto Err = Executor.execute(newFrontendActionFactory(),
-  getClangSyntaxOnlyAdjuster());
+  auto Err = Executor.execute({newFrontendActionFactory(),
+   getClangSyntaxOnlyAdjuster

[PATCH] D155358: [clang-format] Correctly annotate overloaded operator function name

2023-07-15 Thread Emilia Kond via Phabricator via cfe-commits

rymiel accepted this revision.
rymiel added a comment.

Was caused by 3f3620e5c9ee0f7b64afc39e5a26c6f4cc5e7b37 
, thank 
you for fixing it up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155358

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

[PATCH] D153616: [clang][Interp] Create a new local variable in visitLambdaExpr()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153616

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

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:3288-3291
+  assert(
+  (StringToks.size() != 1 ||
+   !tok::isUnexpandableMsMacro(StringToks[0].getKind())) &&
+  "single predefined identifiers shall be handled by ActOnPredefinedExpr");

RIscRIpt wrote:
> tahonermann wrote:
> > What is the reason for requiring non-concatenated predefined function local 
> > macros to be handled by `ActOnPredefinedExpr()`?
> To ensure generation of the same AST as before my changes (with 
> `PredefinedExpr`):
> 
> ```
> |   | `-VarDecl  col:13 a 'const char[2]' cinit
> |   |   `-PredefinedExpr  'const char[2]' __FUNCTION__
> |   | `-StringLiteral  'const char[2]' "f"
> ```
> 
> This actually brings a question: do we want to wrap StringLiterals (into 
> PredefinedExpr) which were formed from concatenation of string-literals and 
> some predefined identifier(s)? But it does not really make any sense: 
> concatenated string does not represent any of PredefinedExpr.
Yes, this make sense. I think it's better than adding more complexity / memory 
footprint to `StringLiteral`.



Comment at: clang/lib/Parse/ParseExpr.cpp:3294-3298
 StringToks.push_back(Tok);
-ConsumeStringToken();
-  } while (isTokenStringLiteral());
+if (isTokenStringLiteral())
+  ConsumeStringToken();
+else
+  ConsumeToken();

RIscRIpt wrote:
> cor3ntin wrote:
> > I think I'd prefer `ConsumeAnyToken` with an assert that checks it's a 
> > stringLiteral or a predefined ms exppression
> Done. But do we really need an assertion here? We have one in the function 
> preamble and strict condition in `while`.
Looking at that again, i think you can remove the assert (the one at the top 
should stay). which you did, excellent. sorry about that!



Comment at: clang/lib/Sema/SemaExpr.cpp:1955-1991
+  // MSVC treats some of predefined identifiers (e.g. __FUNCTION__) as
+  // expandable predefined macros defined as string literals,
+  // which may be concatenated. Expand them here (in Sema),
+  // because StringLiteralParser (in Lex) doesn't have access to AST.
+  std::vector ExpandedToks;
+  if (getLangOpts().MicrosoftExt) {
+ExpandedToks = StringToks.vec();

RIscRIpt wrote:
> cor3ntin wrote:
> > Can we put that logic in a separate function?
> Done. Tho, I couldn't make it `const` (for the same reason I couldn't make 
> `getCurScopeDecl() const`). And I wasn't sure about the interface:
> ```
> std::vector ExpandMSPredefinedMacros(ArrayRef Toks);
> ```
> vs
> ```
> void ExpandMSPredefinedMacros(MutableArrayRef Toks);
> ```
I think the later let you use a small vector so it's probably more efficient. 
I'm find with either



Comment at: clang/lib/Sema/SemaExpr.cpp:1972-1987
+  SmallString<64> Str;
+  llvm::raw_svector_ostream OS(Str);
+  Token Exp;
+  Exp.startToken();
+  if (Tok.getKind() == tok::kw_L__FUNCTION__ ||
+  Tok.getKind() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

RIscRIpt wrote:
> cor3ntin wrote:
> > I think it might be easier to create a string_literal token directly here. 
> > I'm also not sure we need to use `Lexer::Stringify`
> > I think it might be easier to create a string_literal token directly here.
> 
> What do you mean? Is there a function which creates Token object from 
> StringRef? Or is there a way to associate string literal value with a Token 
> without PP? I would like to simplify it, but I haven't found other ways of 
> achieving the same result.
> 
> > I'm also not sure we need to use Lexer::Stringify
> 
> Well, as far as I can see `StringLiteralParser` expands escape sequences. So, 
> I am just being too careful here.
> If not using `Lexer::Stringify` do we want to assert that function name does 
> not contain neither `"` not `\` (which should not happen™)?
Thanks! I really would get rid of `Stringify` here - I struggle to see how a 
function could be produced that has characters that cannot appear in an 
identifier. even asserting isn't very useful.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

RIscRIpt wrote:
> cor3ntin wrote:
> > do we have any tests that look at the values of these things?
> ```
> clang/test/Analysis/eval-predefined-exprs.cpp
> clang/test/AST/Interp/literals.cpp
> clang/test/SemaCXX/source_location.cpp
> clang/test/SemaCXX/predefined-expr.cpp
> ```
I think it's worth add a few more tests in

[PATCH] D154647: [RISCV] Re-define sha256, Zksed, and Zksh intrinsics to use i32 types.

2023-07-15 Thread Xinlong Wu via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154647

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

[PATCH] D154647: [RISCV] Re-define sha256, Zksed, and Zksh intrinsics to use i32 types.

2023-07-15 Thread Xinlong Wu via Phabricator via cfe-commits

VincentWu added a comment.

What about combine the rv32 and rv64 into one file, since they are entire same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154647

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

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

Thanks a lot for working on this.
This probably needs a release note entry




Comment at: clang/lib/Sema/JumpDiagnostics.cpp:790
 // reach this label scope.
 for (auto [JumpScope, JumpStmt] : JumpScopes) {
+  // This unnecessary copy is because:

We don't need to make 3 copies!



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:791-800
+  // This unnecessary copy is because:
+  // warning: captured structured bindings are a C++20 extension
+  // [-Wc++20-extensions]
+  LabelDecl *TL = TargetLabel;
+  // Is TargetLabel one of the targets of the JumpStmt? If not, then skip
+  // it.
+  if (IsAsmGoto &&

I don't think the pointer adds that much, you can init capture by copy which 
simplifies a bit 



Comment at: clang/test/Sema/asm-goto.cpp:71
+l1:;
+}

can you add more tests? Maybe something like that

```cpp
void f() {
  try{
__label__ label;
asm goto("" : : : : label);
label:;
  }
  catch(...){
__label__ label;
asm goto("" : : : : label);
label:;
  };
  if constexpr(false) {
__label__ label;
asm goto("" : : : : label);
label:;
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155342

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

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:791-800
+  // This unnecessary copy is because:
+  // warning: captured structured bindings are a C++20 extension
+  // [-Wc++20-extensions]
+  LabelDecl *TL = TargetLabel;
+  // Is TargetLabel one of the targets of the JumpStmt? If not, then skip
+  // it.
+  if (IsAsmGoto &&

cor3ntin wrote:
> I don't think the pointer adds that much, you can init capture by copy which 
> simplifies a bit 
> I don't think the pointer adds that much, you can init capture by copy which 
> simplifies a bit 

s/pointer/comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155342

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

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

2023-07-15 Thread Tristan Labelle via Phabricator via cfe-commits

MrTrillian added a comment.

> I took a look at the code and it looks to me like it would be safe to change 
> ModuleMap::canonicalizeModuleMapPath() to use a drive preserving 
> canonicalization

Symbolic links can point across drives so I think a drive preserving 
canonicalization cannot be much more than a conversion to an absolute path -- 
similar to the now renamed `abs_path_preserve_drive` I'm adding to lit. I think 
the logic would have to be split across Windows (only abs) and non-Windows 
(realpath) again. I also think a cache miss in this case is ok, so I will 
follow up with a clang change implementing this. Thanks!


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

https://reviews.llvm.org/D154130

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

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

2023-07-15 Thread Tristan Labelle via Phabricator via cfe-commits

MrTrillian added a comment.

> That could impose a significant performance and drive space penalty in the 
> event that substitute drive paths are changed, but I would expect such 
> changes to be rare.

I think the more likely case is that one would build their repo under the 
substitute drive and later build it under the expanded path on the original 
drive. Though I feel like a lot of caching might then break.


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

https://reviews.llvm.org/D154130

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

[PATCH] D151047: [clang-format] Fix indent for selective formatting.

2023-07-15 Thread Sedenion via Phabricator via cfe-commits

Sedeniono marked 4 inline comments as done.
Sedeniono added inline comments.



Comment at: clang/unittests/Format/FormatTestSelective.cpp:641-646
+  Style = getLLVMStyle();
+  Style.FixNamespaceComments = false;
+  Code = "namespace ns {\n"
+ "#define REF(alias) alias alias_var;\n"
+ "}";
+  EXPECT_EQ(Code, format(Code, 51, 1));

owenpan wrote:
> I suppose this would make the purpose of the test more clear. However, this 
> new test (in either version) would pass without the patch, so it doesn't 
> capture the bug mentioned in D151047#4369742?
When I originally submitted the fix for the incorrect selective formatting and 
you committed it to the main branch, some clang rename unit tests failed 
because they ran into some `assert()` in `LevelIndentTracker` (see [this 
post](https://reviews.llvm.org/D151047#4366917)), making a revert necessary. 
There was no test in the format Google Tests themselves that caught that 
mistake. Hence I added this test. But this also means that this test passes 
with and without the current patch. See an [earlier 
comment](https://reviews.llvm.org/D151047#4369742) where I describe the problem 
in more details.

Note that the change in `LineJoiner::join()` (which triggered the problem) is 
no longer in the current version of the patch because of some other unrelated 
changes that happened in the main branch since then. Again, please compare 
[another earlier comment](https://reviews.llvm.org/D151047#4396727).



Comment at: clang/unittests/Format/FormatTestSelective.cpp:649
+
+TEST_F(FormatTestSelective, FormatMacroRegardlessOfPreviousIndent) {
+  // clang-format currently does not (or should not) take into account the

owenpan wrote:
> Again, these tests would pass without the patch.
As above, these tests fail with the original first submitted and incomplete 
fix. As far as I can remember, it happens if you forget the `if 
(!Line.InPPDirective)` condition in the patch. The result is that the PP 
directives end up being formatted "randomly". No other tests in the formatting 
test suite so far checked the selective PP directive formatting (at least I got 
no test failures with the incomplete patch). So yes, the new tests pass with 
and without the patch, but they are still worthwhile.
Also compare [this earlier comment](https://reviews.llvm.org/D151047#4449996).



Comment at: clang/unittests/Format/FormatTestSelective.cpp:658
+
+  Style.IndentPPDirectives = FormatStyle::PPDirectiveIndentStyle::PPDIS_None;
+  EXPECT_EQ("  class Foo {\n"

owenpan wrote:
> It's the default and can be deleted.
I think setting it explicitly makes it clearer which setting is the important 
one that is under test here.



Comment at: clang/unittests/Format/FormatTestSelective.cpp:664
+"#endif\n"   // That this line is also formatted might be a 
bug.
+"}};", // Dito: Bug?
+format("  class Foo {\n"

owenpan wrote:
> Typo.
I cannot see any typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D155367: [clang][Interp] Implement __builtin_inf() etc.

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155367

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp


Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -116,6 +116,15 @@
   return true;
 }
 
+static bool interp__builtin_inf(InterpState &S, CodePtr OpPC,
+const InterpFrame *Frame, const Function *F) {
+  const llvm::fltSemantics &TargetSemantics =
+  S.getCtx().getFloatTypeSemantics(F->getDecl()->getReturnType());
+
+  S.Stk.push(Floating::getInf(TargetSemantics));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -146,6 +155,14 @@
 if (interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/true))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_inf:
+  case Builtin::BI__builtin_inff:
+  case Builtin::BI__builtin_infl:
+  case Builtin::BI__builtin_inff16:
+  case Builtin::BI__builtin_inff128:
+if (interp__builtin_inf(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;


Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -116,6 +116,15 @@
   return true;
 }
 
+static bool interp__builtin_inf(InterpState &S, CodePtr OpPC,
+const InterpFrame *Frame, const Function *F) {
+  const llvm::fltSemantics &TargetSemantics =
+  S.getCtx().getFloatTypeSemantics(F->getDecl()->getReturnType());
+
+  S.Stk.push(Floating::getInf(TargetSemantics));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -146,6 +155,14 @@
 if (interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/true))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_inf:
+  case Builtin::BI__builtin_inff:
+  case Builtin::BI__builtin_infl:
+  case Builtin::BI__builtin_inff16:
+  case Builtin::BI__builtin_inff128:
+if (interp__builtin_inf(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155367: [clang][Interp] Implement __builtin_inf() etc.

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Tests will come in a later commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155367

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

[PATCH] D155368: [clang][Interp] __builtin_copysign

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155368

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp


Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -125,6 +125,38 @@
   return true;
 }
 
+static bool interp__builtin_copysign(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg1 = getParam(Frame, 0);
+  const Floating &Arg2 = getParam(Frame, 1);
+
+  APFloat Copy = Arg1.getAPFloat();
+  Copy.copySign(Arg2.getAPFloat());
+  S.Stk.push(Floating(Copy));
+
+  return true;
+}
+
+static bool interp__builtin_fmin(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame, const Function *F) {
+  const Floating &LHS = getParam(Frame, 0);
+  const Floating &RHS = getParam(Frame, 1);
+
+  Floating Result;
+
+  // When comparing zeroes, return -0.0 if one of the zeroes is negative.
+  if (LHS.isZero() && RHS.isZero() && RHS.isNegative())
+Result = RHS;
+  else if (LHS.isNan() || RHS < LHS)
+Result = RHS;
+  else
+Result = LHS;
+
+  S.Stk.push(Result);
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -163,6 +195,22 @@
 if (interp__builtin_inf(S, OpPC, Frame, F))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_copysign:
+  case Builtin::BI__builtin_copysignf:
+  case Builtin::BI__builtin_copysignl:
+  case Builtin::BI__builtin_copysignf128:
+if (interp__builtin_copysign(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_fmin:
+  case Builtin::BI__builtin_fminf:
+  case Builtin::BI__builtin_fminl:
+  case Builtin::BI__builtin_fminf16:
+  case Builtin::BI__builtin_fminf128:
+if (interp__builtin_fmin(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;


Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -125,6 +125,38 @@
   return true;
 }
 
+static bool interp__builtin_copysign(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg1 = getParam(Frame, 0);
+  const Floating &Arg2 = getParam(Frame, 1);
+
+  APFloat Copy = Arg1.getAPFloat();
+  Copy.copySign(Arg2.getAPFloat());
+  S.Stk.push(Floating(Copy));
+
+  return true;
+}
+
+static bool interp__builtin_fmin(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame, const Function *F) {
+  const Floating &LHS = getParam(Frame, 0);
+  const Floating &RHS = getParam(Frame, 1);
+
+  Floating Result;
+
+  // When comparing zeroes, return -0.0 if one of the zeroes is negative.
+  if (LHS.isZero() && RHS.isZero() && RHS.isNegative())
+Result = RHS;
+  else if (LHS.isNan() || RHS < LHS)
+Result = RHS;
+  else
+Result = LHS;
+
+  S.Stk.push(Result);
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -163,6 +195,22 @@
 if (interp__builtin_inf(S, OpPC, Frame, F))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_copysign:
+  case Builtin::BI__builtin_copysignf:
+  case Builtin::BI__builtin_copysignl:
+  case Builtin::BI__builtin_copysignf128:
+if (interp__builtin_copysign(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_fmin:
+  case Builtin::BI__builtin_fminf:
+  case Builtin::BI__builtin_fminl:
+  case Builtin::BI__builtin_fminf16:
+  case Builtin::BI__builtin_fminf128:
+if (interp__builtin_fmin(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Add a way to pop arguments based on the call expression instead of the function 
and use that.

`clang/test/Sema/constant-builtins-fmin.cpp` is the promised test form the 
other reviews.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155369

Files:
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/Sema/constant-builtins-fmin.cpp

Index: clang/test/Sema/constant-builtins-fmin.cpp
===
--- clang/test/Sema/constant-builtins-fmin.cpp
+++ clang/test/Sema/constant-builtins-fmin.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s
 // expected-no-diagnostics
 
 constexpr double NaN = __builtin_nan("");
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -158,6 +158,20 @@
   return true;
 }
 
+/// Defined as __builtin_isnan(...), to accommodate the fact that it can
+/// take a float, double, long double, etc.
+/// But for us, that's all a Floating anyway.
+static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F) {
+  // FIXME: We are not going through getParam() here, because the function
+  // doesn't have any parameters. Wait for a pattern to emerge and maybe
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -213,6 +227,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isnan:
+if (interp__builtin_isnan(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -200,6 +200,9 @@
 // Returning values
 //===--===//
 
+/// Pop arguments of builtins defined as func-name(...).
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC);
+
 template ::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop();
@@ -216,8 +219,16 @@
   }
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
-S.Current->popArgs();
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
+// Certain builtin functions are declared as func-name(...), so the
+// parameters are checked in Sema and only available through the CallExpr.
+// The interp::Function we create for them has 0 parameters, so we need to
+// remove them from the stack by checking the CallExpr.
+if (S.Current && S.Current->getFunction()->needsRuntimeArgPop())
+  popBuiltinArgs(S, PC);
+else
+  S.Current->popArgs();
+  }
 
   if (InterpFrame *Caller = S.Current->Caller) {
 PC = S.Current->getRetPC();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -122,6 +122,18 @@
 namespace clang {
 namespace interp {
 
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC) {
+  assert(S.Current && S.Current->getFunction()->needsRuntimeArgPop());
+  const Expr *E = S.Current->getExpr(OpPC);
+  assert(isa(E));
+  const CallExpr *CE = cast(E);
+  for (const auto &A : CE->arguments()) {
+PrimType Ty = S.getContext().classify(A->getType()).value_or(PT_Ptr);
+TYPE_SWITCH(Ty, S.Stk.discard());
+  }
+  return true;
+}
+
 bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   if (!Ptr.isExtern())
 return true;
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -171,6 +171,17 @@
 
   unsigned getBuiltinID() const { return F->getBuiltinID(); }
 
+  bool isBuiltin() const { return F->getBuiltinID() != 0; }
+
+  bool needsRuntimeArgPop() const {
+if (!isBuiltin())
+  return false;
+// FIXME: Is there a better way to get this information?
+if (getName() == "__builtin_isnan

[PATCH] D155094: Refactoring and asserts in LevelIndentTracker. (NFC)

2023-07-15 Thread Sedenion via Phabricator via cfe-commits

Sedeniono updated this revision to Diff 540682.
Sedeniono added a comment.

Implemented review changes suggested by @owenpan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155094

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -88,14 +88,15 @@
   /// level to the same indent.
   /// Note that \c nextLine must have been called before this method.
   void adjustToUnmodifiedLine(const AnnotatedLine &Line) {
+if (Line.InPPDirective)
+  return;
+assert(Line.Level < IndentForLevel.size());
+if (Line.First->is(tok::comment) && IndentForLevel[Line.Level] != -1)
+  return;
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
-assert(Line.Level < IndentForLevel.size());
-if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
-!Line.InPPDirective) {
-  IndentForLevel[Line.Level] = LevelIndent;
-}
+IndentForLevel[Line.Level] = LevelIndent;
   }
 
 private:
@@ -148,6 +149,7 @@
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
   unsigned getIndent(unsigned Level) const {
+assert(Level < IndentForLevel.size());
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
@@ -159,7 +161,10 @@
   const AdditionalKeywords &Keywords;
   const unsigned AdditionalIndent;
 
-  /// The indent in characters for each level.
+  /// The indent in characters for each level. It remembers the indent of
+  /// previous lines (that are not PP directives) of equal or lower levels. 
This
+  /// is used to align formatted lines to the indent of previous non-formatted
+  /// lines. Think about the --lines parameter of clang-format.
   SmallVector IndentForLevel;
 
   /// Offset of the current line relative to the indent level.


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -88,14 +88,15 @@
   /// level to the same indent.
   /// Note that \c nextLine must have been called before this method.
   void adjustToUnmodifiedLine(const AnnotatedLine &Line) {
+if (Line.InPPDirective)
+  return;
+assert(Line.Level < IndentForLevel.size());
+if (Line.First->is(tok::comment) && IndentForLevel[Line.Level] != -1)
+  return;
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
-assert(Line.Level < IndentForLevel.size());
-if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
-!Line.InPPDirective) {
-  IndentForLevel[Line.Level] = LevelIndent;
-}
+IndentForLevel[Line.Level] = LevelIndent;
   }
 
 private:
@@ -148,6 +149,7 @@
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
   unsigned getIndent(unsigned Level) const {
+assert(Level < IndentForLevel.size());
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
@@ -159,7 +161,10 @@
   const AdditionalKeywords &Keywords;
   const unsigned AdditionalIndent;
 
-  /// The indent in characters for each level.
+  /// The indent in characters for each level. It remembers the indent of
+  /// previous lines (that are not PP directives) of equal or lower levels. This
+  /// is used to align formatted lines to the indent of previous non-formatted
+  /// lines. Think about the --lines parameter of clang-format.
   SmallVector IndentForLevel;
 
   /// Offset of the current line relative to the indent level.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151047: [clang-format] Fix indent for selective formatting.

2023-07-15 Thread Sedenion via Phabricator via cfe-commits

Sedeniono updated this revision to Diff 540683.
Sedeniono marked 4 inline comments as done.
Sedeniono added a comment.

Fixed typo in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp

Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@
 format("  int a;\n"
"void ff() {}",
11, 0));
+
+  // https://github.com/llvm/llvm-project/issues/59178
+  Style = getMozillaStyle();
+  EXPECT_EQ("int a()\n"
+"{\n"
+"return 0;\n"
+"}\n"
+"int b()\n"
+"{\n"
+"  return 42;\n"
+"}",
+format("int a()\n"
+   "{\n"
+   "return 0;\n"
+   "}\n"
+   "int b()\n"
+   "{\n"
+   "return 42;\n" // Format this line only
+   "}",
+   32, 0));
 }
 
 TEST_F(FormatTestSelective, UnderstandsTabs) {
@@ -617,6 +637,70 @@
  "namespace ns1 { namespace ns2 {\n"
  "}}";
   EXPECT_EQ(Code, format(Code, 0, 0));
+
+  Style = getLLVMStyle();
+  Style.FixNamespaceComments = false;
+  Code = "namespace ns {\n"
+ "#define REF(alias) alias alias_var;\n"
+ "}";
+  EXPECT_EQ(Code, format(Code, 51, 1));
+}
+
+TEST_F(FormatTestSelective, FormatMacroRegardlessOfPreviousIndent) {
+  // clang-format currently does not (or should not) take into account the
+  // indent of previous unformatted lines when formatting a PP directive.
+  // Technically speaking, LevelIndentTracker::IndentForLevel is only for non-PP
+  // lines. So these tests here check that the indent of previous non-PP lines
+  // do not affect the formatting. If this requirement changes, the tests here
+  // need to be adapted.
+  Style = getLLVMStyle();
+
+  Style.IndentPPDirectives = FormatStyle::PPDirectiveIndentStyle::PPDIS_None;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"#define some\n" // Formatted line
+"#endif\n"   // That this line is also formatted might be a bug.
+"}};", // Ditto: Bug?
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
+
+  Style.IndentPPDirectives =
+  FormatStyle::PPDirectiveIndentStyle::PPDIS_BeforeHash;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"  #define some\n" // Formatted line
+" #endif\n"
+"}};",
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
+
+  Style.IndentPPDirectives =
+  FormatStyle::PPDirectiveIndentStyle::PPDIS_AfterHash;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"#  define some\n" // Formatted line
+"#endif\n" // That this line is also formatted might be a bug.
+"}};",
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -74,6 +74,13 @@
: Line.Level * PPIndentWidth;
   Indent += AdditionalIndent;
 } else {
+  // When going to lower levels, forget previous higher levels so that we
+  // recompute future higher levels. But don't forget them if we enter a PP
+  // directive, since these do not terminate a C++ code block.
+  if (!Line.InPPDirective) {
+assert(Line.Level <= IndentForLevel.size());
+IndentForLevel.resize(Line.Level + 1);
+  }
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
___
cfe

[PATCH] D151047: [clang-format] Fix indent for selective formatting.

2023-07-15 Thread Sedenion via Phabricator via cfe-commits

Sedeniono added inline comments.



Comment at: clang/unittests/Format/FormatTestSelective.cpp:664
+"#endif\n"   // That this line is also formatted might be a 
bug.
+"}};", // Dito: Bug?
+format("  class Foo {\n"

Sedeniono wrote:
> owenpan wrote:
> > Typo.
> I cannot see any typo.
Oh, ok, ditto, not dito. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D155204: [clang][dataflow] Add `refreshStructValue()`.

2023-07-15 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:706-711
+/// This function is primarily intended for use by checks that set custom
+/// properties on `StructValue`s to model the state of these values. Such 
checks
+/// should avoid modifying the properties of an existing `StructValue` because
+/// these changes would be visible to other `Environment`s that share the same
+/// `StructValue`. Instead, call `refreshStructValue()`, then set the 
properties
+/// on the new `StructValue` that it returns. Typical usage:

I think this is fine for now, but I wonder if we should come up with an API 
where errors like this cannot happen. One potential way would be to no longer 
include these properties in the `StructValue`s, but have a separate mapping 
`StructValue => Properties`. So, one can update the mapping in an environment 
without any unintended consequences in other environments. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155204

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

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/AST/Interp/Function.h:180
+// FIXME: Is there a better way to get this information?
+if (getName() == "__builtin_isnan")
+  return true;

You can compare getBuiltinID(), which is better than using strings, it's what 
we seem to be doing elsewhere



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:169
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek();
+

There are only a few functions created by SemaBuiltinFPClassification - only 
two of which are not unary. this looks fine, fpclassify and isfpclass can have 
their own logic.
I don't suppose there is a way to assert we have one argument there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155369

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

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-15 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:2247
bool DryRun, bool Strict) {
-  std::unique_ptr Token =
+  std::unique_ptr Token =
   createBreakableToken(Current, State, AllowBreak);

This const can be readded, or not?



Comment at: clang/lib/Format/WhitespaceManager.cpp:1428
+  // case, it is allowed if there is a replacement for the empty range
+  // between 2 tokens and another non-empty range at the start of the 
second
+  // token.

Can you elaborate where the 2 replacements come from, and why we can't make 
sure there is no empty range?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-15 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:4220
   /// \version 3.7
   bool SpacesInParentheses;
 

The deprecated options should be removed from the struct, see 
`AllowAllConstructorInitializersOnNextLine` for an example.

You also need to adapt the parsing logic a bit.



Comment at: clang/lib/Format/Format.cpp:735
+
+// For backward compatibility.
+IO.enumCase(Value, "false", FormatStyle::SIPO_Never);

Since the option is new, there is no backward. You can drop this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155239

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

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Function.h:180
+// FIXME: Is there a better way to get this information?
+if (getName() == "__builtin_isnan")
+  return true;

cor3ntin wrote:
> You can compare getBuiltinID(), which is better than using strings, it's what 
> we seem to be doing elsewhere
Yeah but that still sucks. Is there no way to query whether the builtin has `t` 
set ("signature is meaningless, use custom typechecking"), regardless of the ID 
or name?



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:169
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek();
+

cor3ntin wrote:
> There are only a few functions created by SemaBuiltinFPClassification - only 
> two of which are not unary. this looks fine, fpclassify and isfpclass can 
> have their own logic.
> I don't suppose there is a way to assert we have one argument there?
We could (and maybe want) to pass along the `CallExpr`, but the arguments and 
their number are already checked in `Sema`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155369

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

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/AST/Interp/Function.h:180
+// FIXME: Is there a better way to get this information?
+if (getName() == "__builtin_isnan")
+  return true;

tbaeder wrote:
> cor3ntin wrote:
> > You can compare getBuiltinID(), which is better than using strings, it's 
> > what we seem to be doing elsewhere
> Yeah but that still sucks. Is there no way to query whether the builtin has 
> `t` set ("signature is meaningless, use custom typechecking"), regardless of 
> the ID or name?
`AstContext.BuiltinInfo.hasCustomTypechecking(F->getBuiltinID()))` seems to do 
that but i have no clue if you could use it on all functions without running 
into issues



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155369

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

[PATCH] D155371: [clang][Interp] Implement __builtin_isinf

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155371

Files:
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -62,3 +62,8 @@
   constexpr float Nan2 = __builtin_nans([](){return "0xAE98";}()); // 
ref-error {{must be initialized by a constant expression}}
 
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -172,6 +172,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -232,6 +246,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -177,7 +177,7 @@
 if (!isBuiltin())
   return false;
 // FIXME: Is there a better way to get this information?
-if (getName() == "__builtin_isnan")
+if (getName() == "__builtin_isnan" || getName() == "__builtin_isinf")
   return true;
 return false;
   }
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -94,6 +94,7 @@
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -62,3 +62,8 @@
   constexpr float Nan2 = __builtin_nans([](){return "0xAE98";}()); // ref-error {{must be initialized by a constant expression}}
 
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -172,6 +172,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -232,6 +246,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Function.h
==

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 540692.

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

https://reviews.llvm.org/D155369

Files:
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/Sema/constant-builtins-fmin.cpp

Index: clang/test/Sema/constant-builtins-fmin.cpp
===
--- clang/test/Sema/constant-builtins-fmin.cpp
+++ clang/test/Sema/constant-builtins-fmin.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s
 // expected-no-diagnostics
 
 constexpr double NaN = __builtin_nan("");
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -158,6 +158,20 @@
   return true;
 }
 
+/// Defined as __builtin_isnan(...), to accommodate the fact that it can
+/// take a float, double, long double, etc.
+/// But for us, that's all a Floating anyway.
+static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F) {
+  // FIXME: We are not going through getParam() here, because the function
+  // doesn't have any parameters. Wait for a pattern to emerge and maybe
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -213,6 +227,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isnan:
+if (interp__builtin_isnan(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -200,6 +200,9 @@
 // Returning values
 //===--===//
 
+/// Pop arguments of builtins defined as func-name(...).
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC);
+
 template ::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop();
@@ -216,8 +219,16 @@
   }
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
-S.Current->popArgs();
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
+// Certain builtin functions are declared as func-name(...), so the
+// parameters are checked in Sema and only available through the CallExpr.
+// The interp::Function we create for them has 0 parameters, so we need to
+// remove them from the stack by checking the CallExpr.
+if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+  popBuiltinArgs(S, PC);
+else
+  S.Current->popArgs();
+  }
 
   if (InterpFrame *Caller = S.Current->Caller) {
 PC = S.Current->getRetPC();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -122,6 +122,18 @@
 namespace clang {
 namespace interp {
 
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC) {
+  assert(S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()));
+  const Expr *E = S.Current->getExpr(OpPC);
+  assert(isa(E));
+  const CallExpr *CE = cast(E);
+  for (const auto &A : CE->arguments()) {
+PrimType Ty = S.getContext().classify(A->getType()).value_or(PT_Ptr);
+TYPE_SWITCH(Ty, S.Stk.discard());
+  }
+  return true;
+}
+
 bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   if (!Ptr.isExtern())
 return true;
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -171,6 +171,12 @@
 
   unsigned getBuiltinID() const { return F->getBuiltinID(); }
 
+  bool isBuiltin() const { return F->getBuiltinID() != 0; }
+
+  /// Does this function need its arguments to be classified at runtime
+  /// rather than at bytecode-compile-time?
+  bool needsRuntimeArgPop(const ASTContext &Ctx) const;
+
   unsigned getNumParams() const { return ParamTypes.size(); }
 
   unsigned getParamOffset(unsigned ParamIndex) const {
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/Interp/Function.cpp
+++ clang/lib/AST/Interp/Function.cpp
@

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder marked 2 inline comments as done.
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Function.h:180
+// FIXME: Is there a better way to get this information?
+if (getName() == "__builtin_isnan")
+  return true;

cor3ntin wrote:
> tbaeder wrote:
> > cor3ntin wrote:
> > > You can compare getBuiltinID(), which is better than using strings, it's 
> > > what we seem to be doing elsewhere
> > Yeah but that still sucks. Is there no way to query whether the builtin has 
> > `t` set ("signature is meaningless, use custom typechecking"), regardless 
> > of the ID or name?
> `AstContext.BuiltinInfo.hasCustomTypechecking(F->getBuiltinID()))` seems to 
> do that but i have no clue if you could use it on all functions without 
> running into issues
> 
That works indeed, thanks.


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

https://reviews.llvm.org/D155369

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

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

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

(this was reverted, I want to reland sometime, but there's a bad interaction 
with a bug in nullability analysis)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153485

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

[PATCH] D155371: [clang][Interp] Implement __builtin_isinf

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 540693.

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

https://reviews.llvm.org/D155371

Files:
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -62,3 +62,8 @@
   constexpr float Nan2 = __builtin_nans([](){return "0xAE98";}()); // 
ref-error {{must be initialized by a constant expression}}
 
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -172,6 +172,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -232,6 +246,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -94,6 +94,7 @@
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -62,3 +62,8 @@
   constexpr float Nan2 = __builtin_nans([](){return "0xAE98";}()); // ref-error {{must be initialized by a constant expression}}
 
 }
+
+namespace inf {
+  static_assert(__builtin_isinf(__builtin_inf()), "");
+  static_assert(!__builtin_isinf(1.0), "");
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -172,6 +172,20 @@
   return true;
 }
 
+static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F,
+  bool CheckSign) {
+  const Floating &Arg = S.Stk.peek();
+  bool IsInf = Arg.isInf();
+
+  if (CheckSign)
+S.Stk.push>(
+Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+  else
+S.Stk.push>(Integral<32, true>::from(Arg.isInf()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -232,6 +246,16 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isinf:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+  return Ret(S, OpPC, Dummy);
+break;
+
+  case Builtin::BI__builtin_isinf_sign:
+if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -94,6 +94,7 @@
   bool isMin() const { return F.isSmallest(); }
   bool isMinusOne() const { return F.isExactlyValue(-1.0); }
   bool isNan() const { return F.isNaN(); }
+  bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155372: [clang][Interp] Implement __builtin_isfinite

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155372

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -66,4 +66,7 @@
 namespace inf {
   static_assert(__builtin_isinf(__builtin_inf()), "");
   static_assert(!__builtin_isinf(1.0), "");
+
+  static_assert(__builtin_isfinite(1.0));
+  static_assert(!__builtin_isfinite(__builtin_inf()));
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -186,6 +186,15 @@
   return true;
 }
 
+static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -256,6 +265,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isfinite:
+if (interp__builtin_isfinite(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -66,4 +66,7 @@
 namespace inf {
   static_assert(__builtin_isinf(__builtin_inf()), "");
   static_assert(!__builtin_isinf(1.0), "");
+
+  static_assert(__builtin_isfinite(1.0));
+  static_assert(!__builtin_isfinite(__builtin_inf()));
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -186,6 +186,15 @@
   return true;
 }
 
+static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -256,6 +265,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isfinite:
+if (interp__builtin_isfinite(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Sorry about the delay here.

Making the whole AutoTypeLoc resolve to the concept doesn't seem right - the 
`auto` part does not refer to the concept, and in principle refers to another 
type entirely.

Really I think there should be a child node representing just the concept part, 
probably a ConceptReference.

Compared to the case we discussed where constrained auto is a template type 
param, this looks easier: all the information is there in AutoTypeLoc.
I think we need RecursiveASTVisitor to assemble it into a ConceptReference, and 
(to use it with SelectionTree) we need the ability to store it in a 
DynTypedNode.




Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:581
+
+[[Fooable]] auto i = 42;
+  )cpp";

this is going to have the same behavior on the `auto` token, right?

This is my main practical concern, that go-to-definition, hover, find-refs, 
go-to-type etc on `auto` will now treat `Fooable` as their target.

(That said, I'm not sure exactly how common it is for `auto` to be constrained 
in a non-dependent context...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154853

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

[PATCH] D155372: [clang][Interp] Implement __builtin_isfinite

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 540699.

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

https://reviews.llvm.org/D155372

Files:
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -66,4 +66,7 @@
 namespace inf {
   static_assert(__builtin_isinf(__builtin_inf()), "");
   static_assert(!__builtin_isinf(1.0), "");
+
+  static_assert(__builtin_isfinite(1.0), "");
+  static_assert(!__builtin_isfinite(__builtin_inf()), "");
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -186,6 +186,15 @@
   return true;
 }
 
+static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -256,6 +265,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isfinite:
+if (interp__builtin_isfinite(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -66,4 +66,7 @@
 namespace inf {
   static_assert(__builtin_isinf(__builtin_inf()), "");
   static_assert(!__builtin_isinf(1.0), "");
+
+  static_assert(__builtin_isfinite(1.0), "");
+  static_assert(!__builtin_isfinite(__builtin_inf()), "");
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -186,6 +186,15 @@
   return true;
 }
 
+static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -256,6 +265,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isfinite:
+if (interp__builtin_isfinite(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155374: [clang][Interp] Implement __builtin_isnormal

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155374

Files:
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -69,4 +69,7 @@
 
   static_assert(__builtin_isfinite(1.0), "");
   static_assert(!__builtin_isfinite(__builtin_inf()), "");
+
+  static_assert(__builtin_isnormal(1.0), "");
+  static_assert(!__builtin_isnormal(__builtin_inf()), "");
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -195,6 +195,15 @@
   return true;
 }
 
+static bool interp__builtin_isnormal(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isNormal()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -269,6 +278,10 @@
 if (interp__builtin_isfinite(S, OpPC, Frame, F))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_isnormal:
+if (interp__builtin_isnormal(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -96,6 +96,7 @@
   bool isNan() const { return F.isNaN(); }
   bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
+  bool isNormal() const { return F.isNormal(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {
 return Compare(F, RHS.F);


Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -69,4 +69,7 @@
 
   static_assert(__builtin_isfinite(1.0), "");
   static_assert(!__builtin_isfinite(__builtin_inf()), "");
+
+  static_assert(__builtin_isnormal(1.0), "");
+  static_assert(!__builtin_isnormal(__builtin_inf()), "");
 }
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -195,6 +195,15 @@
   return true;
 }
 
+static bool interp__builtin_isnormal(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *F) {
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isNormal()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -269,6 +278,10 @@
 if (interp__builtin_isfinite(S, OpPC, Frame, F))
   return Ret(S, OpPC, Dummy);
 break;
+  case Builtin::BI__builtin_isnormal:
+if (interp__builtin_isnormal(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
 
   default:
 return false;
Index: clang/lib/AST/Interp/Floating.h
===
--- clang/lib/AST/Interp/Floating.h
+++ clang/lib/AST/Interp/Floating.h
@@ -96,6 +96,7 @@
   bool isNan() const { return F.isNaN(); }
   bool isInf() const { return F.isInfinity(); }
   bool isFinite() const { return F.isFinite(); }
+  bool isNormal() const { return F.isNormal(); }
 
   ComparisonCategoryResult compare(const Floating &RHS) const {
 return Compare(F, RHS.F);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cc2ff02 - [clang-format] Correctly annotate overloaded operator function name

2023-07-15 Thread Owen Pan via cfe-commits

Author: Owen Pan
Date: 2023-07-15T11:02:31-07:00
New Revision: cc2ff02eadede988ab890b2d0f428bd1531a3803

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

LOG: [clang-format] Correctly annotate overloaded operator function name

The operator keyword preceded by a template closer should be annotated as
TT_FunctionDeclarationName.

Fixes #63879.

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 52973a3ec95171..fbcc3a03ba56ae 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3175,7 +3175,7 @@ static bool isFunctionDeclarationName(bool IsCpp, const 
FormatToken &Current,
 !Previous->isOneOf(tok::kw_return, tok::kw_co_return)) {
   return true;
 }
-if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp, 
TT_TemplateCloser))
   return false;
 Next = skipOperatorName(Next);
   } else {

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 8767b97c94de27..fd42dfded83e5f 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -717,6 +717,16 @@ TEST_F(TokenAnnotatorTest, UnderstandsOverloadedOperators) 
{
   EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
   EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("std::vector operator()(Foo &foo);");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[5], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[6], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {



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

[PATCH] D155358: [clang-format] Correctly annotate overloaded operator function name

2023-07-15 Thread Owen Pan via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc2ff02eaded: [clang-format] Correctly annotate overloaded 
operator function name (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155358

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -717,6 +717,16 @@
   EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
   EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("std::vector operator()(Foo &foo);");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[5], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[6], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3175,7 +3175,7 @@
 !Previous->isOneOf(tok::kw_return, tok::kw_co_return)) {
   return true;
 }
-if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp, 
TT_TemplateCloser))
   return false;
 Next = skipOperatorName(Next);
   } else {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -717,6 +717,16 @@
   EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
   EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("std::vector operator()(Foo &foo);");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[5], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[6], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3175,7 +3175,7 @@
 !Previous->isOneOf(tok::kw_return, tok::kw_co_return)) {
   return true;
 }
-if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+if (!Previous->isOneOf(tok::star, tok::amp, tok::ampamp, TT_TemplateCloser))
   return false;
 Next = skipOperatorName(Next);
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 599421a - [RISCV] Use unsigned instead of signed types for Zk* and Zb* builtins.

2023-07-15 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2023-07-15T11:19:18-07:00
New Revision: 599421ae36c332474c5034b4baaab59833e76418

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

LOG: [RISCV] Use unsigned instead of signed types for Zk* and Zb* builtins.

Unsigned is a better representation for bitmanipulation and cryptography.w

The only exception being the return values for clz and ctz intrinsics is
a signed int. That matches the target independent clz and ctz builtins.

This is consistent with the current scalar crypto proposal
https://github.com/riscv-non-isa/riscv-c-api-doc/pull/44

Reviewed By: VincentWu

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsRISCV.def
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkb.c
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkx.c
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb-error.c
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb.c
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkx.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknd.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zkne.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd-zkne.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zkne.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsRISCV.def 
b/clang/include/clang/Basic/BuiltinsRISCV.def
index 98c86633c709d5..0e157a0f5beb2e 100644
--- a/clang/include/clang/Basic/BuiltinsRISCV.def
+++ b/clang/include/clang/Basic/BuiltinsRISCV.def
@@ -32,58 +32,58 @@ TARGET_BUILTIN(__builtin_riscv_clmulr_32, "UiUiUi", "nc", 
"zbc,32bit")
 TARGET_BUILTIN(__builtin_riscv_clmulr_64, "UWiUWiUWi", "nc", "zbc,64bit")
 
 // Zbkx
-TARGET_BUILTIN(__builtin_riscv_xperm4_32, "iii", "nc", "zbkx,32bit")
-TARGET_BUILTIN(__builtin_riscv_xperm4_64, "WiWiWi", "nc", "zbkx,64bit")
-TARGET_BUILTIN(__builtin_riscv_xperm8_32, "iii", "nc", "zbkx,32bit")
-TARGET_BUILTIN(__builtin_riscv_xperm8_64, "WiWiWi", "nc", "zbkx,64bit")
+TARGET_BUILTIN(__builtin_riscv_xperm4_32, "UiUiUi", "nc", "zbkx,32bit")
+TARGET_BUILTIN(__builtin_riscv_xperm4_64, "UWiUWiUWi", "nc", "zbkx,64bit")
+TARGET_BUILTIN(__builtin_riscv_xperm8_32, "UiUiUi", "nc", "zbkx,32bit")
+TARGET_BUILTIN(__builtin_riscv_xperm8_64, "UWiUWiUWi", "nc", "zbkx,64bit")
 
 // Zbkb extension
-TARGET_BUILTIN(__builtin_riscv_brev8_32, "ii", "nc", "zbkb")
-TARGET_BUILTIN(__builtin_riscv_brev8_64, "WiWi", "nc", "zbkb,64bit")
-TARGET_BUILTIN(__builtin_riscv_zip_32, "ZiZi", "nc", "zbkb,32bit")
-TARGET_BUILTIN(__builtin_riscv_unzip_32, "ZiZi", "nc", "zbkb,32bit")
+TARGET_BUILTIN(__builtin_riscv_brev8_32, "UiUi", "nc", "zbkb")
+TARGET_BUILTIN(__builtin_riscv_brev8_64, "UWiUWi", "nc", "zbkb,64bit")
+TARGET_BUILTIN(__builtin_riscv_zip_32, "UiUi", "nc", "zbkb,32bit")
+TARGET_BUILTIN(__builtin_riscv_unzip_32, "UiUi", "nc", "zbkb,32bit")
 
 // Zknd extension
-TARGET_BUILTIN(__builtin_riscv_aes32dsi_32, "ZiZiZiIUi", "nc", "zknd,32bit")
-TARGET_BUILTIN(__builtin_riscv_aes32dsmi_32, "ZiZiZiIUi", "nc", "zknd,32bit")
-TARGET_BUILTIN(__builtin_riscv_aes64ds_64, "WiWiWi", "nc", "zknd,64bit")
-TARGET_BUILTIN(__builtin_riscv_aes64dsm_64, "WiWiWi", "nc", "zknd,64bit")
-TARGET_BUILTIN(__builtin_riscv_aes64im_64, "WiWi", "nc", "zknd,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes32dsi_32, "UiUiUiIUi", "nc", "zknd,32bit")
+TARGET_BUILTIN(__builtin_riscv_aes32dsmi_32, "UiUiUiIUi", "nc", "zknd,32bit")
+TARGET_BUILTIN(__builtin_riscv_aes64ds_64, "UWiUWiUWi", "nc", "zknd,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes64dsm_64, "UWiUWiUWi", "nc", "zknd,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes64im_64, "UWiUWi", "nc", "zknd,64bit")
 
 // Zknd & zkne
-TARGET_BUILTIN(__builtin_riscv_aes64ks1i_64, "WiWiIUi", "nc", 
"zknd|zkne,64bit")
-TARGET_BUILTIN(__builtin_riscv_aes64ks2_64, "WiWiWi", "nc", "zknd|zkne,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes64ks1i_64, "UWiUWiIUi", "nc", 
"zknd|zkne,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes64ks2_64, "UWiUWiUWi", "nc", 
"zknd|zkne,64bit")
 
 // Zkne extension
-TARGET_BUILTIN(__builtin_riscv_aes32esi_32, "ZiZiZiIUi", "nc", "zkne,32bit")
-TARGET_BUILTIN(__builtin_riscv_aes32esmi_32, "ZiZiZiIUi", "nc", "zkne,32bit")
-TARGET_BUILTIN(__builtin_riscv_aes64es_64, "WiWiWi", "nc", "zkne,64bit")
-TARGET_BUILTIN(__builtin_riscv_aes64esm_64, "WiWiWi", "nc", "zkne,64bit")
+TARGET_BUILTIN(__builtin_riscv_aes3

[PATCH] D154616: [RISCV] Use unsigned instead of signed types for Zk* and Zb* builtins.

2023-07-15 Thread Craig Topper via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG599421ae36c3: [RISCV] Use unsigned instead of signed types 
for Zk* and Zb* builtins. (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154616

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkx.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb-error.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkx.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c

Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
@@ -10,7 +10,7 @@
 // RV64ZKSH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sm3p0.i64(i64 [[TMP0]])
 // RV64ZKSH-NEXT:ret i64 [[TMP1]]
 //
-long sm3p0(long rs1) {
+unsigned long sm3p0(unsigned long rs1) {
   return __builtin_riscv_sm3p0(rs1);
 }
 
@@ -23,6 +23,6 @@
 // RV64ZKSH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sm3p1.i64(i64 [[TMP0]])
 // RV64ZKSH-NEXT:ret i64 [[TMP1]]
 //
-long sm3p1(long rs1) {
+unsigned long sm3p1(unsigned long rs1) {
   return __builtin_riscv_sm3p1(rs1);
 }
Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
@@ -13,7 +13,7 @@
 // RV64ZKSED-NEXT:[[TMP2:%.*]] = call i64 @llvm.riscv.sm4ks.i64(i64 [[TMP0]], i64 [[TMP1]], i32 0)
 // RV64ZKSED-NEXT:ret i64 [[TMP2]]
 //
-long sm4ks(long rs1, long rs2) {
+unsigned long sm4ks(unsigned long rs1, unsigned long rs2) {
   return __builtin_riscv_sm4ks(rs1, rs2, 0);
 }
 
@@ -28,6 +28,6 @@
 // RV64ZKSED-NEXT:[[TMP2:%.*]] = call i64 @llvm.riscv.sm4ed.i64(i64 [[TMP0]], i64 [[TMP1]], i32 0)
 // RV64ZKSED-NEXT:ret i64 [[TMP2]]
 //
-long sm4ed(long rs1, long rs2) {
+unsigned long sm4ed(unsigned long rs1, unsigned long rs2) {
   return __builtin_riscv_sm4ed(rs1, rs2, 0);
 }
Index: clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
===
--- clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
+++ clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple riscv64 -target-feature +zknh -emit-llvm %s -o - \
 // RUN: | FileCheck %s  -check-prefix=RV64ZKNH
 
+#include 
 
 // RV64ZKNH-LABEL: @sha512sig0(
 // RV64ZKNH-NEXT:  entry:
@@ -11,7 +12,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sig0(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sig0(long rs1) {
+uint64_t sha512sig0(uint64_t rs1) {
   return __builtin_riscv_sha512sig0_64(rs1);
 }
 
@@ -24,7 +25,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sig1(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sig1(long rs1) {
+uint64_t sha512sig1(uint64_t rs1) {
   return __builtin_riscv_sha512sig1_64(rs1);
 }
 
@@ -37,7 +38,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sum0(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sum0(long rs1) {
+uint64_t sha512sum0(uint64_t rs1) {
   return __builtin_riscv_sha512sum0_64(rs1);
 }
 
@@ -50,7 +51,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha512sum1(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha512sum1(long rs1) {
+uint64_t sha512sum1(uint64_t rs1) {
   return __builtin_riscv_sha512sum1_64(rs1);
 }
 
@@ -63,7 +64,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha256sig0.i64(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha256sig0(long rs1) {
+uint64_t sha256sig0(uint64_t rs1) {
   return __builtin_riscv_sha256sig0(rs1);
 }
 
@@ -75,7 +76,7 @@
 // RV64ZKNH-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.sha256sig1.i64(i64 [[TMP0]])
 // RV64ZKNH-NEXT:ret i64 [[TMP1]]
 //
-long sha256sig1(long rs1) {
+uint64_t sha256sig1(uint64_t rs1) {
   return __builtin_riscv_sha256sig1(rs1);
 }
 
@@ -88,7 +8

[PATCH] D154647: [RISCV] Re-define sha256, Zksed, and Zksh intrinsics to use i32 types.

2023-07-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper updated this revision to Diff 540714.
craig.topper added a comment.

Rebase. Will merge tests next.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154647

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVInstrInfoZk.td
  llvm/test/CodeGen/RISCV/rv32zknh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv32zksed-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv32zksh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zknh-intrinsic-autoupgrade.ll
  llvm/test/CodeGen/RISCV/rv64zknh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zksed-intrinsic-autoupgrade2.ll
  llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zksh-intrinsic-autoupgrade.ll
  llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
  llvm/test/CodeGen/RISCV/sextw-removal.ll

Index: llvm/test/CodeGen/RISCV/sextw-removal.ll
===
--- llvm/test/CodeGen/RISCV/sextw-removal.ll
+++ llvm/test/CodeGen/RISCV/sextw-removal.ll
@@ -1319,13 +1319,11 @@
 ; NOREMOVAL-NEXT:addi sp, sp, 32
 ; NOREMOVAL-NEXT:ret
 bb:
-  %sext = sext i32 %arg1 to i64
-  %i = call i64 @llvm.riscv.sha256sig0.i64(i64 %sext)
-  %trunc = trunc i64 %i to i32
+  %i = call i32 @llvm.riscv.sha256sig0(i32 %arg1)
   br label %bb2
 
 bb2:  ; preds = %bb2, %bb
-  %i3 = phi i32 [ %trunc, %bb ], [ %i5, %bb2 ]
+  %i3 = phi i32 [ %i, %bb ], [ %i5, %bb2 ]
   %i4 = tail call signext i32 @bar(i32 signext %i3)
   %i5 = shl i32 %i3, %arg1
   %i6 = icmp eq i32 %i4, 0
@@ -1334,7 +1332,7 @@
 bb7:  ; preds = %bb2
   ret void
 }
-declare i64 @llvm.riscv.sha256sig0.i64(i64)
+declare i32 @llvm.riscv.sha256sig0(i32)
 
 ; The type promotion of %7 forms a sext_inreg, but %7 and %6 are combined to
 ; form a sh2add. This leaves behind a sext.w that isn't needed.
Index: llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
===
--- llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
+++ llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
@@ -2,24 +2,24 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zksh -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s -check-prefix=RV64ZKSH
 
-declare i64 @llvm.riscv.sm3p0.i64(i64);
+declare i32 @llvm.riscv.sm3p0(i32);
 
-define i64 @sm3p0_i64(i64 %a) nounwind {
-; RV64ZKSH-LABEL: sm3p0_i64:
+define signext i32 @sm3p0_i32(i32 signext %a) nounwind {
+; RV64ZKSH-LABEL: sm3p0_i32:
 ; RV64ZKSH:   # %bb.0:
 ; RV64ZKSH-NEXT:sm3p0 a0, a0
 ; RV64ZKSH-NEXT:ret
-  %val = call i64 @llvm.riscv.sm3p0.i64(i64 %a)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm3p0(i32 signext %a)
+  ret i32 %val
 }
 
-declare i64 @llvm.riscv.sm3p1.i64(i64);
+declare i32 @llvm.riscv.sm3p1(i32);
 
-define i64 @sm3p1_i64(i64 %a) nounwind {
-; RV64ZKSH-LABEL: sm3p1_i64:
+define signext i32 @sm3p1_i32(i32 signext %a) nounwind {
+; RV64ZKSH-LABEL: sm3p1_i32:
 ; RV64ZKSH:   # %bb.0:
 ; RV64ZKSH-NEXT:sm3p1 a0, a0
 ; RV64ZKSH-NEXT:ret
-  %val = call i64 @llvm.riscv.sm3p1.i64(i64 %a)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm3p1(i32 signext %a)
+  ret i32 %val
 }
Index: llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
===
--- llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
+++ llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
@@ -2,24 +2,24 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zksed -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s -check-prefix=RV64ZKSED
 
-declare i64 @llvm.riscv.sm4ks.i64(i64, i64, i32);
+declare i32 @llvm.riscv.sm4ks(i32, i32, i32);
 
-define i64 @sm4ks_i64(i64 %a, i64 %b) nounwind {
-; RV64ZKSED-LABEL: sm4ks_i64:
+define signext i32 @sm4ks_i32(i32 signext %a, i32 signext %b) nounwind {
+; RV64ZKSED-LABEL: sm4ks_i32:
 ; RV64ZKSED:   # %bb.0:
-; RV64ZKSED-NEXT:sm4ks a0, a0, a1, 0
+; RV64ZKSED-NEXT:sm4ks a0, a0, a1, 2
 ; RV64ZKSED-NEXT:ret
-  %val = call i64 @llvm.riscv.sm4ks.i64(i64 %a, i64 %b, i32 0)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm4ks(i32 %a, i32 %b, i32 2)
+  ret i32 %val
 }
 
-declare i64 @llvm.riscv.sm4ed.i64(i64, i64, i32);
+declare i32 @llvm.riscv.sm4ed(i32, i32, i32);
 
-define i64 @sm4ed_i64(i64 %a, i64 %b) nounwind {
-; RV64ZKSED-LABEL: sm4ed_i64:
+define signext i32 @sm4ed_i32(i32 signext %a, i32 signext %b) nounwind {

[PATCH] D154647: [RISCV] Re-define sha256, Zksed, and Zksh intrinsics to use i32 types.

2023-07-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper updated this revision to Diff 540716.
craig.topper added a comment.

Merge identical tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154647

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/zksh.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVInstrInfoZk.td
  llvm/test/CodeGen/RISCV/rv32zknh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv32zksed-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv32zksh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zknh-intrinsic-autoupgrade.ll
  llvm/test/CodeGen/RISCV/rv64zknh-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zksed-intrinsic-autoupgrade2.ll
  llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zksh-intrinsic-autoupgrade.ll
  llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
  llvm/test/CodeGen/RISCV/sextw-removal.ll

Index: llvm/test/CodeGen/RISCV/sextw-removal.ll
===
--- llvm/test/CodeGen/RISCV/sextw-removal.ll
+++ llvm/test/CodeGen/RISCV/sextw-removal.ll
@@ -1319,13 +1319,11 @@
 ; NOREMOVAL-NEXT:addi sp, sp, 32
 ; NOREMOVAL-NEXT:ret
 bb:
-  %sext = sext i32 %arg1 to i64
-  %i = call i64 @llvm.riscv.sha256sig0.i64(i64 %sext)
-  %trunc = trunc i64 %i to i32
+  %i = call i32 @llvm.riscv.sha256sig0(i32 %arg1)
   br label %bb2
 
 bb2:  ; preds = %bb2, %bb
-  %i3 = phi i32 [ %trunc, %bb ], [ %i5, %bb2 ]
+  %i3 = phi i32 [ %i, %bb ], [ %i5, %bb2 ]
   %i4 = tail call signext i32 @bar(i32 signext %i3)
   %i5 = shl i32 %i3, %arg1
   %i6 = icmp eq i32 %i4, 0
@@ -1334,7 +1332,7 @@
 bb7:  ; preds = %bb2
   ret void
 }
-declare i64 @llvm.riscv.sha256sig0.i64(i64)
+declare i32 @llvm.riscv.sha256sig0(i32)
 
 ; The type promotion of %7 forms a sext_inreg, but %7 and %6 are combined to
 ; form a sh2add. This leaves behind a sext.w that isn't needed.
Index: llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
===
--- llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
+++ llvm/test/CodeGen/RISCV/rv64zksh-intrinsic.ll
@@ -2,24 +2,24 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zksh -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s -check-prefix=RV64ZKSH
 
-declare i64 @llvm.riscv.sm3p0.i64(i64);
+declare i32 @llvm.riscv.sm3p0(i32);
 
-define i64 @sm3p0_i64(i64 %a) nounwind {
-; RV64ZKSH-LABEL: sm3p0_i64:
+define signext i32 @sm3p0_i32(i32 signext %a) nounwind {
+; RV64ZKSH-LABEL: sm3p0_i32:
 ; RV64ZKSH:   # %bb.0:
 ; RV64ZKSH-NEXT:sm3p0 a0, a0
 ; RV64ZKSH-NEXT:ret
-  %val = call i64 @llvm.riscv.sm3p0.i64(i64 %a)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm3p0(i32 signext %a)
+  ret i32 %val
 }
 
-declare i64 @llvm.riscv.sm3p1.i64(i64);
+declare i32 @llvm.riscv.sm3p1(i32);
 
-define i64 @sm3p1_i64(i64 %a) nounwind {
-; RV64ZKSH-LABEL: sm3p1_i64:
+define signext i32 @sm3p1_i32(i32 signext %a) nounwind {
+; RV64ZKSH-LABEL: sm3p1_i32:
 ; RV64ZKSH:   # %bb.0:
 ; RV64ZKSH-NEXT:sm3p1 a0, a0
 ; RV64ZKSH-NEXT:ret
-  %val = call i64 @llvm.riscv.sm3p1.i64(i64 %a)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm3p1(i32 signext %a)
+  ret i32 %val
 }
Index: llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
===
--- llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
+++ llvm/test/CodeGen/RISCV/rv64zksed-intrinsic.ll
@@ -2,24 +2,24 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zksed -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s -check-prefix=RV64ZKSED
 
-declare i64 @llvm.riscv.sm4ks.i64(i64, i64, i32);
+declare i32 @llvm.riscv.sm4ks(i32, i32, i32);
 
-define i64 @sm4ks_i64(i64 %a, i64 %b) nounwind {
-; RV64ZKSED-LABEL: sm4ks_i64:
+define signext i32 @sm4ks_i32(i32 signext %a, i32 signext %b) nounwind {
+; RV64ZKSED-LABEL: sm4ks_i32:
 ; RV64ZKSED:   # %bb.0:
-; RV64ZKSED-NEXT:sm4ks a0, a0, a1, 0
+; RV64ZKSED-NEXT:sm4ks a0, a0, a1, 2
 ; RV64ZKSED-NEXT:ret
-  %val = call i64 @llvm.riscv.sm4ks.i64(i64 %a, i64 %b, i32 0)
-  ret i64 %val
+  %val = call i32 @llvm.riscv.sm4ks(i32 %a, i32 %b, i32 2)
+  ret i32 %val
 }
 
-declare i64 @llvm.riscv.sm4ed.i64(i64, i64, i32);
+declare i32 @llvm.riscv.sm4ed(i32, i32, i32);
 
-define i64 @sm4ed_i64(i64 %a, i64 %b) nounwind {
-; RV64ZKSED-LA

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Thanks for putting this together, it definitely seems like something people 
want.

My main concern is that the configuration has the wrong scope.
We're checking whether reserved-ident indexing is on for the **TU** we're 
indexing, but really we want to specify which **headers** we should index 
reserved-idents from.
If hell freezes over and the linux kernel starts using the C++ standard library 
one day, we want the preamble index to contain `__free_pages()` but not 
`std::__variant_impl_helper`. But the preamble indexing all runs under the same 
config.
(Also we more or less assume that all TUs will index a header the same way: if 
we've indexed foo.h under A.cpp, we won't reindex it under B.cpp even if the 
config is different. So we get "first-one-wins" behavior...)

I don't think we can afford to re-evaluate the config each time we switch 
between headers while indexing. (Stupid over-flexible config system is too 
slow... I've learned my lesson from that one!)
So I'm not really sure what to suggest...

- we could do it this way anyway and accept that it's basically only usable as 
a global flag: this is probably fine for the linux kernel as it's mostly 
isolated anyway (no standard library).
- we could add some rules (regexes?) to the config that describe which headers 
can have interesting symbols. This could be slow too, but at least cacheable, 
like the self-contained check
- maybe we can come up with some heuristics to improve the "no reserved names" 
rule, with or without configuration. (Maybe apply it to only -isystem headers? 
Linux uses angle-quoted includes, but not actually `-isystem` AFAICS)

What do you think? Is this a real concern or am I missing something?




Comment at: clang-tools-extra/clangd/ConfigFragment.h:205
 std::optional> StandardLibrary;
+/// Whether identifiers reserved for use by the implementation (e.g.
+/// beginning with two underscores) should be indexed.

nit: it's the *symbols* we're indexing, based on whether their names are 
reserved identifiers.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:207
+/// beginning with two underscores) should be indexed.
+std::optional> ReservedIdentifiers;
   };

For some reason I find the name "ReservedNames" a little clearer.

I tend to think of an "identifier" as a type of token at the lexer level, and a 
"name" as its role at the language level. But I don't think this actually 
matches any standard terminology, so no need to change it unless it also reads 
better to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153946

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

[PATCH] D155380: [clang] Fix delayed template parsing

2023-07-15 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall, aaron.ballman, efriedma, mgorny.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added a project: clang.

Commit 98390ccb80569e8fbb20e6c996b4b8cff87fbec6 
 fixed 
late template
instantiation by clearing FP pragma stack before instantiation. This
solution was based on the assumptions:

- FP pragma stack is not used anymore and it is safe to clear it,
- Default FP options are determined by command line options.

Both the assumptions are wrong. When compilation produces precompiled
header file, state of the stack is serialized and then restored when the
precompiled header is used. Delayed template parsing occurs at the end
of translation unit but before serialization, so clearing FP pragma
stack effects serialized representation. When the precompiled file is
loaded, some conditions can be broken and clang crashed, it was
described in https://github.com/llvm/llvm-project/issues/63704. The
crash was observed only in few cases, on most buildbots it was absent.

The violation of expected conditions was caused by violation of the
second assumption. FPEvalMethod can be modified by target, so it is not
possible to deduce it from LangOptions only. So default FP state read
from precompiled header was different from the state in the initialized
Sema, and this was the crash reason.

Only two targets do such modification of default FP options, these are
i386 and AIX. so the problem was hard to reproduce.

Delayed template parsing should occur with empty pragma stack, so it
must be cleared before the instantiation, but the stack now is saved
and restored after the instantiation is done. Also default FP state is
not deduced from command line options, it is taken from the value stored
in the FP pragma stack.

This change should fix https://github.com/llvm/llvm-project/issues/63704.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155380

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/PCH/late-parsed-instantiations.cpp


Index: clang/test/PCH/late-parsed-instantiations.cpp
===
--- clang/test/PCH/late-parsed-instantiations.cpp
+++ clang/test/PCH/late-parsed-instantiations.cpp
@@ -4,7 +4,9 @@
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch 
-fpch-instantiate-templates %s -o %t.pch -verify
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch 
%s -verify
 
-// XFAIL: target={{.*}}-aix{{.*}}
+// Run this test for i686 as this is the target that modifies default FP 
options.
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fdelayed-template-parsing 
-std=c++14 -emit-pch -fpch-instantiate-templates %s -o %t.pch -verify
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fdelayed-template-parsing 
-std=c++14 -include-pch %t.pch %s -verify
 
 #ifndef HEADER_INCLUDED
 
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -1744,6 +1744,7 @@
 
   // Parsing should occur with empty FP pragma stack and FP options used in the
   // point of the template definition.
+  Sema::FpPragmaStackSaveRAII SavedStack(Actions);
   Actions.resetFPOptions(LPT.FPO);
 
   assert(!LPT.Toks.empty() && "Empty body!");
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -710,10 +710,19 @@
 return result;
   }
 
+  class FpPragmaStackSaveRAII {
+  public:
+FpPragmaStackSaveRAII(Sema &S) : S(S), SavedStack(S.FpPragmaStack) {}
+~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }
+  private:
+Sema &S;
+PragmaStack SavedStack;
+  };
+
   void resetFPOptions(FPOptions FPO) {
 CurFPFeatures = FPO;
 FpPragmaStack.Stack.clear();
-FpPragmaStack.CurrentValue = FPO.getChangesFrom(FPOptions(LangOpts));
+FpPragmaStack.CurrentValue = FpPragmaStack.DefaultValue;
   }
 
   // RAII object to push / pop sentinel slots for all MS #pragma stacks.


Index: clang/test/PCH/late-parsed-instantiations.cpp
===
--- clang/test/PCH/late-parsed-instantiations.cpp
+++ clang/test/PCH/late-parsed-instantiations.cpp
@@ -4,7 +4,9 @@
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch -fpch-instantiate-templates %s -o %t.pch -verify
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch %s -verify
 
-// XFAIL: target={{.*}}-aix{{.*}}
+// Run this test for i686 as this is the target that modifies default FP options.
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fdelayed-template-parsing -std=c++14 -emit-pch -fpch-instantiate-templates %s -o %t.p

[clang] bb6ab91 - Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-15 Thread Hubert Tong via cfe-commits

Author: Zheng Qian
Date: 2023-07-15T16:13:48-04:00
New Revision: bb6ab91b1dcd2fadfddffcd020439978af184862

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

LOG: Add option -fkeep-persistent-storage-variables to emit all variables that 
have a persistent storage duration

This patch adds a new option -fkeep-persistent-storage-variables to emit
all variables that have a persistent storage duration, including global,
static and thread-local variables. This could be useful in cases where
the presence of all these variables as symbols in the object file are
required, so that they can be directly addressed.

Reviewed By: hubert.reinterpretcast

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

Added: 
clang/test/CodeGen/keep-persistent-storage-variables.cpp
clang/test/Driver/fkeep-persistent-storage-variables.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index a03447ca2931e2..f9903b18603b58 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -477,6 +477,10 @@ CODEGENOPT(Addrsig, 1, 0)
 /// Whether to emit unused static constants.
 CODEGENOPT(KeepStaticConsts, 1, 0)
 
+/// Whether to emit all variables that have a persistent storage duration,
+/// including global, static and thread local variables.
+CODEGENOPT(KeepPersistentStorageVariables, 1, 0)
+
 /// Whether to follow the AAPCS enforcing at least one read before storing to 
a volatile bitfield
 CODEGENOPT(ForceAAPCSBitfieldLoad, 1, 0)
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8d061e0ca191ea..d4e3e168b4cd01 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1757,6 +1757,10 @@ defm keep_static_consts : 
BoolFOption<"keep-static-consts",
   CodeGenOpts<"KeepStaticConsts">, DefaultFalse,
   PosFlag, NegFlag,
   BothFlags<[NoXarchOption], " static const variables even if unused">>;
+defm keep_persistent_storage_variables : 
BoolFOption<"keep-persistent-storage-variables",
+  CodeGenOpts<"KeepPersistentStorageVariables">, DefaultFalse,
+  PosFlag, NegFlag,
+  BothFlags<[NoXarchOption], " keeping all variables that have a persistent 
storage duration, including global, static and thread-local variables, to 
guarantee that they can be directly addressed">>;
 defm fixed_point : BoolFOption<"fixed-point",
   LangOpts<"FixedPoint">, DefaultFalse,
   PosFlag, NegFlag,

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index f0953eed6acde9..b0d6eb05acc22e 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -470,6 +470,9 @@ void CodeGenFunction::EmitStaticVarDecl(const VarDecl &D,
   else if (D.hasAttr())
 CGM.addUsedOrCompilerUsedGlobal(var);
 
+  if (CGM.getCodeGenOpts().KeepPersistentStorageVariables)
+CGM.addUsedOrCompilerUsedGlobal(var);
+
   // We may have to cast the constant because of the initializer
   // mismatch above.
   //

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index d8a45e4dcb3064..1a9cfc21ba71db 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2426,12 +2426,14 @@ void CodeGenModule::SetCommonAttributes(GlobalDecl GD, 
llvm::GlobalValue *GV) {
   if (D && D->hasAttr())
 addUsedOrCompilerUsedGlobal(GV);
 
-  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
-const auto *VD = cast(D);
-if (VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  addUsedOrCompilerUsedGlobal(GV);
-  }
+  if (const auto *VD = dyn_cast_if_present(D);
+  VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+(VD->getStorageDuration() == SD_Static ||
+ VD->getStorageDuration() == SD_Thread)) ||
+   (CodeGenOpts.KeepStaticConsts && VD->getStorageDuration() == SD_Static 
&&
+VD->getType().isConstQualified(
+addUsedOrCompilerUsedGlobal(GV);
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
@@ -3312,12 +3314,14 @@ bool CodeGenModule::MustBeEmitted(const ValueDecl 
*Global) {
   if (LangOpts.EmitAllDecls)
 return true;
 
-  if (CodeGenOpts.KeepStaticConsts) {
-const auto *VD = dyn_cast(Global);
-if (VD && VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  return true;
-  }
+  const auto *VD = dyn_cast(Global);
+  if (VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-15 Thread Hubert Tong via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb6ab91b1dcd: Add option -fkeep-persistent-storage-variables 
to emit all variables that have… (authored by qianzhen, committed by 
hubert.reinterpretcast).

Changed prior to commit:
  https://reviews.llvm.org/D150221?vs=540060&id=540732#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/keep-persistent-storage-variables.cpp
  clang/test/Driver/fkeep-persistent-storage-variables.c

Index: clang/test/Driver/fkeep-persistent-storage-variables.c
===
--- /dev/null
+++ clang/test/Driver/fkeep-persistent-storage-variables.c
@@ -0,0 +1,5 @@
+// RUN: %clang -fkeep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s
+// RUN: %clang -fkeep-persistent-storage-variables -fno-keep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NOKEEP
+
+// CHECK: "-fkeep-persistent-storage-variables"
+// CHECK-NOKEEP-NOT: "-fkeep-persistent-storage-variables"
Index: clang/test/CodeGen/keep-persistent-storage-variables.cpp
===
--- /dev/null
+++ clang/test/CodeGen/keep-persistent-storage-variables.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=powerpc64-ibm-aix-xcoff | FileCheck %s
+
+// CHECK: @_ZL2g1 = internal global i32 0, align 4
+// CHECK: @_ZL2g2 = internal global i32 1, align 4
+// CHECK: @tl1 = thread_local global i32 0, align 4
+// CHECK: @tl2 = thread_local global i32 3, align 4
+// CHECK: @_ZL3tl3 = internal thread_local global i32 0, align 4
+// CHECK: @_ZL3tl4 = internal thread_local global i32 4, align 4
+// CHECK: @g5 = global i32 0, align 4
+// CHECK: @g6 = global i32 6, align 4
+// CHECK: @_ZZ5test3vE2s3 = internal global i32 0, align 4
+// CHECK: @_ZN12_GLOBAL__N_12s4E = internal global i32 42, align 4
+// CHECK: @_ZZ5test5vE3tl5 = internal thread_local global i32 1, align 4
+// CHECK: @_ZN2ST2s6E = global i32 7, align 4
+// CHECK: @_Z2v7 = internal global %union.anon zeroinitializer, align 4
+// CHECK: @_ZDC2v8E = global %struct.ST8 zeroinitializer, align 4
+// CHECK: @llvm{{(\.compiler)?}}.used = appending global [14 x ptr] [ptr @_ZL2g1, ptr @_ZL2g2, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr @_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E, ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E, ptr @_Z2v7, ptr @_ZDC2v8E], section "llvm.metadata"
+
+static int g1;
+static int g2 = 1;
+__thread int tl1;
+__thread int tl2 = 3;
+static __thread int tl3;
+static __thread int tl4 = 4;
+int g5;
+int g6 = 6;
+
+int test3() {
+  static int s3 = 0;
+  ++s3;
+  return s3;
+}
+
+namespace {
+  int s4 = 42;
+}
+
+int test5() {
+  static __thread int tl5 = 1;
+  ++tl5;
+  return tl5;
+}
+
+struct ST {
+  static int s6;
+};
+int ST::s6 = 7;
+
+static union { int v7; };
+
+struct ST8 { int v8; };
+auto [v8] = ST8{0};
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7404,6 +7404,8 @@
 
   Args.addOptInFlag(CmdArgs, options::OPT_fkeep_static_consts,
 options::OPT_fno_keep_static_consts);
+  Args.addOptInFlag(CmdArgs, options::OPT_fkeep_persistent_storage_variables,
+options::OPT_fno_keep_persistent_storage_variables);
   Args.addOptInFlag(CmdArgs, options::OPT_fcomplete_member_pointers,
 options::OPT_fno_complete_member_pointers);
   Args.addOptOutFlag(CmdArgs, options::OPT_fcxx_static_destructors,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2426,12 +2426,14 @@
   if (D && D->hasAttr())
 addUsedOrCompilerUsedGlobal(GV);
 
-  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
-const auto *VD = cast(D);
-if (VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  addUsedOrCompilerUsedGlobal(GV);
-  }
+  if (const auto *VD = dyn_cast_if_present(D);
+  VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+(VD->getStorageDuration() == SD_Static ||
+ VD->getStorageDuration() == SD_Thread)) ||
+   (CodeGenOpts.KeepStaticConsts && VD->getStorageDuration() == SD_Static &&
+VD->getType().isConstQualified(
+addUsedOrCompilerUsedGlobal(GV);
 }

[PATCH] D155381: [clangd] Allow indexing of __reserved_names outside system headers

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

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

The special handling for these names was added in
https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28
and the motivation was the C++ standard library.

It turns out some projects like using these names anyway, in particular the
linux kernel. D153946  proposed making this a 
config option, but there are
some implementation issues with the config system.

As an alternative, this patch tweaks the heuristic so we only drop these symbols
in system headers. This does the right thing for linux and the C++ standard
library, at least.

Fixes https://github.com/clangd/clangd/issues/1680
Fixes https://github.com/llvm/llvm-project/issues/63862


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155381

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -303,10 +303,12 @@
 Args, Factory->create(), Files.get(),
 std::make_shared());
 
-InMemoryFileSystem->addFile(TestHeaderName, 0,
-llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-InMemoryFileSystem->addFile(TestFileName, 0,
-llvm::MemoryBuffer::getMemBuffer(MainCode));
+// Multiple calls to runSymbolCollector with different contents will fail
+// to update the filesystem! Why are we sharing one across tests, anyway?
+EXPECT_TRUE(InMemoryFileSystem->addFile(
+TestHeaderName, 0, llvm::MemoryBuffer::getMemBuffer(HeaderCode)));
+EXPECT_TRUE(InMemoryFileSystem->addFile(
+TestFileName, 0, llvm::MemoryBuffer::getMemBuffer(MainCode)));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
 Refs = Factory->Collector->takeRefs();
@@ -1956,17 +1958,25 @@
 
 TEST_F(SymbolCollectorTest, Reserved) {
   const char *Header = R"cpp(
+#pragma once
 void __foo();
 namespace _X { int secret; }
   )cpp";
 
   CollectorOpts.CollectReserved = true;
-  runSymbolCollector("", Header);
+  runSymbolCollector(Header, "");
   EXPECT_THAT(Symbols, UnorderedElementsAre(qName("__foo"), qName("_X"),
 qName("_X::secret")));
 
   CollectorOpts.CollectReserved = false;
-  runSymbolCollector("", Header); //
+  runSymbolCollector(Header, "");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(qName("__foo"), qName("_X"),
+qName("_X::secret")));
+
+  // Ugly: for some reason we reuse the test filesystem across tests.
+  // You can't overwrite the same filename with new content!
+  InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
+  runSymbolCollector("#pragma GCC system_header\n" + std::string(Header), "");
   EXPECT_THAT(Symbols, IsEmpty());
 }
 
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -87,6 +87,7 @@
 bool CollectMainFileRefs = false;
 /// Collect symbols with reserved names, like __Vector_base.
 /// This does not currently affect macros (many like _WIN32 are important!)
+/// This only affects system headers.
 bool CollectReserved = false;
 /// If set to true, SymbolCollector will collect doc for all symbols.
 /// Note that documents of symbols being indexed for completion will always
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -490,7 +490,8 @@
   if (isPrivateProtoDecl(ND))
 return false;
   if (!Opts.CollectReserved &&
-  (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext(
+  (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext())) &&
+  ASTCtx.getSourceManager().isInSystemHeader(ND.getLocation()))
 return false;
 
   return true;


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -303,10 +303,12 @@
 Args, Factory->create(), Files.get(),

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D153946#4503585 , @sammccall wrote:

> - maybe we can come up with some heuristics to improve the "no reserved 
> names" rule, with or without configuration. (Maybe apply it to only -isystem 
> headers? Linux uses angle-quoted includes, but not actually `-isystem` AFAICS)

I tried this out, and it behaves well at least for linux & the C++ stdlib on my 
system. D155381  if you like the approach.

I have the feeling there may still be some exceptions, but we don't know about 
them yet and it sidesteps the config questions...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153946

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

[PATCH] D155339: Enable zba and zbs for RISCV64 Android

2023-07-15 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/test/Driver/riscv-features.c:13
 
+// ANDROID: "-target-feature" "+zba"
 // ANDROID: "-target-feature" "+zbb"

If the features are adjacent, test them on one line. `// ANDROID: 
"-target-feature" "+zba" "-target-feature" "+zbb" ...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155339

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

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-15 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added a comment.

Thank you for the update. I think this Clang patch is good enough to land even 
if the lld part is still in review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

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

[PATCH] D155383: [clang][AST] TextNodeDumper learned to output exception specifications

2023-07-15 Thread Timo Stripf via Phabricator via cfe-commits

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

Extended TextNodeDumper::VisitFunctionProtoType to output exception 
specifications


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155383

Files:
  clang/lib/AST/TextNodeDumper.cpp


Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1541,7 +1541,64 @@
 OS << " &&";
 break;
   }
-  // FIXME: Exception specification.
+
+  switch (EPI.ExceptionSpec.Type) {
+  case EST_None:
+break;
+  case EST_DynamicNone:
+OS << " exceptionspec_dynamicnone";
+break;
+  case EST_Dynamic:
+OS << " exceptionspec_dynamic";
+break;
+  case EST_MSAny:
+OS << " exceptionspec_msany";
+break;
+  case EST_NoThrow:
+OS << " exceptionspec_nothrow";
+break;
+  case EST_BasicNoexcept:
+OS << " exceptionspec_basicnoexcept";
+break;
+  case EST_DependentNoexcept:
+OS << " exceptionspec_dependentnoexcept";
+break;
+  case EST_NoexceptFalse:
+OS << " exceptionspec_noexceptfalse";
+break;
+  case EST_NoexceptTrue:
+OS << " exceptionspec_noexcepttrue";
+break;
+  case EST_Unevaluated:
+OS << " exceptionspec_unevaluated";
+break;
+  case EST_Uninstantiated:
+OS << " exceptionspec_uninstantiated";
+break;
+  case EST_Unparsed:
+OS << " exceptionspec_unparsed";
+break;
+  }
+  if (!EPI.ExceptionSpec.Exceptions.empty()) {
+AddChild([=] {
+  OS << "Exceptions:";
+  for (unsigned I = 0, N = EPI.ExceptionSpec.Exceptions.size(); I != N;
+   ++I) {
+if (I)
+  OS << ",";
+dumpType(EPI.ExceptionSpec.Exceptions[I]);
+  }
+});
+  }
+  if (EPI.ExceptionSpec.NoexceptExpr) {
+AddChild([=] {
+  OS << "NoexceptExpr: ";
+  Visit(EPI.ExceptionSpec.NoexceptExpr);
+});
+  }
+  dumpDeclRef(EPI.ExceptionSpec.SourceDecl, "ExceptionSourceDecl");
+  dumpDeclRef(EPI.ExceptionSpec.SourceTemplate, "ExceptionSourceTemplate");
+
   // FIXME: Consumed parameters.
   VisitFunctionType(T);
 }


Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1541,7 +1541,64 @@
 OS << " &&";
 break;
   }
-  // FIXME: Exception specification.
+
+  switch (EPI.ExceptionSpec.Type) {
+  case EST_None:
+break;
+  case EST_DynamicNone:
+OS << " exceptionspec_dynamicnone";
+break;
+  case EST_Dynamic:
+OS << " exceptionspec_dynamic";
+break;
+  case EST_MSAny:
+OS << " exceptionspec_msany";
+break;
+  case EST_NoThrow:
+OS << " exceptionspec_nothrow";
+break;
+  case EST_BasicNoexcept:
+OS << " exceptionspec_basicnoexcept";
+break;
+  case EST_DependentNoexcept:
+OS << " exceptionspec_dependentnoexcept";
+break;
+  case EST_NoexceptFalse:
+OS << " exceptionspec_noexceptfalse";
+break;
+  case EST_NoexceptTrue:
+OS << " exceptionspec_noexcepttrue";
+break;
+  case EST_Unevaluated:
+OS << " exceptionspec_unevaluated";
+break;
+  case EST_Uninstantiated:
+OS << " exceptionspec_uninstantiated";
+break;
+  case EST_Unparsed:
+OS << " exceptionspec_unparsed";
+break;
+  }
+  if (!EPI.ExceptionSpec.Exceptions.empty()) {
+AddChild([=] {
+  OS << "Exceptions:";
+  for (unsigned I = 0, N = EPI.ExceptionSpec.Exceptions.size(); I != N;
+   ++I) {
+if (I)
+  OS << ",";
+dumpType(EPI.ExceptionSpec.Exceptions[I]);
+  }
+});
+  }
+  if (EPI.ExceptionSpec.NoexceptExpr) {
+AddChild([=] {
+  OS << "NoexceptExpr: ";
+  Visit(EPI.ExceptionSpec.NoexceptExpr);
+});
+  }
+  dumpDeclRef(EPI.ExceptionSpec.SourceDecl, "ExceptionSourceDecl");
+  dumpDeclRef(EPI.ExceptionSpec.SourceTemplate, "ExceptionSourceTemplate");
+
   // FIXME: Consumed parameters.
   VisitFunctionType(T);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-15 Thread Richard Dzenis via Phabricator via cfe-commits

RIscRIpt added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1972-1987
+  SmallString<64> Str;
+  llvm::raw_svector_ostream OS(Str);
+  Token Exp;
+  Exp.startToken();
+  if (Tok.getKind() == tok::kw_L__FUNCTION__ ||
+  Tok.getKind() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

cor3ntin wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > I think it might be easier to create a string_literal token directly 
> > > here. I'm also not sure we need to use `Lexer::Stringify`
> > > I think it might be easier to create a string_literal token directly here.
> > 
> > What do you mean? Is there a function which creates Token object from 
> > StringRef? Or is there a way to associate string literal value with a Token 
> > without PP? I would like to simplify it, but I haven't found other ways of 
> > achieving the same result.
> > 
> > > I'm also not sure we need to use Lexer::Stringify
> > 
> > Well, as far as I can see `StringLiteralParser` expands escape sequences. 
> > So, I am just being too careful here.
> > If not using `Lexer::Stringify` do we want to assert that function name 
> > does not contain neither `"` not `\` (which should not happen™)?
> Thanks! I really would get rid of `Stringify` here - I struggle to see how a 
> function could be produced that has characters that cannot appear in an 
> identifier. even asserting isn't very useful.
Replaced `Stringify` with enclosure into raw-string-literal. Despite 
raw-string-literals are available starting from C++11, as far as I can see 
`StringLiteralParser` does not check for the standard.
Let me know, if you believe we cannot abuse this fact, and we should keep using 
`"regular string-literal"` for this purpose.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

cor3ntin wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > do we have any tests that look at the values of these things?
> > ```
> > clang/test/Analysis/eval-predefined-exprs.cpp
> > clang/test/AST/Interp/literals.cpp
> > clang/test/SemaCXX/source_location.cpp
> > clang/test/SemaCXX/predefined-expr.cpp
> > ```
> I think it's worth add a few more tests in 
> clang/test/SemaCXX/predefined-expr.cpp checking concatenations
Added tests to check values of these string literals, as well as moved 
`ms_wide_predefined_expr.cpp` tests into `ms_predefined_expr.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

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

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-15 Thread Richard Dzenis via Phabricator via cfe-commits

RIscRIpt updated this revision to Diff 540741.
RIscRIpt marked 7 inline comments as done.
RIscRIpt added a comment.

Addressed review comments, rebased onto main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = &STR2WSTR(__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = &STR2WSTR(__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,123 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft exten

[PATCH] D150338: [-Wunsafe-buffer-usage] Improving insertion of the [[clang::unsafe_buffer_usage]] attribute

2023-07-15 Thread Ziqing Luo via Phabricator via cfe-commits

ziqingluo-90 added a comment.

In D150338#4502885 , @chapuni wrote:

> Excuse me, I have reverted this due to circular deps.
> `clangAnalysis` should not depend on `clangSema`.

Thank you @chapuni ,  I will fix it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150338

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

[clang] a07a6f6 - Re-land "5b012bf5ab5fcb840fe7f6c8664b8981ce6f24f3"

2023-07-15 Thread via cfe-commits

Author: ziqingluo-90
Date: 2023-07-15T16:11:37-07:00
New Revision: a07a6f6c74a03405eccdcd3832acb2187d8b9c21

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

LOG: Re-land "5b012bf5ab5fcb840fe7f6c8664b8981ce6f24f3"

Removed dependency on `clangSema` from UnsafeBufferAnalysis.

Added: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-attributes-spelling.cpp

Modified: 
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 617bc7c77c5653..6766ba8ec2 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -46,14 +46,9 @@ class UnsafeBufferUsageHandler {
   /// Returns a reference to the `Preprocessor`:
   virtual bool isSafeBufferOptOut(const SourceLocation &Loc) const = 0;
 
-  /// Returns the text indicating that the user needs to provide input there:
   virtual std::string
-  getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") const {
-std::string s = std::string("<# ");
-s += HintTextToUser;
-s += " #>";
-return s;
-  }
+  getUnsafeBufferUsageAttributeTextAt(SourceLocation Loc,
+  StringRef WSSuffix = "") const = 0;
 };
 
 // This function invokes the analysis and allows the caller to react to it

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d8caa683c1e5e1..c8c1452a8f5543 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1290,6 +1290,14 @@ static StringRef getEndOfLine() {
   return EOL;
 }
 
+// Returns the text indicating that the user needs to provide input there:
+std::string getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") {
+  std::string s = std::string("<# ");
+  s += HintTextToUser;
+  s += " #>";
+  return s;
+}
+
 // Return the text representation of the given `APInt Val`:
 static std::string getAPIntText(APInt Val) {
   SmallVector Txt;
@@ -1755,14 +1763,6 @@ static bool hasConflictingOverload(const FunctionDecl 
*FD) {
   return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult();
 }
 
-// Returns the text representation of clang::unsafe_buffer_usage attribute.
-static std::string getUnsafeBufferUsageAttributeText() {
-  static const char *const RawAttr = "[[clang::unsafe_buffer_usage]]";
-  std::stringstream SS;
-  SS << RawAttr << getEndOfLine().str();
-  return SS.str();
-}
-
 // For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new
 // type, this function produces fix-its to make the change self-contained.  Let
 // 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity
@@ -1809,7 +1809,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef 
NewTyText,
   // FIXME: need to make this conflict checking better:
   if (hasConflictingOverload(FD))
 return std::nullopt;
-
+  
   const SourceManager &SM = Ctx.getSourceManager();
   const LangOptions &LangOpts = Ctx.getLangOpts();
   // FIXME Respect indentation of the original code.
@@ -1859,7 +1859,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef 
NewTyText,
   // A lambda that creates the text representation of a function definition 
with
   // the original signature:
   const auto OldOverloadDefCreator =
-  [&Handler, &SM,
+  [&SM, &Handler,
&LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
   StringRef NewTypeText) -> std::optional {
 std::stringstream SS;
@@ -1869,7 +1869,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef 
NewTyText,
 if (auto FDPrefix = getRangeText(
 SourceRange(FD->getBeginLoc(), FD->getBody()->getBeginLoc()), SM,
 LangOpts))
-  SS << getUnsafeBufferUsageAttributeText() << FDPrefix->str() << "{";
+  SS << Handler.getUnsafeBufferUsageAttributeTextAt(FD->getBeginLoc(), " ")
+ << FDPrefix->str() << "{";
 else
   return std::nullopt;
 // Append: "return" func-name "("
@@ -1889,8 +1890,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef 
NewTyText,
  "A parameter of a function definition has no name");
   if (i == ParmIdx)
 // This is our spanified para

[PATCH] D150338: [-Wunsafe-buffer-usage] Improving insertion of the [[clang::unsafe_buffer_usage]] attribute

2023-07-15 Thread Ziqing Luo via Phabricator via cfe-commits

ziqingluo-90 added a comment.

Re-landed in `a07a6f6c74a03405eccdcd3832acb2187d8b9c21`

Moved the use of `clang::Sema` from `UnsafeBufferAnalysis` to 
`AnalysisBasedWarnings`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150338

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

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Oh no, I just saw this never landed - I suspect you were waiting for me to 
commit it, and I was expecting you to do it!

I've rebased and I'll commit it for you now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150635

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

[clang-tools-extra] 9e6a342 - [clangd] Implement end-definition-comment inlay hints

2023-07-15 Thread Sam McCall via cfe-commits

Author: daiyousei-qz
Date: 2023-07-16T01:19:02+02:00
New Revision: 9e6a342fdac8978ef6d3e373cbbc7425e3dfe0f8

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

LOG: [clangd] Implement end-definition-comment inlay hints

This patch implements a new inlay hint feature proposed in 
https://github.com/clangd/clangd/issues/1634. It introduces a new inlay hint 
kind BlockEnd which shows a comment-like hint after a definition brace pair, 
including function/type/namespace. For example,
```
void foo() {
} ^
```
In the code shown above, a hint should be displayed at ^ labelling `// foo`. 
Such hint only shows when there's no trailing character after the position 
except whitespaces and optionally ';'.

Also, new configurations are introduced in the inlay hints block
```
InlayHints:
BlockEnd: Yes # toggling the feature
```

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index b46fedac3f88e9..daae8d1c0c833e 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -144,6 +144,7 @@ struct Config {
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+bool BlockEnd = false;
 // Limit the length of type names in inlay hints. (0 means no limit)
 uint32_t TypeNameLimit = 32;
   } InlayHints;

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 64d65399072333..d4ff7ae3181bc3 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -606,6 +606,10 @@ struct FragmentCompiler {
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config &C) {
 C.InlayHints.Designators = Value;
   });
+if (F.BlockEnd)
+  Out.Apply.push_back([Value(**F.BlockEnd)](const Params &, Config &C) {
+C.InlayHints.BlockEnd = Value;
+  });
 if (F.TypeNameLimit)
   Out.Apply.push_back(
   [Value(**F.TypeNameLimit)](const Params &, Config &C) {

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 0300f7a5cb73f2..a59d4124b0828a 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -318,6 +318,8 @@ struct Fragment {
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Show defined symbol names at the end of a definition block.
+std::optional> BlockEnd;
 /// Limit the length of type name hints. (0 means no limit)
 std::optional> TypeNameLimit;
   };

diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 371c9d3ed4d525..cf3cec472bf8a3 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -252,6 +252,10 @@ class Parser {
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("BlockEnd", [&](Node &N) {
+  if (auto Value = boolValue(N, "BlockEnd"))
+F.BlockEnd = *Value;
+});
 Dict.handle("TypeNameLimit", [&](Node &N) {
   if (auto Value = uint32Value(N, "TypeNameLimit"))
 F.TypeNameLimit = *Value;

diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 941a8fdfdfdfee..1347fa0d2b29e0 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -327,6 +327,33 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   addReturnTypeHint(D, FTL.getRParenLoc());
   }
 }
+if (Cfg.InlayHints.BlockEnd && D->isThisDeclarationADefinition()) {
+  // We use `printName` here to properly print name of ctor/dtor/operator
+  // overload.
+  if (const Stmt *Body = D->getBody())
+addBlockEndHint(Body->getSourceRange(), "", printName(AST, *D), "");
+}
+return true;
+  }
+
+  bool VisitTagDecl(TagDecl *D) {
+if (Cfg.InlayHints.BlockEnd && D->isThisDeclarationADefinition()) {
+  std::string DeclPrefix = D->getKindName().str();
+  if (const auto *ED = dyn_cast(D)) {
+if (ED->isScoped())
+  DeclPrefix += ED->isScopedUsingClassTag() ? " class" : " struct";
+  };
+

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e6a342fdac8: [clangd] Implement end-definition-comment 
inlay hints (authored by daiyousei-qz, committed by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D150635?vs=525922&id=540750#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150635

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -77,6 +77,7 @@
   C.InlayHints.Parameters = false;
   C.InlayHints.DeducedTypes = false;
   C.InlayHints.Designators = false;
+  C.InlayHints.BlockEnd = false;
   return C;
 }
 
@@ -122,6 +123,15 @@
   assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
 }
 
+template 
+void assertBlockEndHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+  Config Cfg;
+  Cfg.InlayHints.BlockEnd = true;
+  WithContextValue WithCfg(Config::Key, std::move(Cfg));
+  assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Expected...);
+}
+
 TEST(ParameterHints, Smoke) {
   assertParameterHints(R"cpp(
 void foo(int param);
@@ -1629,6 +1639,194 @@
   ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"},
   ExpectedHint{"c: ", "param3"});
 }
+
+TEST(BlockEndHints, Functions) {
+  assertBlockEndHints(R"cpp(
+int foo() {
+  return 41;
+$foo[[}]]
+
+template 
+int bar() { 
+  // No hint for lambda for now
+  auto f = []() { 
+return X; 
+  };
+  return f(); 
+$bar[[}]]
+
+// No hint because this isn't a definition
+int buz();
+
+struct S{};
+bool operator==(S, S) {
+  return true;
+$opEqual[[}]]
+  )cpp",
+  ExpectedHint{" // foo", "foo"},
+  ExpectedHint{" // bar", "bar"},
+  ExpectedHint{" // operator==", "opEqual"});
+}
+
+TEST(BlockEndHints, Methods) {
+  assertBlockEndHints(R"cpp(
+struct Test {
+  // No hint because there's no function body
+  Test() = default;
+  
+  ~Test() {
+  $dtor[[}]]
+
+  void method1() {
+  $method1[[}]]
+
+  // No hint because this isn't a definition
+  void method2();
+
+  template 
+  void method3() {
+  $method3[[}]]
+
+  // No hint because this isn't a definition
+  template 
+  void method4();
+
+  Test operator+(int) const {
+return *this;
+  $opIdentity[[}]]
+
+  operator bool() const {
+return true;
+  $opBool[[}]]
+
+  // No hint because there's no function body
+  operator int() const = delete;
+} x;
+
+void Test::method2() {
+$method2[[}]]
+
+template 
+void Test::method4() {
+$method4[[}]]
+  )cpp",
+  ExpectedHint{" // ~Test", "dtor"},
+  ExpectedHint{" // method1", "method1"},
+  ExpectedHint{" // method3", "method3"},
+  ExpectedHint{" // operator+", "opIdentity"},
+  ExpectedHint{" // operator bool", "opBool"},
+  ExpectedHint{" // Test::method2", "method2"},
+  ExpectedHint{" // Test::method4", "method4"});
+}
+
+TEST(BlockEndHints, Namespaces) {
+  assertBlockEndHints(
+  R"cpp(
+namespace {
+  void foo();
+$anon[[}]]
+
+namespace ns {
+  void bar();
+$ns[[}]]
+  )cpp",
+  ExpectedHint{" // namespace", "anon"},
+  ExpectedHint{" // namespace ns", "ns"});
+}
+
+TEST(BlockEndHints, Types) {
+  assertBlockEndHints(
+  R"cpp(
+struct S {
+$S[[};]]
+
+class C {
+$C[[};]]
+
+union U {
+$U[[};]]
+
+enum E1 {
+$E1[[};]]
+
+enum class E2 {
+$E2[[};]]
+  )cpp",
+  ExpectedHint{" // struct S", "S"}, ExpectedHint{" // class C", "C"},
+  ExpectedHint{" // union U", "U"}, ExpectedHint{" // enum E1", "E1"},
+  ExpectedHint{" // enum class E2", "E2"});
+}
+
+TEST(BlockEndHints, TrailingSemicolon) {
+  assertBlockEndHints(R"cpp(
+// The hint is placed after the trailing ';'
+struct S1 {
+$S1[[}  ;]]   
+
+// The hint is always placed in the same line with the closing '}'.
+// So in this case where ';' is missing, it is attached to '}'.
+struct S2 {
+$S2[[}]]
+
+;
+
+// No hint because only one trailing ';' is allowed
+struct S3 {
+};;
+
+// No hint because trailing ';' is

[PATCH] D155385: [clangd] enable unused-include warnings for standard library headers

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

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

Other  headers are still excluded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155385

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -73,10 +73,6 @@
 }
 
 TEST(IncludeCleaner, StdlibUnused) {
-  setIncludeCleanerAnalyzesStdlib(true);
-  auto Cleanup =
-  llvm::make_scope_exit([] { setIncludeCleanerAnalyzesStdlib(false); });
-
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -273,15 +273,6 @@
 init(CodeCompleteOptions().ImportInsertions),
 };
 
-opt IncludeCleanerStdlib{
-"include-cleaner-stdlib",
-cat(Features),
-desc("Apply include-cleaner analysis to standard library headers "
- "(immature!)"),
-init(false),
-Hidden,
-};
-
 opt HeaderInsertionDecorators{
 "header-insertion-decorators",
 cat(Features),
@@ -317,7 +308,7 @@
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
 RetiredFlag FoldingRanges("folding-ranges");
-
+RetiredFlag IncludeCleanerStdlib("include-cleaner-stdlib");
 
 opt LimitResults{
 "limit-results",
@@ -968,7 +959,6 @@
   };
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
 Opts.Encoding = ForceOffsetEncoding;
-  setIncludeCleanerAnalyzesStdlib(IncludeCleanerStdlib);
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -36,7 +36,6 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -54,10 +53,6 @@
 #include 
 
 namespace clang::clangd {
-
-static bool AnalyzeStdlib = false;
-void setIncludeCleanerAnalyzesStdlib(bool B) { AnalyzeStdlib = B; }
-
 namespace {
 
 bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) {
@@ -77,7 +72,7 @@
   // System headers are likely to be standard library headers.
   // Until we have good support for umbrella headers, don't warn about them.
   if (Inc.Written.front() == '<') {
-if (AnalyzeStdlib && tooling::stdlib::Header::named(Inc.Written))
+if (tooling::stdlib::Header::named(Inc.Written))
   return true;
 return false;
   }


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -73,10 +73,6 @@
 }
 
 TEST(IncludeCleaner, StdlibUnused) {
-  setIncludeCleanerAnalyzesStdlib(true);
-  auto Cleanup =
-  llvm::make_scope_exit([] { setIncludeCleanerAnalyzesStdlib(false); });
-
   auto TU = TestTU::withCode(R"cpp(
 #include 
 #include 
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -273,15 +273,6 @@
 init(CodeCompleteOptions().ImportInsertions),
 };
 
-opt IncludeCleanerStdlib{
-"include-cleaner-stdlib",
-cat(Features),
-desc("Apply include-cleaner analysis to standard library headers "
- "(immature!)"),
-init(false),
-Hidden,
-};
-
 opt HeaderInsertionDecorators{
 "header-insertion-decorators",
 cat(Features),
@@ -317,7 +308,7 @@
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
 RetiredFlag FoldingRanges("folding-ranges");
-
+RetiredFlag IncludeCleanerStdlib("include-cleaner-stdlib");
 
 opt LimitResults{
 "limit-results",
@@ -968,7 +959,6 @@
   };
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
 Opts.Encoding = ForceOffsetEncoding;
-  setIncludeCleanerAnalyzesStdlib(IncludeCleanerStdlib);
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
Index:

[PATCH] D155370: [CodeComplete] Don't emit parameters when not FunctionCanBeCall

2023-07-15 Thread Younan Zhang via Phabricator via cfe-commits

zyounan created this revision.
zyounan added reviewers: nridge, tom-anders, sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added a comment.
zyounan updated this revision to Diff 540687.
zyounan published this revision for review.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Changing the code completion string to `RK_Pattern` makes the completion 
clearer, in a way that showing candidates with signatures in the completion 
items if `FunctionCanBeCall` while a mere name otherwise.

F28256820: image.png 

F28256841: image.png 

(Comparing to that we always show signatures even if no parameters are produced 
when hit the item.)

F28256859: image.png  -> F28256863: 
image.png 

---

P.S. Despite the fact that I changed many associated tests, I still hope people 
don't rely on the former behavior so that this patch could be marked as NFC. :D

I discovered that patch when trying to figure out why clangd doesn't complete 
function parameters for members in global scope while it produces parameters 
when I'm inside a call expression. Regard to the heuristic itself, it's 
currently a bit inconvenient that I have to switch headers and sources back and 
forth and do copy-pastes when I'm writing member function definitions outside 
of a class. I think we could be more lenient here e.g., don't set the flag on 
if we're in the global scope. What do you guys think? (I'm working on another 
follow-up patch to address this, although it is not strightforward to tell 
which scope the building Declarator is in inside Sema. )


zyounan added a comment.

Format


This tries to avoid extra calculations in D137040 
.

In that patch the extra completion strings were dropped at the client
site, after function parameters became available, to implement the
heuristic. However, we can make the process a bit more terse. In the
context where a call isn't required, we could teach the Sema to emit
the completion string without additional parameters by changing the Decl
to a Pattern with the function name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155370

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/member-access.cpp
  clang/test/Index/complete-qualified.cpp
  clang/unittests/Sema/CodeCompleteTest.cpp

Index: clang/unittests/Sema/CodeCompleteTest.cpp
===
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -59,14 +59,12 @@
   unsigned NumResults) override {
 for (unsigned I = 0; I < NumResults; ++I) {
   auto R = Results[I];
-  if (R.Kind == CodeCompletionResult::RK_Declaration) {
-if (const auto *FD = llvm::dyn_cast(R.getDeclaration())) {
-  CompletedFunctionDecl D;
-  D.Name = FD->getNameAsString();
-  D.CanBeCall = R.FunctionCanBeCall;
-  D.IsStatic = FD->isStatic();
-  CompletedFuncDecls.emplace_back(std::move(D));
-}
+  if (const auto *FD = llvm::dyn_cast(R.getDeclaration())) {
+CompletedFunctionDecl D;
+D.Name = FD->getNameAsString();
+D.CanBeCall = R.FunctionCanBeCall;
+D.IsStatic = FD->isStatic();
+CompletedFuncDecls.emplace_back(std::move(D));
   }
 }
   }
Index: clang/test/Index/complete-qualified.cpp
===
--- clang/test/Index/complete-qualified.cpp
+++ clang/test/Index/complete-qualified.cpp
@@ -16,5 +16,6 @@
 // RUN: c-index-test -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: FieldDecl:{ResultType C}{TypedText c} (35)
 // CHECK-CC1: ClassDecl:{TypedText Foo} (35)
-// CHECK-CC1: CXXMethod:{ResultType Foo &}{TypedText operator=}{LeftParen (}{Placeholder const Foo &}{RightParen )}
-// CHECK-CC1: CXXDestructor:{ResultType void}{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
+// CHECK-CC1: CXXMethod:{TypedText operator=} (50)
+// CHECK-CC1: CXXMethod:{TypedText operator=} (50)
+// CHECK-CC1: CXXMethod:{TypedText ~Foo} (50)
Index: clang/test/CodeCompletion/member-access.cpp
===
--- clang/test/CodeCompletion/member-access.cpp
+++ clang/test/CodeCompletion/member-access.cpp
@@ -171,7 +171,7 @@
 template
 void dependentColonColonCompletion() {
   Template::staticFn();
-// CHECK-CC7: function : [#void#]function()
+// CHECK-CC7: Pattern : function
 // CHECK-CC7: Nested : Nested
 // CHECK-CC7: o1 : [#BaseTemplate#]o1
 // CHECK-CC7: o2 : [#BaseTemplate#]o2
Index: clang/lib/Sema/SemaCodeComplete.cpp

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-15 Thread sstwcw via Phabricator via cfe-commits

sstwcw updated this revision to Diff 540752.
sstwcw added a comment.

- Add back the const qualifier


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/WhitespaceManager.cpp
  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
@@ -1802,6 +1802,30 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[5], tok::comma, TT_Unknown);
   EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+
+  // String literals in concatenation.
+  Tokens = Annotate("x = {\"\"};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_VerilogStringInConcatenation);
+  Tokens = Annotate("x = {\"\", \"\"};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_VerilogStringInConcatenation);
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_VerilogStringInConcatenation);
+  Tokens = Annotate("x = '{{\"\"}};");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_VerilogStringInConcatenation);
+  // Cases where the string should not be annotated that type.  Fix the
+  // `TT_Unknown` if needed in the future.
+  Tokens = Annotate("x = {\"\" == \"\"};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_Unknown);
+  Tokens = Annotate("x = {(\"\")};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::string_literal, TT_Unknown);
+  Tokens = Annotate("x = '{\"\"};");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::string_literal, TT_Unknown);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -1155,6 +1155,66 @@
   verifyFormat("{< 0 && Changes[i - 1].OriginalWhitespaceRange.getBegin() ==
- C.OriginalWhitespaceRange.getBegin()) {
-  // Do not generate two replacements for the same location.
-  continue;
+if (i > 0) {
+  auto Last = Changes[i - 1].OriginalWhitespaceRange;
+  auto New = Changes[i].OriginalWhitespaceRange;
+  // Do not generate two replacements for the same location.  As a special
+  // case, it is allowed if there is a replacement for the empty range
+  // between 2 tokens and another non-empty range at the start of the second
+  // token.
+  //
+  // An original whitespace range that is empty is encountered when 2 tokens
+  // have no whitespace in between in the input.  It does not matter whether
+  // whitespace is to be added.  For example, if the input is `foo();`,
+  // there will be a replacement for the range between every consecutive
+  // pair of tokens.
+  //
+  //  A replacement at the start of a token can be added by
+  // `BreakableVerilogStringLiteral::insertBreak` when it adds braces around
+  // the string literal.  Say the line `x("long string");` is to become the
+  // 2 lines `x({"long ",` and `   "string"});`.  There will be a
+  // replacement for the empty range between the parenthesis and the string
+  // and another replacement for the quote character.
+  if (Last.getBegin() == New.getBegin() &&
+  (Last.getEnd() != Last.getBegin() ||
+   New.getEnd() == New.getBegin())) {
+continue;
+  }
 }
 if (C.CreateReplacement) {
   std::string ReplacementText = C.PreviousLinePostfix;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -862,6 +862,11 @@
 OpeningBrace.Previous->is(TT_JsTypeColon)) {
   Contexts.back().IsExpression = false;
 }
+if (Style.isVerilog() &&
+(!OpeningBrace.getPreviousNonComment() ||
+ OpeningBrace.getPreviousNonComment()->isNot(Keywords.kw_apostrophe))) {
+  Contexts.back().VerilogMayBeConcatenation = true;
+}
 
 unsigned CommaCount = 0;
 while (CurrentToken) {
@@ -1736,6 +1741,9 @@
 bool InCpp11AttributeSpecifier = false;
 bool InCSharpAttributeSpecifier = false;
 bo

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-15 Thread sstwcw via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/ContinuationIndenter.cpp:2247
bool DryRun, bool Strict) {
-  std::unique_ptr Token =
+  std::unique_ptr Token =
   createBreakableToken(Current, State, AllowBreak);

HazardyKnusperkeks wrote:
> This const can be readded, or not?
I forgot to add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

2023-07-15 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik created this revision.
shafik added reviewers: aaron.ballman, rsmith, cor3ntin.
Herald added a project: All.
shafik requested review of this revision.

There are some cases during member lookup we are aggressively suppressing 
diagnostics when we should just be suppressing access control diagnostic.

In this PR I add the ability to simply suppress access diagnostics while not 
suppressing ambiguous lookup diagnostics.

Fixes: https://github.com/llvm/llvm-project/issues/22413 
https://github.com/llvm/llvm-project/issues/29942
https://github.com/llvm/llvm-project/issues/35574


https://reviews.llvm.org/D155387

Files:
  clang/include/clang/Sema/Lookup.h
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
  clang/test/CXX/class.derived/class.member.lookup/p11.cpp

Index: clang/test/CXX/class.derived/class.member.lookup/p11.cpp
===
--- /dev/null
+++ clang/test/CXX/class.derived/class.member.lookup/p11.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct B1 {
+  void f();
+  static void f(int);
+  int i; // expected-note 2{{member found by ambiguous name lookup}}
+};
+struct B2 {
+  void f(double);
+};
+struct I1: B1 { };
+struct I2: B1 { };
+
+struct D: I1, I2, B2 {
+  using B1::f;
+  using B2::f;
+  void g() {
+f(); // expected-error {{ambiguous conversion from derived class 'D' to base class 'B1'}}
+f(0); // ok
+f(0.0); // ok
+// FIXME next line should be well-formed
+int B1::* mpB1 = &D::i; // expected-error {{non-static member 'i' found in multiple base-class subobjects of type 'B1'}}
+int D::* mpD = &D::i; // expected-error {{non-static member 'i' found in multiple base-class subobjects of type 'B1'}}
+  }
+};
Index: clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
===
--- /dev/null
+++ clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {
+  void operator()(int); // expected-note {{member found by ambiguous name lookup}}
+  void f(int); // expected-note {{member found by ambiguous name lookup}}
+};
+struct B {
+  void operator()(); // expected-note {{member found by ambiguous name lookup}}
+  void f() {} // expected-note {{member found by ambiguous name lookup}}
+};
+
+struct C : A, B {};
+
+int f() {
+C c;
+c(); // expected-error {{member 'operator()' found in multiple base classes of different types}}
+c.f(10); //expected-error {{member 'f' found in multiple base classes of different types}}
+return 0;
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -593,7 +593,7 @@
   // postfix-expression and does not name a class template, the name
   // found in the class of the object expression is used, otherwise
   FoundOuter.clear();
-} else if (!Found.isSuppressingDiagnostics()) {
+} else if (!Found.isSuppressingAmbiguousDiagnostics()) {
   //   - if the name found is a class template, it must refer to the same
   // entity as the one found in the class of the object expression,
   // otherwise the program is ill-formed.
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -14927,7 +14927,7 @@
   const auto *Record = Object.get()->getType()->castAs();
   LookupResult R(*this, OpName, LParenLoc, LookupOrdinaryName);
   LookupQualifiedName(R, Record->getDecl());
-  R.suppressDiagnostics();
+  R.suppressAccessDiagnostics();
 
   for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end();
Oper != OperEnd; ++Oper) {
Index: clang/include/clang/Sema/Lookup.h
===
--- clang/include/clang/Sema/Lookup.h
+++ clang/include/clang/Sema/Lookup.h
@@ -148,20 +148,22 @@
   : SemaPtr(&SemaRef), NameInfo(NameInfo), LookupKind(LookupKind),
 Redecl(Redecl != Sema::NotForRedeclaration),
 ExternalRedecl(Redecl == Sema::ForExternalRedeclaration),
-Diagnose(Redecl == Sema::NotForRedeclaration) {
+DiagnoseAccess(Redecl == Sema::NotForRedeclaration),
+DiagnoseAmbiguous(Redecl == Sema::NotForRedeclaration) {
 configure();
   }
 
   // TODO: consider whether this constructor should be restricted to take
   // as input a const IdentifierInfo* (instead of Name),
   // forcing other cases towards the constructor taking a DNInfo.
-  LookupResult(Sema &SemaRef, DeclarationName Name,
-   SourceLocation NameLoc, Sema::LookupNameKind LookupKind,
+  LookupResult(Sema &SemaRef, DeclarationName Name, SourceLocation NameLoc,
+   Sema

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

2023-07-15 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik added a comment.

It looks like the paragraphs in `class.member.lookup` have changed a lot and so 
the tests don't totally match in that directory. Fixing that seems like a big 
piece of work but maybe worth doing.




Comment at: clang/include/clang/Sema/Lookup.h:152
+DiagnoseAccess(Redecl == Sema::NotForRedeclaration),
+DiagnoseAmbiguous(Redecl == Sema::NotForRedeclaration) {
 configure();

Note sure about this, alternatively I could set it to what `DiagnoseAccess` is 
set to as well.



Comment at: clang/lib/Sema/SemaOverload.cpp:14930
   LookupQualifiedName(R, Record->getDecl());
-  R.suppressDiagnostics();
+  R.suppressAccessDiagnostics();
 

I was a bit conservative where I changed this but maybe we should do this 
everywhere.


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

https://reviews.llvm.org/D155387

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

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 540760.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D155369

Files:
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/Sema/constant-builtins-fmin.cpp

Index: clang/test/Sema/constant-builtins-fmin.cpp
===
--- clang/test/Sema/constant-builtins-fmin.cpp
+++ clang/test/Sema/constant-builtins-fmin.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s
 // expected-no-diagnostics
 
 constexpr double NaN = __builtin_nan("");
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -158,6 +158,20 @@
   return true;
 }
 
+/// Defined as __builtin_isnan(...), to accommodate the fact that it can
+/// take a float, double, long double, etc.
+/// But for us, that's all a Floating anyway.
+static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
+  const InterpFrame *Frame, const Function *F) {
+  // FIXME: We are not going through getParam() here, because the function
+  // doesn't have any parameters. Wait for a pattern to emerge and maybe
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek();
+
+  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
@@ -213,6 +227,11 @@
   return Ret(S, OpPC, Dummy);
 break;
 
+  case Builtin::BI__builtin_isnan:
+if (interp__builtin_isnan(S, OpPC, Frame, F))
+  return Ret(S, OpPC, Dummy);
+break;
+
   default:
 return false;
   }
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -200,6 +200,9 @@
 // Returning values
 //===--===//
 
+/// Pop arguments of builtins defined as func-name(...).
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC);
+
 template ::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop();
@@ -216,8 +219,16 @@
   }
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
-S.Current->popArgs();
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
+// Certain builtin functions are declared as func-name(...), so the
+// parameters are checked in Sema and only available through the CallExpr.
+// The interp::Function we create for them has 0 parameters, so we need to
+// remove them from the stack by checking the CallExpr.
+if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+  popBuiltinArgs(S, PC);
+else
+  S.Current->popArgs();
+  }
 
   if (InterpFrame *Caller = S.Current->Caller) {
 PC = S.Current->getRetPC();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -122,6 +122,18 @@
 namespace clang {
 namespace interp {
 
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC) {
+  assert(S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()));
+  const Expr *E = S.Current->getExpr(OpPC);
+  assert(isa(E));
+  const CallExpr *CE = cast(E);
+  for (const auto *A : llvm::reverse(CE->arguments())) {
+PrimType Ty = S.getContext().classify(A->getType()).value_or(PT_Ptr);
+TYPE_SWITCH(Ty, S.Stk.discard());
+  }
+  return true;
+}
+
 bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   if (!Ptr.isExtern())
 return true;
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -171,6 +171,12 @@
 
   unsigned getBuiltinID() const { return F->getBuiltinID(); }
 
+  bool isBuiltin() const { return F->getBuiltinID() != 0; }
+
+  /// Does this function need its arguments to be classified at runtime
+  /// rather than at bytecode-compile-time?
+  bool needsRuntimeArgPop(const ASTContext &Ctx) const;
+
   unsigned getNumParams() const { return ParamTypes.size(); }
 
   unsigned getParamOffset(unsigned ParamIndex) const {
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/In

[PATCH] D155392: [clangd] add a config knob to disable modules

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

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

We don't yet have any intentional modules support:
https://github.com/clangd/clangd/issues/1293

However if the user has modules-related flags in their CDB, modules will still
be enabled inside clang.

- for some configurations this works well enough, apparently. We should allow 
them to keep using this config if they wish, though unsupported.
- for other configurations this is crashy/buggy. We can't really control the 
behavior/stability, so long-term this should not be the default.
- there is no flag to disable modules if when using `-std=c++20`. Since clangd 
does not support modules, we should provide a mechanism.
- in future we're likely to explicitly add modules support. This will not be a 
simple passthrough of flags to clang, so we'll need another mode.

This patch adds a config option:

  CompileFlags:
NaiveModules: false

For now, the default is true, and modules are internally enabled/disabled
according to compile flags. When set to false, we force Modules/CPlusPlusModules
lang opts off.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155392

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/ModulesTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -7,9 +7,11 @@
 //===--===//
 
 #include "Annotations.h"
+#include "Config.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/SymbolCollector.h"
+#include "support/Context.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -28,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1918,6 +1921,10 @@
 
 // Regression test for a crash-bug we used to have.
 TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
+  Config EnableModules;
+  EnableModules.CompileFlags.NaiveModules = true;
+  WithContextValue WithCfg(Config::Key, std::move(EnableModules));
+
   auto TU = TestTU::withCode(R"cpp(#include "bar.h")cpp");
   TU.AdditionalFiles["bar.h"] = R"cpp(
 #include "foo.h"
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -6,8 +6,10 @@
 //
 //===--===//
 
+#include "Config.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "support/Context.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,7 +20,21 @@
 namespace clangd {
 namespace {
 
-TEST(Modules, TextualIncludeInPreamble) {
+enum class ModulesMode { Disabled, Naive };
+Config makeModuleConfig(ModulesMode M) {
+  Config EnableModules;
+  EnableModules.CompileFlags.NaiveModules = M == ModulesMode::Naive;
+  return EnableModules;
+}
+
+class ModulesTest : public ::testing::TestWithParam {
+  WithContextValue WithCfg;
+
+protected:
+  ModulesTest() : WithCfg(Config::Key, makeModuleConfig(GetParam())) {}
+};
+
+TEST_P(ModulesTest, TextualIncludeInPreamble) {
   TestTU TU = TestTU::withCode(R"cpp(
 #include "Textual.h"
 
@@ -40,7 +56,7 @@
 
 // Verify that visibility of AST nodes belonging to modules, but loaded from
 // preamble PCH, is restored.
-TEST(Modules, PreambleBuildVisibility) {
+TEST_P(ModulesTest, PreambleBuildVisibility) {
   TestTU TU = TestTU::withCode(R"cpp(
 #include "module.h"
 
@@ -63,7 +79,7 @@
   EXPECT_TRUE(TU.build().getDiagnostics().empty());
 }
 
-TEST(Modules, Diagnostic) {
+TEST_P(ModulesTest, Diagnostic) {
   // Produce a diagnostic while building an implicit module. Use
   // -fmodules-strict-decluse, but any non-silenced diagnostic will do.
   TestTU TU = TestTU::withCode(R"cpp(
@@ -92,7 +108,7 @@
 }
 
 // Unknown module formats are a fatal failure for clang. Ensure we don't crash.
-TEST(Modules, UnknownFormat) {
+TEST_P(ModulesTest, UnknownFormat) {
   TestTU TU = TestTU::withCode(R"(#include "modular.h")");
   TU.OverlayRealFileSystemForModules = true;
   TU.ExtraArgs.push_back("-Xclang");
@@ -109,6 +125,25 @@
   // Test that we do not crash.
   TU.build(

[PATCH] D155392: [clangd] add a config knob to disable modules

2023-07-15 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:497
 TEST(DocumentSymbols, ExportContext) {
+  Config EnableModules;
+  EnableModules.CompileFlags.NaiveModules = true;

For existing tests of how modules interacts with other features, testing these 
with modules-enabled seems sufficient.

For tests that were specifically about modules, I think it makes sense to test 
in both modes (behavior may differ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155392

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

[PATCH] D155393: [clang][Interp] Implement __builtin_isfpclass

2023-07-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155393

Files:
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/lib/AST/Interp/PrimType.h
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -73,3 +73,46 @@
   static_assert(__builtin_isnormal(1.0), "");
   static_assert(!__builtin_isnormal(__builtin_inf()), "");
 }
+
+namespace isfpclass {
+  char isfpclass_inf_pos_0[__builtin_isfpclass(__builtin_inf(), 0x0200) ? 1 : -1]; // fcPosInf
+  char isfpclass_inf_pos_1[!__builtin_isfpclass(__builtin_inff(), 0x0004) ? 1 : -1]; // fcNegInf
+  char isfpclass_inf_pos_2[__builtin_isfpclass(__builtin_infl(), 0x0207) ? 1 : -1]; // fcSNan|fcQNan|fcNegInf|fcPosInf
+  char isfpclass_inf_pos_3[!__builtin_isfpclass(__builtin_inf(), 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_pos_0[__builtin_isfpclass(1.0, 0x0100) ? 1 : -1]; // fcPosNormal
+  char isfpclass_pos_1[!__builtin_isfpclass(1.0f, 0x0008) ? 1 : -1]; // fcNegNormal
+  char isfpclass_pos_2[__builtin_isfpclass(1.0L, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_pos_3[!__builtin_isfpclass(1.0, 0x0003) ? 1 : -1]; // fcSNan|fcQNan
+  char isfpclass_pdenorm_0[__builtin_isfpclass(1.0e-40f, 0x0080) ? 1 : -1]; // fcPosSubnormal
+  char isfpclass_pdenorm_1[__builtin_isfpclass(1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_pdenorm_2[!__builtin_isfpclass(1.0e-40f, 0x003C) ? 1 : -1]; // fcNegative
+  char isfpclass_pdenorm_3[!__builtin_isfpclass(1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+  char isfpclass_pzero_0  [__builtin_isfpclass(0.0f, 0x0060) ? 1 : -1]; // fcZero
+  char isfpclass_pzero_1  [__builtin_isfpclass(0.0, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_pzero_2  [!__builtin_isfpclass(0.0L, 0x0020) ? 1 : -1]; // fcNegZero
+  char isfpclass_pzero_3  [!__builtin_isfpclass(0.0, 0x0003) ? 1 : -1]; // fcNan
+  char isfpclass_nzero_0  [__builtin_isfpclass(-0.0f, 0x0060) ? 1 : -1]; // fcZero
+  char isfpclass_nzero_1  [__builtin_isfpclass(-0.0, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_nzero_2  [!__builtin_isfpclass(-0.0L, 0x0040) ? 1 : -1]; // fcPosZero
+  char isfpclass_nzero_3  [!__builtin_isfpclass(-0.0, 0x0003) ? 1 : -1]; // fcNan
+  char isfpclass_ndenorm_0[__builtin_isfpclass(-1.0e-40f, 0x0010) ? 1 : -1]; // fcNegSubnormal
+  char isfpclass_ndenorm_1[__builtin_isfpclass(-1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_ndenorm_2[!__builtin_isfpclass(-1.0e-40f, 0x03C0) ? 1 : -1]; // fcPositive
+  char isfpclass_ndenorm_3[!__builtin_isfpclass(-1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+  char isfpclass_neg_0[__builtin_isfpclass(-1.0, 0x0008) ? 1 : -1]; // fcNegNormal
+  char isfpclass_neg_1[!__builtin_isfpclass(-1.0f, 0x00100) ? 1 : -1]; // fcPosNormal
+  char isfpclass_neg_2[__builtin_isfpclass(-1.0L, 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_neg_3[!__builtin_isfpclass(-1.0, 0x0003) ? 1 : -1]; // fcSNan|fcQNan
+  char isfpclass_inf_neg_0[__builtin_isfpclass(-__builtin_inf(), 0x0004) ? 1 : -1]; // fcNegInf
+  char isfpclass_inf_neg_1[!__builtin_isfpclass(-__builtin_inff(), 0x0200) ? 1 : -1]; // fcPosInf
+  char isfpclass_inf_neg_2[__builtin_isfpclass(-__builtin_infl(), 0x0207) ? 1 : -1]; // ~fcFinite
+  char isfpclass_inf_neg_3[!__builtin_isfpclass(-__builtin_inf(), 0x03C0) ? 1 : -1]; // fcPositive
+  char isfpclass_qnan_0   [__builtin_isfpclass(__builtin_nan(""), 0x0002) ? 1 : -1]; // fcQNan
+  char isfpclass_qnan_1   [!__builtin_isfpclass(__builtin_nanf(""), 0x0001) ? 1 : -1]; // fcSNan
+  char isfpclass_qnan_2   [__builtin_isfpclass(__builtin_nanl(""), 0x0207) ? 1 : -1]; // ~fcFinite
+  char isfpclass_qnan_3   [!__builtin_isfpclass(__builtin_nan(""), 0x01F8) ? 1 : -1]; // fcFinite
+  char isfpclass_snan_0   [__builtin_isfpclass(__builtin_nansf(""), 0x0001) ? 1 : -1]; // fcSNan
+  char isfpclass_snan_1   [!__builtin_isfpclass(__builtin_nans(""), 0x0002) ? 1 : -1]; // fcQNan
+  char isfpclass_snan_2   [__builtin_isfpclass(__builtin_nansl(""), 0x0207) ? 1 : -1]; // ~fcFinite
+  char isfpclass_snan_3   [!__builtin_isfpclass(__builtin_nans(""), 0x01F8) ? 1 : -1]; // fcFinite
+}
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -119,6 +119,23 @@
 }  \
   } while (0)
 
+#define INT_TYPE_SWITCH(Expr, B)   \
+  do {

72 matches

Mail list logo