[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

This triggers failed asserts for me:

  $ cat repro.c 
  static float strtof(char *, char *) {}
  void a() { strtof(a, 0); }
  $ clang -target x86_64-w64-mingw32 -w -c repro.c -O3
  clang: ../lib/Analysis/CGSCCPassManager.cpp:958: 
updateCGAndAnalysisManagerForPass(llvm::LazyCallGraph&, 
llvm::LazyCallGraph::SCC&, llvm::LazyCallGraph::Node&, 
llvm::CGSCCAnalysisManager&, llvm::CGSCCUpdateResult&, 
llvm::FunctionAnalysisManager&, bool)::: Assertion 
`RefereeN && "Visited function should already have an associated node"' failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-26 Thread Dong-hee Na via Phabricator via cfe-commits
corona10 marked an inline comment as done.
corona10 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp:1059
 
   priority_queue.emplace(Foo(13));
   // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: unnecessary temporary object 
created while calling emplace

njames93 wrote:
> Can you please add a test like this, where emplace is called with a temporary 
> object when the container type is an alias.
Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132640

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


[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

2022-08-26 Thread Rong Xu via Phabricator via cfe-commits
xur added inline comments.



Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:589
+
+if (SampleProfileHasFUnique) {
+  // If profile also uses funqiue, nothing to do here.

tmsriram wrote:
> Maybe rewrite this slightly as:
> 
> //  If Sample Profile and Instrumented Profile do not agree on symbol 
> uniqification.
> if (SampleProfileHasFunique != ProfileHasFUnique) {
>if (ProfileHasFUnique) {
>   // trim
>} else {
>   // Build Map
>}
> }
Good point. I will change to this form.


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

https://reviews.llvm.org/D132600

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


[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

2022-08-26 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D127973#3749153 , @isuckatcs wrote:

> @steakhal
>
> Can you send me a snippet please, which reproduces this issue? For me the 
> egraph rewriter works fine.

I had the time for reducing it. Luckily it wasn't that bad.

  digraph "Exploded Graph" {
label="Exploded Graph";
  
Node0x7fda6c098a40 [shape=record,label="{\{ \"state_id\": 
0,\l  \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 20, \"dst_id\": 19, \"terminator\": null, \"term_kind\": 
null, \"tag\": null, \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0 
\},\l\{ \"kind\": \"BlockEntrance\", \"block_id\": 19, 
\"tag\": null, \"node_id\": 2, \"is_sink\": 0, \"has_report\": 0 
\},\l\{ \"kind\": \"Statement\", \"stmt_kind\": 
\"DeclRefExpr\", \"stmt_id\": 7381222, \"pointer\": \"0x7fda78fd1e50\", 
\"pretty\": \"storage\", \"location\": \{ \"line\": 12, \"column\": 28, 
\"file\": \"/redacted/file.cc\" \}, \"stmt_point_kind\": 
\"PreStmtPurgeDeadSymbols\", \"tag\": \"ExprEngine : Clean Node\", \"node_id\": 
3, \"is_sink\": 0, \"has_report\": 0 
\}\l  ],\l  \"program_state\": 
\{\l\"store\": 
null,\l\"environment\": 
null,\l\"constraints\": 
null,\l\"equivalence_classes\": 
null,\l\"disequality_info\": 
null,\l\"dynamic_types\": 
null,\l\"dynamic_casts\": 
null,\l\"constructing_objects\": 
null,\l\"checker_messages\": 
null\l  \}\l\}\l}"];
Node0x7fda6c098a40 -> Node0x7fda6c098cb0;
Node0x7fda6c098cb0 [shape=record,label="{\{ \"state_id\": 
78,\l  \"program_points\": [\l\{ \"kind\": 
\"Statement\", \"stmt_kind\": \"DeclRefExpr\", \"stmt_id\": 7381222, 
\"pointer\": \"0x7fda78fd1e50\", \"pretty\": \"storage\", \"location\": \{ 
\"line\": 12, \"column\": 28, \"file\": \"/redacted/file.cc\" \}, 
\"stmt_point_kind\": \"PostLValue\", \"tag\": null, \"node_id\": 4, 
\"is_sink\": 0, \"has_report\": 0 \},\l\{ \"kind\": 
\"Statement\", \"stmt_kind\": \"DeclRefExpr\", \"stmt_id\": 7381222, 
\"pointer\": \"0x7fda78fd1e50\", \"pretty\": \"storage\", \"location\": \{ 
\"line\": 12, \"column\": 28, \"file\": \"/redacted/file.cc\" \}, 
\"stmt_point_kind\": \"PostStmt\", \"tag\": null, \"node_id\": 5, \"is_sink\": 
0, \"has_report\": 0 \}\l  ],\l  \"program_state\": 
\{\l\"store\": 
null,\l\"environment\": \{ \"pointer\": 
\"0x7fda6c02e890\", \"items\": [\l  \{ 
\"lctx_id\": 1, \"location_context\": \"#0 Call\", \"calling\": 
\"redacted::Fun\", \"location\": null, \"items\": 
[\l\{ \"stmt_id\": 7381222, 
\"pretty\": \"storage\", \"value\": \"&storage\" 
\}\l  ]\}\l]\},\l\"constraints\":
 null,\l\"equivalence_classes\": 
null,\l\"disequality_info\": 
null,\l\"dynamic_types\": 
null,\l\"dynamic_casts\": 
null,\l\"constructing_objects\": 
null,\l\"checker_messages\": 
null\l  \}\l\}\l}"];
Node0x7fda6c098cb0 -> Node0x7fda6c099160;
Node0x7fda6c099160 [shape=record,label="{\{ \"state_id\": 
228,\l  \"program_points\": [\l\{ \"kind\": 
\"Statement\", \"stmt_kind\": \"MemberExpr\", \"stmt_id\": 7381229, 
\"pointer\": \"0x7fda78fd1e88\", \"pretty\": \"storage-\>Comp\", \"location\": 
\{ \"line\": 12, \"column\": 28, \"file\": \"/redacted/file.cc\" \}, 
\"stmt_point_kind\": \"PostStmt\", \"tag\": null, \"node_id\": 8, \"is_sink\": 
0, \"has_report\": 0 \}\l  ],\l  \"program_state\": 
\{\l\"store\": 
null,\l\"environment\": \{ \"pointer\": 
\"0x7fda6c02e890\", \"items\": [\l  \{ 
\"lctx_id\": 1, \"location_context\": \"#0 Call\", \"calling\": 
\"redacted::Fun\", \"location\": null, \"items\": 
[\l\{ \"stmt_id\": 7381222, 
\"pretty\": \"storage\", \"value\": \"&storage\" 
\},\l\{ \"stmt_id\": 7381226, 
\"pretty\": \"storage\", \"value\": \"&SymRegion\{reg_$0\\}\" \},\l\{ 
\"stmt_id\": 7381229, \"pretty\": \"storage-\>Comp\", \"value\": 
\"&code\{Comp\}\" 
\}\l  ]\}\l]\},\l\"constraints\":
 null,\l\"equivalence_classes\": 
null,\l\"disequality_info\": 
null,\l\"dynamic_types\": 
null,\l\"dynamic_casts\": 
null,\l\"constructing_objects\": 
null,\l\"checker_messages\": 
null\l  \}\l\}\l}"];
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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


[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

2022-08-26 Thread Rong Xu via Phabricator via cfe-commits
xur updated this revision to Diff 455816.
xur added a comment.

Integrated Sriraman's suggestion.


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

https://reviews.llvm.org/D132600

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/ProfileData/SampleProf.h
  llvm/test/tools/llvm-profdata/Inputs/FUnique.afdotext
  llvm/test/tools/llvm-profdata/Inputs/FUnique.proftext
  llvm/test/tools/llvm-profdata/Inputs/NoFUnique.afdotext
  llvm/test/tools/llvm-profdata/Inputs/NoFUnique.proftext
  llvm/test/tools/llvm-profdata/suppl-instr-with-sample-static-func.test
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -31,6 +31,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MD5.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ThreadPool.h"
@@ -42,6 +43,10 @@
 
 using namespace llvm;
 
+// We use this string to indicate that there are
+// multiple static functions map to the same name.
+const std::string DuplicateNameStr = "";
+
 enum ProfileFormat {
   PF_None = 0,
   PF_Text,
@@ -478,7 +483,7 @@
   ZeroCounterRatio = (float)ZeroCntNum / CntNum;
 }
 
-/// Either set all the counters in the instr profile entry \p IFE to -1
+// Either set all the counters in the instr profile entry \p IFE to -1
 /// in order to drop the profile or scale up the counters in \p IFP to
 /// be above hot threshold. We use the ratio of zero counters in the
 /// profile of a function to decide the profile is helpful or harmful
@@ -539,7 +544,73 @@
unsigned InstrProfColdThreshold) {
   // Function to its entry in instr profile.
   StringMap InstrProfileMap;
+  StringMap StaticFuncMap;
   InstrProfSummaryBuilder IPBuilder(ProfileSummaryBuilder::DefaultCutoffs);
+
+  auto checkSampleProfileHasFUnique = [&Reader]() {
+for (const auto &PD : Reader->getProfiles()) {
+  auto &FContext = PD.first;
+  if (FContext.toString().find(FunctionSamples::UniqSuffix) !=
+  std::string::npos) {
+return true;
+  }
+}
+return false;
+  };
+
+  bool SampleProfileHasFUnique = checkSampleProfileHasFUnique();
+
+  auto buildStaticFuncMap = [&StaticFuncMap,
+ SampleProfileHasFUnique](const StringRef Name) {
+std::string Prefixes[] = {".cpp:", "cc:", ".c:", ".hpp:", ".h:"};
+size_t PrefixPos = StringRef::npos;
+for (auto &Prefix : Prefixes) {
+  PrefixPos = Name.find_insensitive(Prefix);
+  if (PrefixPos == StringRef::npos)
+continue;
+  PrefixPos += Prefix.size();
+  break;
+}
+
+if (PrefixPos == StringRef::npos) {
+  return;
+}
+
+StringRef NewName = Name.drop_front(PrefixPos);
+StringRef FName = Name.substr(0, PrefixPos - 1);
+if (NewName.size() == 0) {
+  return;
+}
+
+// This name should have a static linkage.
+size_t PostfixPos = NewName.find(FunctionSamples::UniqSuffix);
+bool ProfileHasFUnique = (PostfixPos != StringRef::npos);
+
+// If sample profile and instrumented profile do not agree on symbol
+// uniqification.
+if (SampleProfileHasFUnique != ProfileHasFUnique) {
+  // If instrumented profile uses -funique-internal-linakge-symbols,
+  // we need to trim the name.
+  if (ProfileHasFUnique) {
+NewName = NewName.substr(0, PostfixPos);
+  } else {
+// If sample profile uses -funique-internal-linakge-symbols,
+// we build the map.
+std::string NStr =
+NewName.str() + getUniqueInternalLinkagePostfix(FName);
+NewName = StringRef(NStr);
+StaticFuncMap[NewName] = Name;
+return;
+  }
+}
+
+if (StaticFuncMap.find(NewName) == StaticFuncMap.end()) {
+  StaticFuncMap[NewName] = Name;
+} else {
+  StaticFuncMap[NewName] = DuplicateNameStr;
+}
+  };
+
   for (auto &PD : WC->Writer.getProfileData()) {
 // Populate IPBuilder.
 for (const auto &PDV : PD.getValue()) {
@@ -553,7 +624,9 @@
 
 // Initialize InstrProfileMap.
 InstrProfRecord *R = &PD.getValue().begin()->second;
-InstrProfileMap[PD.getKey()] = InstrProfileEntry(R);
+StringRef FullName = PD.getKey();
+InstrProfileMap[FullName] = InstrProfileEntry(R);
+buildStaticFuncMap(FullName);
   }
 
   ProfileSummary InstrPS = *IPBuilder.getSummary();
@@ -581,16 +654,27 @@
   // Find hot/warm functions in sample profile which is cold in instr profile
   // and adjust the profiles of those functions in the instr profile.
   for (const auto &PD : Reader->getProfiles()) {
-auto &FContext = PD.first;
 const sampleprof::FunctionSamples &FS = PD.second;
+if (FS.getMaxCountInside() <= ColdSampleThreshold ||
+FS.

[PATCH] D130207: [HLSL] Move DXIL validation version out of ModuleFlags

2022-08-26 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 455817.
python3kgae marked 3 inline comments as done.
python3kgae added a comment.

Remove empty function and replace auto with correct type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130207

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/test/CodeGenHLSL/validator_version.hlsl
  llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
  llvm/test/CodeGen/DirectX/dxil_ver.ll

Index: llvm/test/CodeGen/DirectX/dxil_ver.ll
===
--- llvm/test/CodeGen/DirectX/dxil_ver.ll
+++ llvm/test/CodeGen/DirectX/dxil_ver.ll
@@ -3,18 +3,18 @@
 target triple = "dxil-pc-shadermodel6.3-library"
 
 ; Make sure dx.valver metadata is generated.
-; CHECK:!dx.valver = !{![[valver:[0-9]+]]}
+; CHECK-DAG:!dx.valver = !{![[valver:[0-9]+]]}
 ; Make sure module flags still exist and only have 1 operand left.
-; CHECK:!llvm.module.flags = !{{{![0-9]}}}
+; CHECK-DAG:!llvm.module.flags = !{{{![0-9]}}}
 ; Make sure validator version is 1.1.
-; CHECK:![[valver]] = !{i32 1, i32 1}
+; CHECK-DAG:![[valver]] = !{i32 1, i32 1}
 ; Make sure wchar_size still exist.
-; CHECK:!{i32 1, !"wchar_size", i32 4}
+; CHECK-DAG:!{i32 1, !"wchar_size", i32 4}
 
-!llvm.module.flags = !{!0, !1}
-!llvm.ident = !{!3}
+!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
+!llvm.ident = !{!2}
 
 !0 = !{i32 1, !"wchar_size", i32 4}
-!1 = !{i32 6, !"dx.valver", !2}
-!2 = !{i32 1, i32 1}
-!3 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project 71de12113a0661649ecb2f533fba4a2818a1ad68)"}
+!1 = !{i32 1, i32 1}
+!2 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project 71de12113a0661649ecb2f533fba4a2818a1ad68)"}
Index: llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
===
--- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -58,32 +58,6 @@
   return VersionTuple(Major, Minor);
 }
 
-static void cleanModuleFlags(Module &M) {
-  constexpr StringLiteral DeadKeys[] = {ValVerKey};
-  // Collect DeadKeys in ModuleFlags.
-  StringSet<> DeadKeySet;
-  for (auto &Key : DeadKeys) {
-if (M.getModuleFlag(Key))
-  DeadKeySet.insert(Key);
-  }
-  if (DeadKeySet.empty())
-return;
-
-  SmallVector ModuleFlags;
-  M.getModuleFlagsMetadata(ModuleFlags);
-  NamedMDNode *MDFlags = M.getModuleFlagsMetadata();
-  MDFlags->eraseFromParent();
-  // Add ModuleFlag which not dead.
-  for (auto &Flag : ModuleFlags) {
-StringRef Key = Flag.Key->getString();
-if (DeadKeySet.contains(Key))
-  continue;
-M.addModuleFlag(Flag.Behavior, Key, Flag.Val);
-  }
-}
-
-static void cleanModule(Module &M) { cleanModuleFlags(M); }
-
 namespace {
 class DXILTranslateMetadata : public ModulePass {
 public:
@@ -101,13 +75,12 @@
 } // namespace
 
 bool DXILTranslateMetadata::runOnModule(Module &M) {
-  if (MDNode *ValVerMD = cast_or_null(M.getModuleFlag(ValVerKey))) {
-auto ValVer = loadDXILValidatorVersion(ValVerMD);
+  if (NamedMDNode *ValVerMD = M.getNamedMetadata(ValVerKey)) {
+VersionTuple ValVer = loadDXILValidatorVersion(ValVerMD->getOperand(0));
 if (!ValVer.empty())
   ValidatorVer = ValVer;
   }
   emitDXILValidatorVersion(M, ValidatorVer);
-  cleanModule(M);
   return false;
 }
 
Index: clang/test/CodeGenHLSL/validator_version.hlsl
===
--- clang/test/CodeGenHLSL/validator_version.hlsl
+++ clang/test/CodeGenHLSL/validator_version.hlsl
@@ -1,8 +1,11 @@
 // RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -S -emit-llvm -xhlsl -validator-version 1.1 -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple spirv32 -S -emit-llvm -xhlsl -validator-version 1.1 -o - %s | FileCheck %s --check-prefix=NOT_DXIL
 
-// CHECK:!"dx.valver", ![[valver:[0-9]+]]}
+// CHECK:!dx.valver = !{![[valver:[0-9]+]]}
 // CHECK:![[valver]] = !{i32 1, i32 1}
 
+// NOT_DXIL-NOT:!dx.valver
+
 float bar(float a, float b);
 
 float foo(float a, float b) {
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -42,16 +42,18 @@
   IRBuilder<> B(M.getContext());
   MDNode *Val = MDNode::get(Ctx, {ConstantAsMetadata::get(B.getInt32(Major)),
   ConstantAsMetadata::get(B.getInt32(Minor))});
-  StringRef DxilValKey = "dx.valver";
-  M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val);
+  StringRef DXILValKey = "dx.valver";
+  auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
+  DXILValMD->addOperand(Val);
 }
 } // namespace
 
 void CGHLSLRuntime::finishCodeGen() {
   auto &TargetOpts = CGM.getTarget().getTargetOpts();
-
   llvm::Module &M = CGM.getModule();
-  addDxilValVersion(TargetOpts.DxilValidatorVers

[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

2022-08-26 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment.

> I had the time for reducing it. Luckily it wasn't that bad.

This egraph doesn't contain an entry with key "index_of_element", which is 
strange.

I still feel like I need a code snippet, which generates such egraph.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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


[PATCH] D132503: [clang] Add cxx scope if needed for requires clause.

2022-08-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

I will commit this on behalf of Luke.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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


[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-26 Thread Dong-hee Na via Phabricator via cfe-commits
corona10 updated this revision to Diff 455819.
corona10 marked an inline comment as done.
corona10 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132640

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -1061,6 +1061,82 @@
   // CHECK-FIXES: priority_queue.emplace(13);
 }
 
+void test_AliasEmplacyFunctions() {
+  typedef std::list L;
+  using DQ = std::deque;
+  L l;
+  l.emplace_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.emplace_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack STACK;
+  using PQ = std::priority_queue;
+  STACK stack;
+  stack.emplace(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.emplace(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: pq.emplace(3);
+
+  typedef std::forward_list FL;
+  using DQ2 = std::deque;
+  FL fl;
+  fl.emplace_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: fl.emplace_front(3);
+
+  DQ2 dq2;
+  dq2.emplace_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: dq2.emplace_front(3);
+}
+
+void test_Alias() {
+  typedef std::list L;
+  using DQ = std::deque;
+  L l;
+  l.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack STACK;
+  using PQ = std::priority_queue;
+  STACK stack;
+  stack.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: pq.emplace(3);
+
+  typedef std::forward_list FL;
+  using DQ2 = std::deque;
+  FL fl;
+  fl.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front instead of push_front [modernize-use-emplace]
+  // CHECK-FIXES: fl.emplace_front(3);
+
+  DQ2 dq2;
+  dq2.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace]
+  // CHECK-FIXES: dq2.emplace_front(3);
+}
+
 struct Bar {
 public:
   Bar(){};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,13 +125,17 @@
   ` when warnings
   would be emitted for uninitialized members of an anonymous union despite
   there being an initializer for one of the other members.
-  
+
 - Improved `modernize-use-emplace `_ check.
 
   The check now supports detecting inefficient invocations of ``push`` and
   ``push_front`` on STL-style containers and replacing them with ``emplace``
   or ``emplace_front``.
 
+  The check now supports detecting alias cases of ``push_back`` ``push`` and
+  ``push_front`` on STL-style containers and replacing them with ``emplace_back``,
+  ``emplace`` or ``emplace_front``.
+
 - Improved `modernize-use-equals-default `_ check.
 
   The check now skips unions since in this case a default constructor with empty body
Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -132,24 +132,27 @@
   // because this requires special treatment (it could cause performance
   // regression)
   // + match for emplace calls that should be replaced with insertion
-  auto CallPushBack = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_back"))),
-  on(has

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

2022-08-26 Thread Alvin Wong via Phabricator via cfe-commits
alvinhochun updated this revision to Diff 455820.
alvinhochun added a comment.

Applied suggestions and added test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-guard_nocf.c


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // 
expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // 
expected-warning {{unknown attribute 'guard' ignored}} 
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // expected-warning {{unknown attribute 'guard' ignored}} 
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAlloc

[clang] 7aa3270 - [clang] Add cxx scope if needed for requires clause.

2022-08-26 Thread Ilya Biryukov via cfe-commits

Author: Luke Nihlen
Date: 2022-08-26T10:15:54+02:00
New Revision: 7aa3270622f495ce1209aeca83a3b216889ccab4

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

LOG: [clang] Add cxx scope if needed for requires clause.

Fixes issue #55216.

Patch by Luke Nihlen! (lu...@google.com, luken-google@)

Reviewed By: #clang-language-wg, aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Parse/ParseTemplate.cpp
clang/test/Parser/cxx-concepts-requires-clause.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6f4747ddd1199..d3b255ace6bd2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -171,6 +171,8 @@ C++20 Feature Support
   Note: The handling of deleted functions is not yet compliant, as Clang
   does not implement `DR1496 
`_
   and `DR1734 
`_.
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 
 

diff  --git a/clang/lib/Parse/ParseTemplate.cpp 
b/clang/lib/Parse/ParseTemplate.cpp
index e32ea6003b84e..55eb3cb6399b5 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@ Decl *Parser::ParseSingleDeclarationAfterTemplate(
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }

diff  --git a/clang/test/Parser/cxx-concepts-requires-clause.cpp 
b/clang/test/Parser/cxx-concepts-requires-clause.cpp
index db8f167ba9755..e4bdd28f61ec1 100644
--- a/clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ b/clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@ struct A {
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@ struct A {
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@ int x = 0;
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();



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


[PATCH] D132503: [clang] Add cxx scope if needed for requires clause.

2022-08-26 Thread Ilya Biryukov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7aa3270622f4: [clang] Add cxx scope if needed for requires 
clause. (authored by luken-google, committed by ilya-biryukov).

Changed prior to commit:
  https://reviews.llvm.org/D132503?vs=455622&id=455822#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -171,6 +171,8 @@
   Note: The handling of deleted functions is not yet compliant, as Clang
   does not implement `DR1496 
`_
   and `DR1734 
`_.
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 
 


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -171,6 +171,8 @@
   Note: The handling of deleted functions is not yet compliant, as Clang
   does not implement `DR1496 `_
   and `DR1734 `_.
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 5fef4c6 - [clang] NFC. Small tweak to release notes

2022-08-26 Thread Ilya Biryukov via cfe-commits

Author: Ilya Biryukov
Date: 2022-08-26T10:17:44+02:00
New Revision: 5fef4c6d397cbf9d6a41d42299b55b0cfddc7d12

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

LOG: [clang] NFC. Small tweak to release notes

Forgotten in the last patch.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d3b255ace6bd..6b68172a79d2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -171,7 +171,7 @@ C++20 Feature Support
   Note: The handling of deleted functions is not yet compliant, as Clang
   does not implement `DR1496 
`_
   and `DR1734 
`_.
-- Class member variables are now in scope when parsing requires clauses. Fixes
+- Class member variables are now in scope when parsing a ``requires`` clause. 
Fixes
   `GH55216 `_.
 
 



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


[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang crashes when encountering an `if consteval` statement.
This is the minimum fix not to crash.
The fix is consistent with the current behavior of if constexpr,
which does generate coverage data for the discarded branches.
This is of course not correct and a better solution is
needed for both if constexpr and if consteval.
See https://github.com/llvm/llvm-project/issues/54419.

Fixes #57377


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132723

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp

Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++2b -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
 
 int nop() { return 0; }
 
@@ -13,6 +13,22 @@
 ++j;// CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
+
+// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
+constexpr int check_consteval(int i) {
+if consteval {
+  i++;
+}
+if !consteval {
+  i++;
+}
+if consteval {
+return 42;
+} else {
+return i;
+}
+}
+
 // CHECK-LABEL: main:
 int main() {// CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
   int i = 0;
@@ -50,6 +66,10 @@
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
   i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
+  // GH-57377
+  constexpr int c_i = check_consteval(0);
+  check_consteval(i);
+
   return 0;
 }
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1372,24 +1372,29 @@
 
   void VisitIfStmt(const IfStmt *S) {
 extendRegion(S);
+
 if (S->getInit())
   Visit(S->getInit());
 
 // Extend into the condition before we propagate through it below - this is
 // needed to handle macros that generate the "if" but not the condition.
-extendRegion(S->getCond());
+if (!S->isConsteval()) {
+  extendRegion(S->getCond());
+}
 
 Counter ParentCount = getRegion().getCounter();
 Counter ThenCount = getRegionCounter(S);
 
-// Emitting a counter for the condition makes it easier to interpret the
-// counter for the body when looking at the coverage.
-propagateCounts(ParentCount, S->getCond());
+if (!S->isConsteval()) {
+  // Emitting a counter for the condition makes it easier to interpret the
+  // counter for the body when looking at the coverage.
+  propagateCounts(ParentCount, S->getCond());
 
-// The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
-if (Gap)
-  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+  // The 'then' count applies to the area immediately after the condition.
+  auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+  if (Gap)
+fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+}
 
 extendRegion(S->getThen());
 Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1403,8 @@
 if (const Stmt *Else = S->getElse()) {
   bool ThenHasTerminateStmt = HasTerminateStmt;
   HasTerminateStmt = false;
-
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  auto Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);
@@ -1416,9 +1420,11 @@
   GapRegionCounter = OutCount;
 }
 
-// Create Branch Region around condition.
-createBranchRegion(S->getCond(), ThenCount,
-   subtractCounters(ParentCount, ThenCount));
+if (!S->isConsteval()) {
+  // Create Branch Region around condition.
+  c

[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@aaron.ballman I did a minimal fix, both because It's unclear to me how 
coverage work and because I think we should try to land this is 15. 
I hope that's okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

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


[clang] 4a043c6 - PotentiallyEvaluatedContext in a ImmediateFunctionContext.

2022-08-26 Thread Utkarsh Saxena via cfe-commits

Author: Utkarsh Saxena
Date: 2022-08-26T10:30:10+02:00
New Revision: 4a043c6376468f1883bbfcdac1a160e5bc97f640

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

LOG: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

Body of `consteval` should be in an `ImmediateFunctionContext` instead of 
`ConstantEvaluated`.
PotentiallyEvaluated expressions in Immediate functions are in a 
`ImmediateFunctionContext` as well.

Fixes https://github.com/llvm/llvm-project/issues/51182
Original divergence: https://godbolt.org/z/vadGT5j6f

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6b68172a79d2c..82f4ee482c9d9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -174,6 +174,9 @@ C++20 Feature Support
 - Class member variables are now in scope when parsing a ``requires`` clause. 
Fixes
   `GH55216 `_.
 
+- Correctly set expression evaluation context as 'immediate function context' 
in
+  consteval functions.
+  This fixes `GH51182 https://github.com/llvm/llvm-project/issues/51182`
 
 
 C++2b Feature Support

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3e9a748346250..0ee21cf64aa34 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1351,6 +1351,15 @@ class Sema final {
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
  (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext) ||
+ // C++2b [expr.const]p14:
+ // An expression or conversion is in an immediate function
+ // context if it is potentially evaluated and either:
+ //   * its innermost enclosing non-block scope is a function
+ // parameter scope of an immediate function, or
+ //   * its enclosing statement is enclosed by the compound-
+ // statement of a consteval if statement.
+ (Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
   InImmediateFunctionContext);
 }
 

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 873051fd3a3f4..1a29e6bb86f39 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14800,8 +14800,12 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope 
*FnBodyScope, Decl *D,
   // Do not push if it is a lambda because one is already pushed when building
   // the lambda in ActOnStartOfLambdaDefinition().
   if (!isLambdaCallOperator(FD))
+// [expr.const]/p14.1
+// An expression or conversion is in an immediate function context if it is
+// potentially evaluated and either: its innermost enclosing non-block 
scope
+// is a function parameter scope of an immediate function.
 PushExpressionEvaluationContext(
-FD->isConsteval() ? ExpressionEvaluationContext::ConstantEvaluated
+FD->isConsteval() ? 
ExpressionEvaluationContext::ImmediateFunctionContext
   : ExprEvalContexts.back().Context);
 
   // Check for defining attributes before the check for redefinition.

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index eb1690c007885..80ac6733cbef2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19675,8 +19675,8 @@ void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const 
Expr *Base) {
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation &&
-!FD->isDependentContext())
+!isImmediateFunctionContext() && FD->isConsteval() &&
+!RebuildingImmediateInvocation && !FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);

diff  --git a/clang/test/SemaCXX/cxx2a-consteval.cpp 
b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 78011e2003417..09129a27818d3 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,34 @@ int test() {
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH51182 {
+// Nested consteval function.
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // 

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

2022-08-26 Thread Utkarsh Saxena 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 rG4a043c637646: PotentiallyEvaluatedContext in a 
ImmediateFunctionContext. (authored by usaxena95).

Changed prior to commit:
  https://reviews.llvm.org/D132659?vs=455668&id=455831#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132659

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,34 @@
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH51182 {
+// Nested consteval function.
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // An immediate function context.
+  int n = f(a);
+  return n;
+}
+static_assert(g(100) == 100);
+// --
+template 
+consteval T max(const T& a, const T& b) {
+return (a > b) ? a : b;
+}
+template 
+consteval T mid(const T& a, const T& b, const T& c) {
+T m = max(max(a, b), c);
+if (m == a)
+return max(b, c);
+if (m == b)
+return max(a, c);
+return max(a, b);
+}
+static_assert(max(1,2)==2);
+static_assert(mid(1,2,3)==2);
+} // namespace GH51182
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19675,8 +19675,8 @@
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation &&
-!FD->isDependentContext())
+!isImmediateFunctionContext() && FD->isConsteval() &&
+!RebuildingImmediateInvocation && !FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14800,8 +14800,12 @@
   // Do not push if it is a lambda because one is already pushed when building
   // the lambda in ActOnStartOfLambdaDefinition().
   if (!isLambdaCallOperator(FD))
+// [expr.const]/p14.1
+// An expression or conversion is in an immediate function context if it is
+// potentially evaluated and either: its innermost enclosing non-block scope
+// is a function parameter scope of an immediate function.
 PushExpressionEvaluationContext(
-FD->isConsteval() ? ExpressionEvaluationContext::ConstantEvaluated
+FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
   : ExprEvalContexts.back().Context);
 
   // Check for defining attributes before the check for redefinition.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1351,6 +1351,15 @@
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext ||
  (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext) ||
+ // C++2b [expr.const]p14:
+ // An expression or conversion is in an immediate function
+ // context if it is potentially evaluated and either:
+ //   * its innermost enclosing non-block scope is a function
+ // parameter scope of an immediate function, or
+ //   * its enclosing statement is enclosed by the compound-
+ // statement of a consteval if statement.
+ (Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
   InImmediateFunctionContext);
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -174,6 +174,9 @@
 - Class member variables are now in scope when parsing a ``requires`` clause. Fixes
   `GH55216 `_.
 
+- Correctly set expression evaluation context as 'immediate function context' in
+  consteval functions.
+  This fixes `GH51182 https://github.com/llvm/llvm-project/issues/51182`
 
 
 C++2b Feature Support
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D132719: [clang-format] Rework removeBraces() in Format.cpp

2022-08-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132719

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/Format.cpp:649
 IO.mapOptional("AlignOperands", Style.AlignOperands);
-IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 IO.mapOptional("AllowAllArgumentsOnNextLine",

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > you can't remove an option, otherwise you'll break everyones .clang-format
> That's not correct. We have done it:
> D108752 -> D108882 -> D127390
> 
> You can remove (and in this case should), but you still need to parse it and 
> act accordingly. Which is done as far as I can see.
sorry thats what I meant, you can remove it, but you have to make it turn on 
the correct new style that keeps exactly the old user case, and you can't 
remove it from the configuration parsing otherwise anyone who has it in their 
.clang-format is immediately broken with an unknown option.

to be honest this is an annoyance for introducing new features, which at some 
point I'd like to drop, you can't have a new option which is not read

For me, when we introduced new languages, or new features like InsertBraces 
etc.. I want to put it in my .clang-format even though other people they aren't 
using a version that uses it. (becuase it won't impact them), i.e. it won't add 
braces or correct QualifierOrder that doesn't bother me



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

why do we need Enabled?

isn't it just

```
false:
AcrossEmptyLines: false
AcrossComments: false

true:
AcrossEmptyLines: true
AcrossComments: true
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

2022-08-26 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D127973#3751119 , @isuckatcs wrote:

>> I had the time for reducing it. Luckily it wasn't that bad.
>
> This egraph doesn't contain an entry with key "index_of_element", which is 
> strange.
>
> I still feel like I need a code snippet, which generates such egraph.

Oh, now I see why. The problem is probably because the version of clang I'm 
using is not the same as the rewrite script. For the latter I believe I'm using 
the latest one, hence it hes this change. Unlike with the clang I'm using, 
which doesn't emit this metadata.
I'm wondering if we should have a window for backward compatibility; There are 
a couple of places already where we check for the presence of some metadata and 
display it only if it exists.
Do you think it would be useful for this change as well?
Anyway, my bad for not recognizing the situation sooner. So, I'll just use the 
matching version of the rewriter. Problem solved for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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


[PATCH] D131683: Diagnosing the Future Keywords

2022-08-26 Thread Muhammad Usman Shahid via Phabricator via cfe-commits
Codesbyusman updated this revision to Diff 455841.
Codesbyusman added a comment.

updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Lexer/keywords_test.c
  clang/test/Lexer/keywords_test.cpp
  clang/test/Parser/static_assert.c

Index: clang/test/Parser/static_assert.c
===
--- clang/test/Parser/static_assert.c
+++ clang/test/Parser/static_assert.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -Wc2x-compat -verify=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -fms-compatibility -verify=c17-ms %s
 // RUN: %clang_cc1 -fsyntax-only -std=c2x -Wpre-c2x-compat -verify=c2x-compat %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
@@ -15,11 +15,13 @@
 // Only test the C++ spelling in C mode in some of the tests, to reduce the
 // amount of diagnostics to have to check. This spelling is allowed in MS-
 // compatibility mode in C, but otherwise produces errors.
-static_assert(1, ""); // c2x-error {{expected parameter declarator}} \
-  // c2x-error {{expected ')'}} \
-  // c2x-note {{to match this '('}} \
-  // c2x-error {{a type specifier is required for all declarations}} \
-  // c2x-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}}
+static_assert(1, ""); // c17-warning {{'static_assert' is a keyword in C2x}} \
+  // c17-error {{expected parameter declarator}} \
+  // c17-error {{expected ')'}} \
+  // c17-note {{to match this '('}} \
+  // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+  // c17-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}} 
+ 
 #endif
 
 // We support _Static_assert as an extension in older C modes and in all C++
@@ -42,4 +44,7 @@
// cxx17-compat-warning {{'static_assert' with no message is incompatible with C++ standards before C++17}} \
// c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
// cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
-   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
+   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // c17-warning {{'_Static_assert' with no message is a C2x extension}} \
+   // c17-ms-warning {{'_Static_assert' with no message is a C2x extension}} 
+
Index: clang/test/Lexer/keywords_test.cpp
===
--- clang/test/Lexer/keywords_test.cpp
+++ clang/test/Lexer/keywords_test.cpp
@@ -15,6 +15,8 @@
 // RUN: %clang -std=c++03 -target i686-windows-msvc -DMS -fno-declspec -fsyntax-only %s
 // RUN: %clang -std=c++03 -target x86_64-scei-ps4 -fno-declspec -fsyntax-only %s
 
+// RUN: %clang_cc1 -std=c++98 -DFutureKeyword -fsyntax-only -Wc++11-compat -Wc++20-compat -verify=cxx98 %s
+
 #define IS_KEYWORD(NAME) _Static_assert(!__is_identifier(NAME), #NAME)
 #define NOT_KEYWORD(NAME) _Static_assert(__is_identifier(NAME), #NAME)
 #define IS_TYPE(NAME) void is_##NAME##_type() { int f(NAME); }
@@ -50,17 +52,24 @@
 CXX11_TYPE(char32_t);
 CXX11_KEYWORD(constexpr);
 CXX11_KEYWORD(noexcept);
+
 #ifndef MS
 CXX11_KEYWORD(static_assert);
 #else
 // MS compiler recognizes static_assert in all modes. So should we.
 IS_KEYWORD(static_assert);
 #endif
+
 CXX11_KEYWORD(thread_local);
 
 // Concepts keywords
 CXX20_KEYWORD(concept);
 CXX20_KEYWORD(requires);
+CXX20_KEYWORD(consteval);
+CXX20_KEYWORD(constinit);
+CXX20_KEYWORD(co_await);
+CXX20_KEYWORD(co_return);
+CXX20_KEYWORD(co_yield);
 
 // __declspec extension
 DECLSPEC_KEYWORD(__declspec);
@@ -70,3 +79,26 @@
 IS_TYPE(__char16_t);
 IS_KEYWORD(__char32_t);
 IS_TYPE(__char32_t);
+
+#ifdef FutureKeyword
+
+int nullptr; // cxx98-warning {{'nullptr' is a keyword in C++11}}
+int decltype;  // cxx98-warning {{'decltype' is a keyword in C++11}}
+int alignof;  // cxx98-warning {{'alignof' is a keyword in C++11}}
+int alignas;  // cxx98-warning {{'alignas' is a keyw

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It took me a pretty long time to get the patch this clean (and working), but 
here it is.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+// TODO: There is currently a problem with how we handle
+// Pointer::toAPValue() for arrays. The following static_assert
+// will run into an assertion because we don't set the
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, but pops the pointer as well.
 template ::T>
 bool InitElemPop(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
==

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:553
+if (!visitArrayInitializer(Init))
+  return false;
+if (!this->emitPopPtr(Init))

I've been wondering about a macro to simplify code like this, e.g. `define 
DOIT(x) if(!x) return false;` that I could instead define to `assert(x)` during 
development.



Comment at: clang/test/AST/Interp/arrays.cpp:56
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+

This is quite unfortunate, but it only fails when evaluating `data[0]` by 
itself, when printing the extra info for the failed `static_assert`. I roughly 
know what the problem is though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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


[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:43
 protected:
-  // Emitters for opcodes of various arities.
-  using NullaryFn = bool (ByteCodeExprGen::*)(const SourceInfo &);
-  using UnaryFn = bool (ByteCodeExprGen::*)(PrimType, const SourceInfo &);
-  using BinaryFn = bool (ByteCodeExprGen::*)(PrimType, PrimType,
- const SourceInfo &);
-
-  // Aliases for types defined in the emitter.
-  using LabelTy = typename Emitter::LabelTy;
-  using AddrTy = typename Emitter::AddrTy;
-
-  // Reference to a function generating the pointer of an initialized object.s
+  // Reference to a function generating the pointer of an initialized object.
   using InitFnRef = std::function;

Yes, a few of these cleanups, renames and added doc comments aren't necessary 
for this patch. I will remove them if you want to, I just don't want to rebase 
against `main` all the time to push NFC patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Pavel Samolysov via Phabricator via cfe-commits
psamolysov added a comment.

@mstorsjo Thank you very much for the information. Unfortunately, our tests 
didn't catch this problem.

I've reproduced this on Windows even w/o mingw. Some time is required for 
triaging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

We also saw this assert on our Windows build, and it also can be reproduced in 
Linux:

  $ cat test2.c
  static char *getenv(char *) {}
  void foo() { getenv(""); }
  $ ~/src/upstream/879f5118fc74657e4a5c4eff6810098e1eed75ac-linux/bin/clang -c 
-O3 test2.c  
  test2.c:1:27: warning: omitting the parameter name in a function definition 
is a C2x extension [-Wc2x-extensions]   


  static char *getenv(char *) {}


  
^   


  
  test2.c:1:30: warning: non-void function does not return a value 
[-Wreturn-type] 

   
  static char *getenv(char *) {}


  
   ^

  clang: 
/home/dyung/src/upstream/llvm_clean_git/llvm/lib/Analysis/CGSCCPassManager.cpp:958:
 updateCGAndAnalysisManagerForPass(llvm::LazyCallGraph&, 
llvm::LazyCallGraph::SCC&, llvm::LazyCallGraph::Node&, 
llvm::CGSCCAnalysisManager&, llvm::CGSCCUpdateResult&, 
llvm::FunctionAnalysisManager&, bool)::: Assertion 
`RefereeN && "Visited function should already have an associated node"' failed.

The key seems to be the special function getenv(). If I rename the function, 
the crash does not occur.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 455849.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+// TODO: There is currently a problem with how we handle
+// Pointer::toAPValue() for arrays. The following static_assert
+// will run into an assertion because we don't set the
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, but pops the pointer as well.
 template ::T>
 bool InitElemPop(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

In D128830#3751368 , @dyung wrote:

> We also saw this assert on our Windows build, and it also can be reproduced 
> in Linux:
>
>   $ cat test2.c
>   static char *getenv(char *) {}
>   void foo() { getenv(""); }
>   $ ~/src/upstream/879f5118fc74657e4a5c4eff6810098e1eed75ac-linux/bin/clang 
> -c -O3 test2.c  
>   test2.c:1:27: warning: omitting the parameter name in a function definition 
> is a C2x extension [-Wc2x-extensions] 
>   
> 
>   static char *getenv(char *) {}  
>   
>   
> 
> ^ 
>   
>   
> 
>   test2.c:1:30: warning: non-void function does not return a value 
> [-Wreturn-type]   
>   
>
>   static char *getenv(char *) {}  
>   
>   
> 
>^  
>   
>   clang: 
> /home/dyung/src/upstream/llvm_clean_git/llvm/lib/Analysis/CGSCCPassManager.cpp:958:
>  updateCGAndAnalysisManagerForPass(llvm::LazyCallGraph&, 
> llvm::LazyCallGraph::SCC&, llvm::LazyCallGraph::Node&, 
> llvm::CGSCCAnalysisManager&, llvm::CGSCCUpdateResult&, 
> llvm::FunctionAnalysisManager&, bool)::: Assertion 
> `RefereeN && "Visited function should already have an associated node"' 
> failed.
>
> The key seems to be the special function getenv(). If I rename the function, 
> the crash does not occur.

Note that our internal builder found this while trying to build 
compiler-rt/lib/profile/InstrProfilingFile.c using the newly built compiler. If 
you think this might take a while to debug, could you please revert the change 
while you investigate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[clang] f964417 - Revert "[Pipelines] Introduce DAE after ArgumentPromotion"

2022-08-26 Thread Pavel Samolysov via cfe-commits

Author: Pavel Samolysov
Date: 2022-08-26T13:43:09+03:00
New Revision: f964417c32d05a88c80db315e97ada639d97eda1

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

LOG: Revert "[Pipelines] Introduce DAE after ArgumentPromotion"

The commit breaks the compiler when a function is used as a function
parameter (hm... for a function from the standard C library?):

```
static float strtof(char *, char *) {}
void a() { strtof(a, 0); }
```

This reverts commit 879f5118fc74657e4a5c4eff6810098e1eed75ac.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
llvm/test/Transforms/InstCombine/unused-nonnull.ll
llvm/test/Transforms/PhaseOrdering/dce-after-argument-promotion.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 64ac49420cbdc..463fc522c6a28 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -34,6 +34,7 @@
 ; CHECK-O: Running pass: CalledValuePropagationPass
 ; CHECK-O: Running pass: GlobalOptPass
 ; CHECK-O: Running pass: PromotePass
+; CHECK-O: Running pass: DeadArgumentEliminationPass
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: InlinerPass on (main)
@@ -73,7 +74,6 @@
 ; CHECK-O: Running pass: LCSSAPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: InstCombinePass on main
-; CHECK-O: Running pass: DeadArgumentEliminationPass
 ; CHECK-O: Running pass: GlobalOptPass
 ; CHECK-O: Running pass: GlobalDCEPass
 ; CHECK-O: Running pass: EliminateAvailableExternallyPass

diff  --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 2587c8ce900b9..bd07638b37617 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -639,7 +639,7 @@ void PassBuilder::addPGOInstrPasses(ModulePassManager &MPM,
 
 FunctionPassManager FPM;
 FPM.addPass(SROAPass());
-FPM.addPass(EarlyCSEPass()); // Catch trivial redundancies.
+FPM.addPass(EarlyCSEPass());// Catch trivial redundancies.
 FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(
 true)));// Merge & remove basic blocks.
 FPM.addPass(InstCombinePass()); // Combine silly sequences.
@@ -734,9 +734,10 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
   if (PGOOpt)
 IP.EnableDeferral = EnablePGOInlineDeferral;
 
-  ModuleInlinerWrapperPass MIWP(IP, PerformMandatoryInliningsFirst,
-InlineContext{Phase, InlinePass::CGSCCInliner},
-UseInlineAdvisor, MaxDevirtIterations);
+  ModuleInlinerWrapperPass MIWP(
+  IP, PerformMandatoryInliningsFirst,
+  InlineContext{Phase, InlinePass::CGSCCInliner},
+  UseInlineAdvisor, MaxDevirtIterations);
 
   // Require the GlobalsAA analysis for the module so we can query it within
   // the CGSCC pipeline.
@@ -960,6 +961,10 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // constants.
   MPM.addPass(createModuleToFunctionPassAdaptor(PromotePass()));
 
+  // Remove any dead arguments exposed by cleanups and constant folding
+  // globals.
+  MPM.addPass(DeadArgumentEliminationPass());
+
   // Create a small function pass pipeline to cleanup after all the global
   // optimizations.
   FunctionPassManager GlobalCleanupPM;
@@ -994,10 +999,6 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   else
 MPM.addPass(buildInlinerPipeline(Level, Phase));
 
-  // Remove any dead arguments exposed by cleanups, constant folding globals,
-  // and argument promotion.
-  MPM.addPass(DeadArgumentEliminationPass());
-
   MPM.addPass(CoroCleanupPass());
 
   if (EnableMemProfiler && Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
@@ -1595,6 +1596,9 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel 
Level,
   // keep one copy of each constant.
   MPM.addPass(ConstantMergePass());
 
+  // Remove unused arguments from functions.
+  MPM.addPass(DeadArgumentEliminationPass());
+
   // Reduce the code after globalopt and ipsccp.  Both can open up significant
   // simplification opportunities, and both can propagate functions

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Pavel Samolysov via Phabricator via cfe-commits
psamolysov added a comment.

No problem, I've reverted the commit while I need some time to build `clang` 
with the reverted commit even for make it clear the commit is guilty.

I'm sorry. It's very interesting, in @mstorsjo case, a function from the 
standard `C` library is used: `strtof`. When I renamed it in the test into 
`strtof2`, the problem has becomes not reproducible.

@Michael137 It looks like some DWARF generation/debugger test can fail after 
the revertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Michael Buch via Phabricator via cfe-commits
Michael137 added a comment.

Noted, thanks for the heads up

AFAIK, the only test that'd now fail on the debug-info side is: 
https://reviews.llvm.org/D132664


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

2022-08-26 Thread joanahalili via Phabricator via cfe-commits
joanahalili added a comment.

We have some compilation failures on our end because of function template 
parameter deduction when passing the function template to a function pointer.

  typedef void (*f)(const int&);
  
  template 
  void F(T value) {}
  
  template 
  void F(const T& value){}
  
  void q(f);
  
  void w() {
  q(&F);
  }

Is this an intended outcome for this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745

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


[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

2022-08-26 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

In D128113#3747946 , @alexfh wrote:

> For now we've added a workaround for the most problematic use case and got us 
> unblocked. Thus there is no need now in introducing additional flags. 
> However, I'm rather concerned about the cumulative additional overhead our 
> build infrastructure gets as a result of this patch. Measuring this would 
> unfortunately be a project of its own, which I'm not ready to commit to. If 
> reverting this and discussing with Richard Smith is an option for you, I'd 
> suggest doing this now and if you end up not finding a good alternative, we 
> would probably need to find a way to estimate the impact of this patch. WDYT?

Thanks, and sorry for the long time to respond to this.

I have been thinking about this problem, and I have an idea for a solution 
which is better on the performance side, but more complex change.

We would need to implement some pattern matching during AST traversal, and keep 
track of the pack index there. When extracting a component of the larger type, 
such as when deducing non-pack arguments from a pack, we would wrap it over 
with a new sugar type node which encodes the current pack indexes.

I reflected a bit on this, and yeah I think we should try to design something 
which does not potentially hinder large packs so much.

I am not sure how this is going to play out in practice, and it's possible we 
may need to come back to this solution again, but I think it's prudent to 
revert it for now if we are not sure, as this is an API change, however obscure 
for most users.

As an aside, it would be really helpful if you could track how the compilation 
of your codebase performs as clang evolves, regardless if we keep this change 
or not :)
Maybe you can reuse something from http://llvm-compile-time-tracker.com/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113

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


[PATCH] D132487: [pseudo] Placeholder disambiguation strategy: always choose second

2022-08-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:231
+
+if (Disambiguate && PrintForest) {
+  ForestNode *DisambigRoot = &Root;

hokein wrote:
> nit: we're going to print the forest *and* the disambiguated tree, is that 
> intended? IMO, if we specify the `disambiguate` flag, I'd not print the 
> forest as well.
I guess you're right. My use case was debugging disambiguation with 
`-print-forest -disambiguate -debug=only=Disambiguate.cpp` - I think we need 
the before + after + log.

However we can log the "before" with `LLVM_DEBUG()` inside disambiguate() 
instead.



Comment at: clang-tools-extra/pseudo/unittests/DisambiguateTest.cpp:28
+protected:
+  // Greatly simplified C++ grammar.
+  enum Symbol : SymbolID {

hokein wrote:
> nit: it seems like we write a lot of boilerplate code just for creating a 
> Forest for a single `a*b;` case below, it is fine currently, but if we're 
> going to add some more unittests , we might want an automate way to do that.
Hmm, maybe. It's not obvious to me that you can do much better (other 
approaches have tradeoffs, e.g. I think having all the nodes/rules/symbols as 
named variables is clearer than what we've done in some other places, and I 
think depending on the C++ grammar would confuse things).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132487

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


[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Good catch! LGTM, but please add a release note for the fix.


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

https://reviews.llvm.org/D132712

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


[clang] 4a56470 - Revert "Clang: fix AST representation of expanded template arguments."

2022-08-26 Thread Matheus Izvekov via cfe-commits

Author: Matheus Izvekov
Date: 2022-08-26T13:09:55+02:00
New Revision: 4a56470d0dcd2b6183e7aaf929995b01d986c216

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

LOG: Revert "Clang: fix AST representation of expanded template arguments."

This reverts commit 1d1a56929b725f9a79d98877f12d0a14f8418b38.

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/JSONNodeDumper.h
clang/include/clang/AST/TextNodeDumper.h
clang/include/clang/AST/Type.h
clang/include/clang/AST/TypeProperties.td
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
clang/test/AST/ast-dump-template-decls.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 2d533b49114b6..eb52bf5c736a0 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1614,8 +1614,7 @@ class ASTContext : public RefCountedBase {
QualType Wrapped);
 
   QualType getSubstTemplateTypeParmType(const TemplateTypeParmType *Replaced,
-QualType Replacement,
-Optional PackIndex) const;
+QualType Replacement) const;
   QualType getSubstTemplateTypeParmPackType(
   const TemplateTypeParmType *Replaced,
 const TemplateArgument &ArgPack);

diff  --git a/clang/include/clang/AST/JSONNodeDumper.h 
b/clang/include/clang/AST/JSONNodeDumper.h
index 3597903695797..a5575d7fd441e 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -220,7 +220,6 @@ class JSONNodeDumper
   void VisitUnaryTransformType(const UnaryTransformType *UTT);
   void VisitTagType(const TagType *TT);
   void VisitTemplateTypeParmType(const TemplateTypeParmType *TTPT);
-  void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *STTPT);
   void VisitAutoType(const AutoType *AT);
   void VisitTemplateSpecializationType(const TemplateSpecializationType *TST);
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT);

diff  --git a/clang/include/clang/AST/TextNodeDumper.h 
b/clang/include/clang/AST/TextNodeDumper.h
index e6853b12ae7e5..2e4bcdd27a8ab 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -317,7 +317,6 @@ class TextNodeDumper
   void VisitUnaryTransformType(const UnaryTransformType *T);
   void VisitTagType(const TagType *T);
   void VisitTemplateTypeParmType(const TemplateTypeParmType *T);
-  void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T);
   void VisitAutoType(const AutoType *T);
   void VisitDeducedTemplateSpecializationType(
   const DeducedTemplateSpecializationType *T);

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index f46224ff3d703..88e2fb338a328 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1793,18 +1793,6 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 unsigned NumArgs;
   };
 
-  class SubstTemplateTypeParmTypeBitfields {
-friend class SubstTemplateTypeParmType;
-
-unsigned : NumTypeBits;
-
-/// Represents the index within a pack if this represents a substitution
-/// from a pack expansion.
-/// Positive non-zero number represents the index + 1.
-/// Zero means this is not substituted from an expansion.
-unsigned PackIndex;
-  };
-
   class SubstTemplateTypeParmPackTypeBitfields {
 friend class SubstTemplateTypeParmPackType;
 
@@ -1887,7 +1875,6 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 ElaboratedTypeBitfields ElaboratedTypeBits;
 VectorTypeBitfields VectorTypeBits;
 SubstTemplateTypeParmPackTypeBitfields SubstTemplateTypeParmPackTypeBits;
-SubstTemplateTypeParmTypeBitfields SubstTemplateTypeParmTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
 DependentTemplateSpecializationTypeBitfields
   DependentTemplateSpecializationTypeBits;
@@ -4991,12 +4978,9 @@ class SubstTemplateTypeParmType : public Type, public 
llvm::FoldingSetNode {
   // The original type parameter.
   const TemplateTypeParmType *Replaced;
 
-  SubstTemplateTypeParmType(const TemplateTypeParmType *Param, QualType Canon,
-Optional PackIndex)
+  SubstTemplateTypeParmT

[PATCH] D132470: [pseudo] add the spurious left-and-upwads recovery case to unittest.

2022-08-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/unittests/GLRTest.cpp:711
 
+TEST_F(GLRTest, SpuriousRecoveryLeftAndUpwardsCase) {
+  build(R"bnf(

it's not clear to me what this is testing, can you add some documentation?

(also I think "left" is spurious and "up" is correct, so why LeftAndUpwards 
rather than just left?)



Comment at: clang-tools-extra/pseudo/unittests/GLRTest.cpp:757
+"[  2,   3) │ ├─{ := tok[2]\n"
+"[  3,   8) │ ├─stmt-seq := \n"
+"[  8,   9) │ └─} := tok[8]\n"

this looks correct to me, what's the problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132470

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


[clang-tools-extra] 56c54cf - [pseudo] Placeholder disambiguation strategy: always choose second

2022-08-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-08-26T13:16:09+02:00
New Revision: 56c54cf66bcd6c2266cd96ab4da45c766fbad540

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

LOG: [pseudo] Placeholder disambiguation strategy: always choose second

Mostly mechanics here. Interesting decisions:
 - apply disambiguation in-place instead of copying the forest
   debatable, but even the final tree size is significant
 - split decide/apply into different functions - this allows the hard part
   (decide) to be tested non-destructively and combined with HTML forest easily
 - add non-const accessors to forest to enable apply
 - unit tests but no lit tests: my plan is to test actual C++ disambiguation
   heuristics with lit, generic disambiguation mechanics without the C++ grammar

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

Added: 
clang-tools-extra/pseudo/include/clang-pseudo/Disambiguate.h
clang-tools-extra/pseudo/lib/Disambiguate.cpp
clang-tools-extra/pseudo/unittests/DisambiguateTest.cpp

Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
clang-tools-extra/pseudo/lib/CMakeLists.txt
clang-tools-extra/pseudo/lib/GLR.cpp
clang-tools-extra/pseudo/tool/ClangPseudo.cpp
clang-tools-extra/pseudo/tool/HTMLForest.cpp
clang-tools-extra/pseudo/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Disambiguate.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Disambiguate.h
new file mode 100644
index 0..5f3a22c9cabb3
--- /dev/null
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Disambiguate.h
@@ -0,0 +1,64 @@
+//===--- Disambiguate.h - Find the best tree in the forest ---*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// A GLR parse forest represents every possible parse tree for the source code.
+//
+// Before we can do useful analysis/editing of the code, we need to pick a
+// single tree which we think is accurate. We use three main types of clues:
+//
+// A) Semantic language rules may restrict which parses are allowed.
+//For example, `string string string X` is *grammatical* C++, but only a
+//single type-name is allowed in a decl-specifier-sequence.
+//Where possible, these interpretations are forbidden by guards.
+//Sometimes this isn't possible, or we want our parser to be lenient.
+//
+// B) Some constructs are rarer, while others are common.
+//For example `a::c` is often a template specialization, and rarely a
+//double comparison between a, b, and c.
+//
+// C) Identifier text hints whether they name types/values/templates etc.
+//"std" is usually a namespace, a project index may also guide us.
+//Hints may be within the document: if one occurrence of 'foo' is a 
variable
+//then the others probably are too.
+//(Text need not match: similar CaseStyle can be a weak hint, too).
+//
+////
+//
+// Mechanically, we replace each ambiguous node with its best alternative.
+//
+// "Best" is determined by assigning bonuses/penalties to nodes, to express
+// the clues of type A and B above. A forest node representing an unlikely
+// parse would apply a penalty to every subtree is is present in.
+// Disambiguation proceeds bottom-up, so that the score of each alternative
+// is known when a decision is made.
+//
+// Identifier-based hints within the document mean some nodes should be
+// *correlated*. Rather than resolve these simultaneously, we make the most
+// certain decisions first and use these results to update bonuses elsewhere.
+//
+//===--===//
+
+#include "clang-pseudo/Forest.h"
+
+namespace clang::pseudo {
+
+struct DisambiguateParams {};
+
+// Maps ambiguous nodes onto the index of their preferred alternative.
+using Disambiguation = llvm::DenseMap;
+
+// Resolve each ambiguous node in the forest.
+// Maps each ambiguous node to the index of the chosen alternative.
+// FIXME: current implementation is a placeholder and chooses arbitrarily.
+Disambiguation disambiguate(const ForestNode *Root,
+const DisambiguateParams &Params);
+
+// Remove all ambiguities from the forest, resolving them according to 
Disambig.
+void removeAmbiguities(ForestNode *&Root, const Disambiguation &Disambig);
+
+} // namespace clang::pseudo

diff  --git a/clang-tools-extra/pseudo

[PATCH] D132487: [pseudo] Placeholder disambiguation strategy: always choose second

2022-08-26 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG56c54cf66bcd: [pseudo] Placeholder disambiguation strategy: 
always choose second (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D132487?vs=454887&id=455870#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132487

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Disambiguate.h
  clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Disambiguate.cpp
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp
  clang-tools-extra/pseudo/tool/HTMLForest.cpp
  clang-tools-extra/pseudo/unittests/CMakeLists.txt
  clang-tools-extra/pseudo/unittests/DisambiguateTest.cpp

Index: clang-tools-extra/pseudo/unittests/DisambiguateTest.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/unittests/DisambiguateTest.cpp
@@ -0,0 +1,111 @@
+//===--- DisambiguateTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang-pseudo/Disambiguate.h"
+#include "clang-pseudo/Forest.h"
+#include "clang-pseudo/Token.h"
+#include "clang/Basic/TokenKinds.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace pseudo {
+namespace {
+using testing::ElementsAre;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
+// Common disambiguation test fixture.
+// This is the ambiguous forest representing parses of 'a * b;'.
+class DisambiguateTest : public ::testing::Test {
+protected:
+  // Greatly simplified C++ grammar.
+  enum Symbol : SymbolID {
+Statement,
+Declarator,
+Expression,
+DeclSpecifier,
+Type,
+Template,
+  };
+  enum Rule : RuleID {
+/* LHS__RHS1_RHS2 means LHS := RHS1 RHS2 */
+Statement__DeclSpecifier_Declarator_Semi,
+Declarator__Star_Declarator,
+Declarator__Identifier,
+Statement__Expression_Semi,
+Expression__Expression_Star_Expression,
+Expression__Identifier,
+DeclSpecifier__Type,
+DeclSpecifier__Template,
+Type__Identifier,
+Template__Identifier,
+  };
+
+  ForestArena Arena;
+  ForestNode &A = Arena.createTerminal(tok::identifier, 0);
+  ForestNode &Star = Arena.createTerminal(tok::star, 1);
+  ForestNode &B = Arena.createTerminal(tok::identifier, 2);
+  ForestNode &Semi = Arena.createTerminal(tok::semi, 3);
+
+  // Parse as multiplication expression.
+  ForestNode &AExpr =
+  Arena.createSequence(Expression, Expression__Identifier, &A);
+  ForestNode &BExpr =
+  Arena.createSequence(Expression, Expression__Identifier, &B);
+  ForestNode &Expr =
+  Arena.createSequence(Expression, Expression__Expression_Star_Expression,
+   {&AExpr, &Star, &BExpr});
+  ForestNode &ExprStmt = Arena.createSequence(
+  Statement, Statement__Expression_Semi, {&Expr, &Semi});
+  // Parse as declaration (`a` may be CTAD or not).
+  ForestNode &AType =
+  Arena.createSequence(DeclSpecifier, DeclSpecifier__Type,
+   &Arena.createSequence(Type, Type__Identifier, &A));
+  ForestNode &ATemplate = Arena.createSequence(
+  DeclSpecifier, DeclSpecifier__Template,
+  &Arena.createSequence(Template, Template__Identifier, &A));
+  ForestNode &DeclSpec =
+  Arena.createAmbiguous(DeclSpecifier, {&AType, &ATemplate});
+  ForestNode &BDeclarator =
+  Arena.createSequence(Declarator, Declarator__Identifier, &B);
+  ForestNode &BPtr = Arena.createSequence(
+  Declarator, Declarator__Star_Declarator, {&Star, &BDeclarator});
+  ForestNode &DeclStmt =
+  Arena.createSequence(Statement, Statement__DeclSpecifier_Declarator_Semi,
+   {&DeclSpec, &Star, &BDeclarator});
+  // Top-level ambiguity
+  ForestNode &Stmt = Arena.createAmbiguous(Statement, {&ExprStmt, &DeclStmt});
+};
+
+TEST_F(DisambiguateTest, Remove) {
+  Disambiguation D;
+  D.try_emplace(&Stmt, 1); // statement is a declaration, not an expression
+  D.try_emplace(&DeclSpec, 0); // a is a type, not a (CTAD) template
+  ForestNode *Root = &Stmt;
+  removeAmbiguities(Root, D);
+
+  EXPECT_EQ(Root, &DeclStmt);
+  EXPECT_THAT(DeclStmt.elements(), ElementsAre(&AType, &Star, &BDeclarator));
+}
+
+TEST_F(DisambiguateTest, DummyStrategy) {
+  Disambiguation D = disambiguate(&Stmt, {});
+  EXPECT_THAT(D, UnorderedElementsAre(Pair(&Stmt, 1), Pair(&DeclSpec, 1)));
+
+  ForestNode *Root = &Stmt;
+  re

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
inclyc updated this revision to Diff 455871.
inclyc added a comment.

Add LangOpts so we keep the Obj-C behavior of clang just as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/FormatString.h
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/format-strings-freebsd.c
  clang/test/Sema/format-strings-scanf.c
  clang/test/Sema/format-strings.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -819,13 +819,7 @@
 
   Unclear type relationship between a format specifier and its argument
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf";>N2562
-  
-Partial
-  Clang supports diagnostics checking format specifier validity, but
-  does not yet account for all of the changes in this paper, especially
-  regarding length modifiers like h and hh.
-
-  
+  Clang 16
 
 
 
Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -830,3 +830,58 @@
   printf_arg2("foo", "%s string %i\n", "aaa", 123);
   printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
 }
+
+void test_promotion(void) {
+  // Default argument promotions for *printf in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  char c;
+  short ss;
+  unsigned short us;
+
+  printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+  printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+  // %ld %lld %llx
+  printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+  printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+  printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+  printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+  printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+  printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+
+  // ill formed spec for floats
+  printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+
+  // for %hhd and `short` they are compatible by promotions but more likely misuse
+  printf("%hd", ss); // no-warning
+  printf("%hhd", ss); // expected-warning{{format specifies type 'char' but the argument has type 'short'}}
+  printf("%hu", us); // no-warning
+  printf("%hhu", ss); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
+
+  // floats & integers are not compatible
+  printf("%f", i); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+  printf("%f", sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+  printf("%f", uc); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned char'}}
+  printf("%f", c); // expected-warning{{format specifies type 'double' but the argument has type 'char'}}
+  printf("%f", ss); // expected-warning{{format specifies type 'double' but the argument has type 'short'}}
+  printf("%f", us); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned short'}}
+
+  // character literals
+  // In C language engineering practice, printing a character literal with %hhd or %d is common, but %hd may be misuse.
+  printf("%hhu", 'a'); // no-warning
+  printf("%hhd", 'a'); // no-warning
+  printf("%hd", 'a'); // expected-warning{{format specifies type 'short' but the argument has type 'char'}}
+  printf("%hu", 'a'); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'char'}}
+  printf("%d", 'a'); // no-warning
+  printf("%u", 'a'); // no-warning
+
+
+  // pointers
+  printf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+}
Index: clang/test/Sema/format-strings-scanf.c
===
--- clang/test/Sem

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
inclyc added inline comments.



Comment at: clang/lib/AST/FormatString.cpp:367
+case BuiltinType::Char_U:
+case BuiltinType::Bool:
+  return Match;

aaron.ballman wrote:
> inclyc wrote:
> > aaron.ballman wrote:
> > > Isn't this a match promotion as well? e.g. `printf("%hhd", (_Bool)0);` 
> > > where the `_Bool` is promoted to `int` but then cast back down to a char 
> > > type (which seems rather unlikely to be a type confusion issue).
> > Hmm? `_Bool` just matches `AnyCharTy` perfectly. `MatchPromotion` means the 
> > types having **different** length and they can be considered as "partially 
> > matched" because of promotions). 
> > 
> > Isn't `_Bool`  and `AnyChar`  here just have the same length?
> > Hmm? _Bool just matches AnyCharTy perfectly. MatchPromotion means the types 
> > having different length and they can be considered as "partially matched" 
> > because of promotions).
> >
> > Isn't _Bool and AnyChar here just have the same length?
> 
> `_Bool` is not a character type, so it doesn't match `AnyCharTy` perfectly. 
> The size can be the same but they're still different types.
I've add `case BuiltinType::Bool:` back to preserve 
https://reviews.llvm.org/D66856


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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


[PATCH] D131388: [docs] Add "Standard C++ Modules"

2022-08-26 Thread Daniel Ruoso via Phabricator via cfe-commits
ruoso accepted this revision.
ruoso added a comment.

Thanks for taking my last minute suggestions


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

https://reviews.llvm.org/D131388

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


[PATCH] D131632: [clang] Enable output of SARIF diagnostics

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:45-47
+  void emitCodeContext(FullSourceLoc Loc, DiagnosticsEngine::Level Level,
+   SmallVectorImpl &Ranges,
+   ArrayRef Hints) override {}

abrahamcd wrote:
> aaron.ballman wrote:
> > Move this implementation to live with the rest and give it an assertion?
> This function is currently being called from the DiagnosticRenderer class 
> that both Text and SARIFDiagnostics inherit from. Maybe this could be part of 
> the refactoring, making sure that any text-specific function calls are moved 
> to TextDiagnostic rather than being in the general Renderer base class?
Ah, good call, the assert definitely would be wrong then. Let's leave it alone 
for now, it's fine as-is.



Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:53
+  OS.flush();
+}
+

cjdb wrote:
> aaron.ballman wrote:
> > Should we be invalidating that `SARIFDiag` object here so the printer 
> > cannot be used after the source file has ended?
> I suggested that we don't do this, because the next run of `BeginSourceFile` 
> can take care of that when making the new `SARIFDiagnostic`. However, if we 
> were to put an assert at the top of every member function other than 
> `BeginSourceFile` to ensure that SARIFDiag isn't null (and one at the top of 
> `BeginSourceFile` to ensure that it _is_), I think there might be good value 
> in re-adding this.
SGTM, thanks for weighing in!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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


[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-26 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:60
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.

Could you please indicate which members are mandatory and which are optional. 
Perhaps comments could start either with `/// Mandatory. ...` or `/// Optional. 
...`.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:244
+///
+/// Requirements:
+///

Should requirements include the full list of the requirements of 
`checkDataflow` below?



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:295-296
 
-// Runs dataflow on the body of the function that matches `TargetFuncMatcher` 
in
-// code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
+// FIXME: Remove this function after usage has been updated to the overload
+// which uses the `AnalysisInputs` struct.
+//

Let's clearly indicate that it's deprecated.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:339-340
 
-// Runs dataflow on the body of the function named `target_fun` in code snippet
-// `code`.
+// FIXME: Remove this function after usage has been updated to the overload
+// which uses the `AnalysisInputs` struct.
+//

Let's clearly indicate that it's deprecated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

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


[PATCH] D132695: [Clang] Avoid crashes when parsing using enum declarations

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

Thanks, this looks like it cleans up some needlessly chatty diagnostics! Just 
had a question that you can fix when landing if you agree, otherwise LGTM as 
well.




Comment at: clang/lib/Parse/ParseDecl.cpp:4651
   if (Tok.isNot(tok::l_brace)) {
+DS.SetTypeSpecError();
 // Has no name and is not a definition.

Shouldn't this be lifted out of the `if` statement into the parent one where we 
issue the error diagnostic?


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

https://reviews.llvm.org/D132695

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


[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!




Comment at: clang/include/clang/Basic/Attr.td:373-375
   // Specifies Operating Systems for which the target applies, based off the
   // OSType enumeration in Triple.h
   list OSes;

rnk wrote:
> Do we need customcode? Can we not use the OS list here?
Great catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

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


[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-26 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:88
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.

Why move this? It makes it hard to tell if there are other changes. If there 
are other changes, let's keep it where it is to have a clean diff and move it 
in a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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


[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 455873.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, bu

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:56
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+

tbaeder wrote:
> This is quite unfortunate, but it only fails when evaluating `data[0]` by 
> itself, when printing the extra info for the failed `static_assert`. I 
> roughly know what the problem is though.
Ignore this, I fixed it.


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

https://reviews.llvm.org/D132727

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


[PATCH] D132592: [Clang] Implement function attribute nouwtable

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132592#3749567 , @ychen wrote:

> Thanks for taking a look!
>
> In D132592#3749261 , @aaron.ballman 
> wrote:
>
>> Do we have any evidence that users need this level of control or will 
>> understand how to properly use the attribute? The command line option makes 
>> sense to me because it's an all-or-nothing flag, but I'm not certain I 
>> understand the need for per-function control.
>
> https://github.com/llvm/llvm-project/blob/064a08cd955019da9130f1109bfa534e79b8ec7c/llvm/include/llvm/IR/Function.h#L639-L641,
>  per-function unwind table entry depends on both nounwind and uwtable. We 
> have nothrow attribute for nounwind but not nouwtable for uwtable. With this, 
> a user could use function attributes to control unwind table entry generation 
> which could only be achieved by inline assembly or writing assembly files 
> directly. I'd consider this a companion of nothrow. So making them both 
> per-function attribute seems natural?
>
>> Also, if a user gets this wrong (applies the attribute where they 
>> shouldn't), what is the failure mode (does it introduce any exploitable 
>> behavior)?
>
> The flag may only suppress unwind table emission instead of causing more, the 
> lack of unwind table may only stop control flow rather than giving it more 
> freedom. So I think this is safe from a security perspective. Using it wrong 
> may cause unnecessary crashes just like any other memory bugs, but not a 
> malicious binary.

Thank you for the explanations, that helped. :-)

You're missing all of the semantic tests (that the attr takes no arguments, 
only applies to function-like things, etc). Also, the subject you picked is 
`FunctionLike` so you should have some test coverage showing how this works 
when calling through a function pointer with the attribute (or you should pick 
a more appropriate subject if that one is wrong).




Comment at: clang/include/clang/Basic/AttrDocs.td:4545-4546
+the unwind table entry for the specified function. This attribute is useful for
+selectively emitting the unwind table entry on some functions when building 
with
+``-funwind-tables`` compiler option.
+}];




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132592

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


[PATCH] D132732: [pseudo] Collect positions in TokenStream that trigger the error recovery.

2022-08-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

Mainly for making it easier to debug broken code. No intend to land it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132732

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp


Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -153,12 +153,26 @@
   "The start symbol {0} doesn't exit in the grammar!\n", StartSymbol);
   return 2;
 }
-auto &Root =
-glrParse(clang::pseudo::ParseParams{*ParseableStream, Arena, GSS},
- *StartSymID, Lang);
+std::vector FailingPoints;
+auto &Root = glrParse(clang::pseudo::ParseParams{*ParseableStream, Arena,
+ GSS, &FailingPoints},
+  *StartSymID, Lang);
 if (PrintForest)
   llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/ForestAbbrev);
-
+if (!FailingPoints.empty()) {
+  llvm::for_each(FailingPoints, [&](Token::Index Pos) {
+if (Pos >= ParseableStream->tokens().size())
+  return;
+const auto &T = ParseableStream->tokens()[Pos];
+std::string LineCode;
+llvm::for_each(ParseableStream->tokens(), [&](const Token &E) {
+  if (E.Line == T.Line)
+LineCode += (" " + E.text().str());
+});
+llvm::outs() << llvm::formatv("Error on parsing Line {0}: {1}\n",
+  T.Line + 1, LineCode);
+  });
+}
 if (HTMLForest.getNumOccurrences()) {
   std::error_code EC;
   llvm::raw_fd_ostream HTMLOut(HTMLForest, EC);
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -646,7 +646,8 @@
  "Re-reducing without lookahead.\n");
   Heads.resize(HeadsPartition);
   Reduce(Heads, /*allow all reductions*/ tokenSymbol(tok::unknown));
-
+  if (Params.FailingPoints)
+Params.FailingPoints->push_back(I);
   glrRecover(Heads, I, Params, Lang, NextHeads);
   assert(!NextHeads.empty() &&  "no fallback recovery strategy!" );
 } else
Index: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
@@ -122,6 +122,9 @@
   // Arena for data structure used by the GLR algorithm.
   ForestArena &Forest;  // Storage for the output forest.
   GSS &GSStack; // Storage for parsing stacks.
+
+  // If not null, store the token indices that trigger the error-recovery.
+  std::vector *FailingPoints = nullptr;
 };
 
 // Parses the given token stream as the start symbol with the GLR algorithm,


Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -153,12 +153,26 @@
   "The start symbol {0} doesn't exit in the grammar!\n", StartSymbol);
   return 2;
 }
-auto &Root =
-glrParse(clang::pseudo::ParseParams{*ParseableStream, Arena, GSS},
- *StartSymID, Lang);
+std::vector FailingPoints;
+auto &Root = glrParse(clang::pseudo::ParseParams{*ParseableStream, Arena,
+ GSS, &FailingPoints},
+  *StartSymID, Lang);
 if (PrintForest)
   llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/ForestAbbrev);
-
+if (!FailingPoints.empty()) {
+  llvm::for_each(FailingPoints, [&](Token::Index Pos) {
+if (Pos >= ParseableStream->tokens().size())
+  return;
+const auto &T = ParseableStream->tokens()[Pos];
+std::string LineCode;
+llvm::for_each(ParseableStream->tokens(), [&](const Token &E) {
+  if (E.Line == T.Line)
+LineCode += (" " + E.text().str());
+});
+llvm::outs() << llvm::formatv("Error on parsing Line {0}: {1}\n",
+  T.Line + 1, LineCode);
+  });
+}
 if (HTMLForest.getNumOccurrences()) {
   std::error_code EC;
   llvm::raw_fd_ostream HTMLOut(HTMLForest, EC);
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -646,7 +646,8

[PATCH] D132244: [docs] improve documentation for misc-const-correctness

2022-08-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

@njames93 friendly ping, for https://reviews.llvm.org/D130793 as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132244

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


[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Can you also add a release note?




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1375
 extendRegion(S);
+
 if (S->getInit())

Spurious whitespace change.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1381-1383
+if (!S->isConsteval()) {
+  extendRegion(S->getCond());
+}

Fighting our usual crusade against braces.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1407
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  auto Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)

1) Pick a name which doesn't shadow
2) Spell out the type instead of using `auto`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

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


[clang] 80e7dec - Typo fix in Release notes.

2022-08-26 Thread Utkarsh Saxena via cfe-commits

Author: Utkarsh Saxena
Date: 2022-08-26T14:36:45+02:00
New Revision: 80e7dec561705386376bbfba0f7d5c290f2c04f6

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

LOG: Typo fix in Release notes.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 82f4ee482c9d..f1be1cd1e44f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -176,7 +176,7 @@ C++20 Feature Support
 
 - Correctly set expression evaluation context as 'immediate function context' 
in
   consteval functions.
-  This fixes `GH51182 https://github.com/llvm/llvm-project/issues/51182`
+  This fixes `GH51182 `
 
 
 C++2b Feature Support



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


[PATCH] D132286: [clang][Interp] Implement function calls

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some small nits you can fix when landing.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:607-610
+  if (auto R =
+  ByteCodeStmtGen(Ctx, P).compileFunc(FuncDecl)) {
+Func = *R;
+  }





Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:626-627
+return this->emitCall(*T, Func, E);
+  else
+return this->emitCallVoid(Func, E);
+} else {




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

https://reviews.llvm.org/D132286

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


[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 455883.
cor3ntin added a comment.

Release notes + style issues (There was no shadowing though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp

Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++2b -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
 
 int nop() { return 0; }
 
@@ -13,6 +13,22 @@
 ++j;// CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
+
+// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
+constexpr int check_consteval(int i) {
+if consteval {
+  i++;
+}
+if !consteval {
+  i++;
+}
+if consteval {
+return 42;
+} else {
+return i;
+}
+}
+
 // CHECK-LABEL: main:
 int main() {// CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
   int i = 0;
@@ -50,6 +66,10 @@
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
   i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
+  // GH-57377
+  constexpr int c_i = check_consteval(0);
+  check_consteval(i);
+
   return 0;
 }
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1377,19 +1377,22 @@
 
 // Extend into the condition before we propagate through it below - this is
 // needed to handle macros that generate the "if" but not the condition.
-extendRegion(S->getCond());
+if (!S->isConsteval())
+  extendRegion(S->getCond());
 
 Counter ParentCount = getRegion().getCounter();
 Counter ThenCount = getRegionCounter(S);
 
-// Emitting a counter for the condition makes it easier to interpret the
-// counter for the body when looking at the coverage.
-propagateCounts(ParentCount, S->getCond());
+if (!S->isConsteval()) {
+  // Emitting a counter for the condition makes it easier to interpret the
+  // counter for the body when looking at the coverage.
+  propagateCounts(ParentCount, S->getCond());
 
-// The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
-if (Gap)
-  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+  // The 'then' count applies to the area immediately after the condition.
+  Optional Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+  if (Gap)
+fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+}
 
 extendRegion(S->getThen());
 Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1401,8 @@
 if (const Stmt *Else = S->getElse()) {
   bool ThenHasTerminateStmt = HasTerminateStmt;
   HasTerminateStmt = false;
-
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  Optional Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);
@@ -1416,9 +1418,11 @@
   GapRegionCounter = OutCount;
 }
 
-// Create Branch Region around condition.
-createBranchRegion(S->getCond(), ThenCount,
-   subtractCounters(ParentCount, ThenCount));
+if (!S->isConsteval()) {
+  // Create Branch Region around condition.
+  createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
+}
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -65,6 +65,9 @@
 - Fix a crash when evaluating a multi-dimensional array's array filler
   expression is element-depe

[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked an inline comment as done.
cor3ntin added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1407
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  auto Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)

aaron.ballman wrote:
> 1) Pick a name which doesn't shadow
> 2) Spell out the type instead of using `auto`
`auto Gap...` was preexisting, I changed both instances. Both variables are in 
their own non nested scopes though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

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


[PATCH] D131388: [docs] Add "Standard C++ Modules"

2022-08-26 Thread Bret Brown via Phabricator via cfe-commits
bbrown105 added a comment.

Nice writeup! I appreciate the work that went into this.




Comment at: clang/docs/StandardCPlusPlusModules.rst:27
+
+standard C++ Named modules
+==

Just matching the capitalization of the other sections and subsections.



Comment at: clang/docs/StandardCPlusPlusModules.rst:87
+``export module module_name:partition_name;``. The ``partition_name`` should be
+unique to the module.
+

This is a small tweak but it should clarify that using the same 
`partition_name` with entirely different `module_names` is fine.



Comment at: clang/docs/StandardCPlusPlusModules.rst:89
+
+A internal module partition unit is a module unit whose module declaration
+is ``module module_name:partition_name;``. The ``partition_name`` should be





Comment at: clang/docs/StandardCPlusPlusModules.rst:91
+is ``module module_name:partition_name;``. The ``partition_name`` should be
+unique to the module.
+





Comment at: clang/docs/StandardCPlusPlusModules.rst:243-244
+since ``--precompile`` option now would only run preprocessor, which is equal 
to `-E` now.
+If we still want the filename of an ``importable module unit`` ends with 
``.cpp`` instead of ``.cppm``,
+we could put ``-x c++-module`` in front of the file. For example,
+

Is there a tracking issue to revise this choice? I seem to recall that we 
settled on recommending a different file extension, `*.ixx`, for at least 
primary module interface units.

We didn't really discuss extensions for other units, but maybe we should if 
different toolchains are going to have different expectations. Of course, build 
systems can work around this sort of thing. But if the expectation of clang is 
that, practically speaking, you'll probably be using a build system, maybe the 
docs should clarify as much. We could document that parts of this document are 
intended for people configuring or implementing build systems.

FYI @Bigcheese 



Comment at: clang/docs/StandardCPlusPlusModules.rst:744
+
+How module speed up compilation
+---




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

https://reviews.llvm.org/D131388

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


[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.

Looking through `isCompleteType` I note that arrays of an elements of 
incomplete types would have also hit this crash, as would member-pointers in 
MicrosoftABI (in some weird cases?), and some ObjC types.

Anyway, other than the nit above, LGTM.




Comment at: clang/lib/Sema/SemaTemplate.cpp:401
 IsDependent = !LookupCtx && ObjectType->isDependentType();
-assert((IsDependent || !ObjectType->isIncompleteType() ||
+assert((IsDependent || !ObjectType->getAs() ||
+!ObjectType->isIncompleteType() ||

Slight preference for making the `getAs` happen after the completeness check, 
since that is in the 'order of costliness'.  

Also, not sure the assert message here makes any sense, but I don't know of 
anything better here.




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

https://reviews.llvm.org/D132712

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


[PATCH] D131683: Diagnosing the Future Keywords

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! There's a minor thing left to be fixed, but I'll handle it on commit. I 
wanted to drop the `|KEYC2X` for the Boolean keywords so it relies solely on 
`BOOLSUPPORT`, but then I realized we could just use the correct flags instead 
of using `BOOLSUPPORT` at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683

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


[clang] 41667a8 - Diagnosing the Future Keywords

2022-08-26 Thread Aaron Ballman via cfe-commits

Author: Muhammad Usman Shahid
Date: 2022-08-26T09:20:05-04:00
New Revision: 41667a8b9b624e282e7c08fadf7091223728d1c1

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

LOG: Diagnosing the Future Keywords

The patch diagnoses an identifier as a future keyword if it exists in a
future language mode, such as:

int restrict;

in C modes earlier than C99. We now give a warning to the user that
such an identifier is a future keyword. Handles keywords from C as well
as C++.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Basic/IdentifierTable.h
clang/include/clang/Basic/TokenKinds.def
clang/lib/Basic/IdentifierTable.cpp
clang/lib/Lex/Preprocessor.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/test/Lexer/keywords_test.c
clang/test/Lexer/keywords_test.cpp
clang/test/Parser/static_assert.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f1be1cd1e44f..75f7f7f293f4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -106,6 +106,13 @@ Improvements to Clang's diagnostics
   compile-time value and printed out if the assertion fails.
 - Diagnostics about uninitialized ``constexpr`` varaibles have been improved
   to mention the missing constant initializer.
+- Correctly diagnose a future keyword if it exist as a keyword in the higher
+  language version and specifies in which version it will be a keyword. This
+  supports both c and c++ language.
+
+ Non-comprehensive list of changes in this release
+ -
+
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 4412c93683ed..ba2fef2a8085 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -139,6 +139,7 @@ def MacroRedefined : DiagGroup<"macro-redefined">;
 def BuiltinMacroRedefined : DiagGroup<"builtin-macro-redefined">;
 def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">;
 def C99Compat : DiagGroup<"c99-compat">;
+def C2xCompat : DiagGroup<"c2x-compat">;
 def CXXCompat: DiagGroup<"c++-compat">;
 def ExternCCompat : DiagGroup<"extern-c-compat">;
 def KeywordCompat : DiagGroup<"keyword-compat">;

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 6032fbd18d56..fd5398fa0409 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -80,6 +80,10 @@ def warn_cxx11_keyword : Warning<"'%0' is a keyword in 
C++11">,
   InGroup, DefaultIgnore;
 def warn_cxx20_keyword : Warning<"'%0' is a keyword in C++20">,
   InGroup, DefaultIgnore;
+def warn_c99_keyword : Warning<"'%0' is a keyword in C99">,
+  InGroup, DefaultIgnore;
+def warn_c2x_keyword : Warning<"'%0' is a keyword in C2x">,
+  InGroup, DefaultIgnore;
 
 def ext_unterminated_char_or_string : ExtWarn<
   "missing terminating %select{'|'\"'}0 character">, InGroup;

diff  --git a/clang/include/clang/Basic/IdentifierTable.h 
b/clang/include/clang/Basic/IdentifierTable.h
index aaf1297c1a64..1e89aa5f670f 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -17,6 +17,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/LexDiagnostic.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -668,6 +669,12 @@ class IdentifierTable {
   /// Populate the identifier table with info about the language keywords
   /// for the language specified by \p LangOpts.
   void AddKeywords(const LangOptions &LangOpts);
+
+  /// Returns the correct diagnostic to issue for a future-compat diagnostic
+  /// warning. Note, this function assumes the identifier passed has already
+  /// been determined to be a future compatible keyword.
+  diag::kind getFutureCompatDiagKind(const IdentifierInfo &II,
+ const LangOptions &LangOpts);
 };
 
 /// A family of Objective-C methods.

diff  --git a/clang/include/clang/Basic/TokenKinds.def 
b/clang/include/clang/Basic/TokenKinds.def
index 7f421add92aa..729442a79f05 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -28,6 +28,9 @@
 #ifndef CXX20_KEYWORD
 #define CXX20_KEYWORD(X,Y) KEYWORD(X,KEYCXX20|(Y))
 #endif
+#ifndef C99_KEYWORD
+#define C99_KEYWORD(X,Y) KEYWORD(X,KEY

[PATCH] D131683: Diagnosing the Future Keywords

2022-08-26 Thread Aaron Ballman 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 rG41667a8b9b62: Diagnosing the Future Keywords (authored by 
Codesbyusman, committed by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D131683?vs=455841&id=455888#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Lexer/keywords_test.c
  clang/test/Lexer/keywords_test.cpp
  clang/test/Parser/static_assert.c

Index: clang/test/Parser/static_assert.c
===
--- clang/test/Parser/static_assert.c
+++ clang/test/Parser/static_assert.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -Wc2x-compat -verify=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -fms-compatibility -verify=c17-ms %s
 // RUN: %clang_cc1 -fsyntax-only -std=c2x -Wpre-c2x-compat -verify=c2x-compat %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
@@ -15,11 +15,13 @@
 // Only test the C++ spelling in C mode in some of the tests, to reduce the
 // amount of diagnostics to have to check. This spelling is allowed in MS-
 // compatibility mode in C, but otherwise produces errors.
-static_assert(1, ""); // c2x-error {{expected parameter declarator}} \
-  // c2x-error {{expected ')'}} \
-  // c2x-note {{to match this '('}} \
-  // c2x-error {{a type specifier is required for all declarations}} \
-  // c2x-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}}
+static_assert(1, ""); // c17-warning {{'static_assert' is a keyword in C2x}} \
+  // c17-error {{expected parameter declarator}} \
+  // c17-error {{expected ')'}} \
+  // c17-note {{to match this '('}} \
+  // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+  // c17-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}}
+
 #endif
 
 // We support _Static_assert as an extension in older C modes and in all C++
@@ -42,4 +44,6 @@
// cxx17-compat-warning {{'static_assert' with no message is incompatible with C++ standards before C++17}} \
// c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
// cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
-   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
+   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // c17-warning {{'_Static_assert' with no message is a C2x extension}} \
+   // c17-ms-warning {{'_Static_assert' with no message is a C2x extension}}
Index: clang/test/Lexer/keywords_test.cpp
===
--- clang/test/Lexer/keywords_test.cpp
+++ clang/test/Lexer/keywords_test.cpp
@@ -15,6 +15,8 @@
 // RUN: %clang -std=c++03 -target i686-windows-msvc -DMS -fno-declspec -fsyntax-only %s
 // RUN: %clang -std=c++03 -target x86_64-scei-ps4 -fno-declspec -fsyntax-only %s
 
+// RUN: %clang_cc1 -std=c++98 -DFutureKeyword -fsyntax-only -Wc++11-compat -Wc++20-compat -verify=cxx98 %s
+
 #define IS_KEYWORD(NAME) _Static_assert(!__is_identifier(NAME), #NAME)
 #define NOT_KEYWORD(NAME) _Static_assert(__is_identifier(NAME), #NAME)
 #define IS_TYPE(NAME) void is_##NAME##_type() { int f(NAME); }
@@ -50,17 +52,24 @@
 CXX11_TYPE(char32_t);
 CXX11_KEYWORD(constexpr);
 CXX11_KEYWORD(noexcept);
+
 #ifndef MS
 CXX11_KEYWORD(static_assert);
 #else
 // MS compiler recognizes static_assert in all modes. So should we.
 IS_KEYWORD(static_assert);
 #endif
+
 CXX11_KEYWORD(thread_local);
 
 // Concepts keywords
 CXX20_KEYWORD(concept);
 CXX20_KEYWORD(requires);
+CXX20_KEYWORD(consteval);
+CXX20_KEYWORD(constinit);
+CXX20_KEYWORD(co_await);
+CXX20_KEYWORD(co_return);
+CXX20_KEYWORD(co_yield);
 
 // __declspec extension
 DECLSPEC_KEYWORD(__declspec);
@@ -70,3 +79,26 @@
 IS_TYPE(__char16_t);
 IS_KEYWORD(__char32_t);
 IS_TYPE(__char32_t);
+
+#ifdef FutureKeyword
+
+int nullptr; // cxx98-warning {{'nullp

[PATCH] D132136: [clang] Perform implicit lvalue-to-rvalue cast with new interpreter

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Would be great if we had a better test here... is there anything we can do to 
validate this is happening other than checking for that one note?


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

https://reviews.llvm.org/D132136

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


[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

2022-08-26 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

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


[PATCH] D132550: Changes to code ownership in clang and clang-tidy

2022-08-26 Thread Renato Golin via Phabricator via cfe-commits
rengolin accepted this revision.
rengolin added a comment.

W00t!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132550

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


[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Nothing suspicious as far as I can tell, other than just punting on 
ArrayFillers.  Don't understand enough of this code to just do a straight-up 
approval though, so hopefully one of the other reviewers can take a look and 
confirm with another uninformed opinion :)




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:535
+
+  // TODO: Fillers?
+  if (const auto *InitList = dyn_cast(Initializer)) {

Heh, THIS is a huge "TODO" here.  The ArrayFillers go through a ton of 
twists/turns in the current interpreter, as array-filler initializers can be 
massive.  Do we have a way to avoid allocating space for 
filled-but-not-referenced values in this interpreter?


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

https://reviews.llvm.org/D132727

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, tahonermann.
Herald added a subscriber: inglorion.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Redo how we do IntegralCasts and implement IntegralToBoolean casts using the 
already existing cast op.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132739

Files:
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -14,6 +14,11 @@
  // expected-note{{evaluates to}} \
  // ref-note{{evaluates to}}
 
+constexpr bool b = number;
+static_assert(b, "");
+constexpr int one = true;
+static_assert(one == 1, "");
+
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
 constexpr void* getNull() { return nullptr; }
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -425,12 +425,16 @@
 
//===--===//
 // TODO: Expand this to handle casts between more types.
 
-def Sint32TypeClass : TypeClass {
-  let Types = [Sint32];
+def FromCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
+}
+
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }
 
 def Cast: Opcode {
-  let Types = [BoolTypeClass, Sint32TypeClass];
+  let Types = [FromCastTypeClass, ToCastTypeClass];
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -117,6 +117,7 @@
   case CK_NullToPointer:
 return this->Visit(SubExpr);
 
+  case CK_IntegralToBoolean:
   case CK_IntegralCast: {
 Optional FromT = classify(SubExpr->getType());
 Optional ToT = classify(CE->getType());
@@ -132,19 +133,6 @@
   case CK_ToVoid:
 return discard(SubExpr);
 
-  case CK_IntegralToBoolean:
-// Compare integral from Subexpr with 0
-if (Optional T = classify(SubExpr->getType())) {
-  if (!this->Visit(SubExpr))
-return false;
-
-  if (!this->emitConst(SubExpr, 0))
-return false;
-
-  return this->emitNE(*T, SubExpr);
-}
-return false;
-
   default:
 assert(false && "Cast not implemented");
   }
Index: clang/lib/AST/Interp/Boolean.h
===
--- clang/lib/AST/Interp/Boolean.h
+++ clang/lib/AST/Interp/Boolean.h
@@ -50,6 +50,7 @@
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
   explicit operator int() const { return V; }
+  explicit operator bool() const { return V; }
 
   APSInt toAPSInt() const {
 return APSInt(APInt(1, static_cast(V), false), true);
@@ -85,9 +86,10 @@
   static Boolean min(unsigned NumBits) { return Boolean(false); }
   static Boolean max(unsigned NumBits) { return Boolean(true); }
 
-  template 
-  static std::enable_if_t::value, Boolean> from(T Value) {
-return Boolean(Value != 0);
+  template  static Boolean from(T Value) {
+if constexpr (std::is_integral::value)
+  return Boolean(Value != 0);
+return Boolean(static_cast(Value) != 0);
   }
 
   template 


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -14,6 +14,11 @@
  // expected-note{{evaluates to}} \
  // ref-note{{evaluates to}}
 
+constexpr bool b = number;
+static_assert(b, "");
+constexpr int one = true;
+static_assert(one == 1, "");
+
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
 constexpr void* getNull() { return nullptr; }
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -425,12 +425,16 @@
 //===--===//
 // TODO: Expand this to handle casts between more types.
 
-def Sint32TypeClass : TypeClass {
-  let Types = [Sint32];
+def FromCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
+}
+
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }
 
 def Cast: Opcode {
-  let Types = [BoolTypeClass, Sint32TypeClass];
+  let Types = [FromCastTypeClass, ToCastTypeClass];
   let Has

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:535
+
+  // TODO: Fillers?
+  if (const auto *InitList = dyn_cast(Initializer)) {

erichkeane wrote:
> Heh, THIS is a huge "TODO" here.  The ArrayFillers go through a ton of 
> twists/turns in the current interpreter, as array-filler initializers can be 
> massive.  Do we have a way to avoid allocating space for 
> filled-but-not-referenced values in this interpreter?
Heh, believe me, I  know ;) I'd have to look into that, but I'm pretty sure the 
code as it is right now doesn't handle them at all.


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

https://reviews.llvm.org/D132727

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/AST/Interp/Opcodes.td:433
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }

Not sure how this works... but is this ONLY int-32 to bool and vice versa?  The 
implementation of any of the other integrals should be trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132739

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:433
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }

erichkeane wrote:
> Not sure how this works... but is this ONLY int-32 to bool and vice versa?  
> The implementation of any of the other integrals should be trivial.
Yes exactly, the primitive types are:
```
def Bool : Type;
def Sint8 : Type;
def Uint8 : Type;
def Sint16 : Type;
def Uint16 : Type;
def Sint32 : Type;
def Uint32 : Type;
def Sint64 : Type;
def Uint64 : Type;
def Ptr : Type;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132739

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:433
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }

tbaeder wrote:
> erichkeane wrote:
> > Not sure how this works... but is this ONLY int-32 to bool and vice versa?  
> > The implementation of any of the other integrals should be trivial.
> Yes exactly, the primitive types are:
> ```
> def Bool : Type;
> def Sint8 : Type;
> def Uint8 : Type;
> def Sint16 : Type;
> def Uint16 : Type;
> def Sint32 : Type;
> def Uint32 : Type;
> def Sint64 : Type;
> def Uint64 : Type;
> def Ptr : Type;
> ```
So why not fill them in here as well? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132739

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:433
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }

erichkeane wrote:
> tbaeder wrote:
> > erichkeane wrote:
> > > Not sure how this works... but is this ONLY int-32 to bool and vice 
> > > versa?  The implementation of any of the other integrals should be 
> > > trivial.
> > Yes exactly, the primitive types are:
> > ```
> > def Bool : Type;
> > def Sint8 : Type;
> > def Uint8 : Type;
> > def Sint16 : Type;
> > def Uint16 : Type;
> > def Sint32 : Type;
> > def Uint32 : Type;
> > def Sint64 : Type;
> > def Uint64 : Type;
> > def Ptr : Type;
> > ```
> So why not fill them in here as well? 
I think I can add `Uint32` as well, but for the others I can't write tests yet 
since I don't have their literals implemented yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132739

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


[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 455897.

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

https://reviews.llvm.org/D132739

Files:
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -14,6 +14,37 @@
  // expected-note{{evaluates to}} \
  // ref-note{{evaluates to}}
 
+constexpr bool b = number;
+static_assert(b, "");
+constexpr int one = true;
+static_assert(one == 1, "");
+
+namespace IntegralCasts {
+  constexpr int i = 12;
+  constexpr unsigned int ui = i;
+  static_assert(ui == 12, "");
+  constexpr unsigned int ub = !false;
+  static_assert(ub == 1, "");
+
+  constexpr int si = ui;
+  static_assert(si == 12, "");
+  constexpr int sb = true;
+  static_assert(sb == 1, "");
+
+  constexpr int zero = 0;
+  constexpr unsigned int uzero = 0;
+  constexpr bool bs = i;
+  static_assert(bs, "");
+  constexpr bool bu = ui;
+  static_assert(bu, "");
+  constexpr bool ns = zero;
+  static_assert(!ns, "");
+  constexpr bool nu = uzero;
+  static_assert(!nu, "");
+};
+
+
+
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
 constexpr void* getNull() { return nullptr; }
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -425,12 +425,16 @@
 //===--===//
 // TODO: Expand this to handle casts between more types.
 
-def Sint32TypeClass : TypeClass {
-  let Types = [Sint32];
+def FromCastTypeClass : TypeClass {
+  let Types = [Uint32, Sint32, Bool];
+}
+
+def ToCastTypeClass : TypeClass {
+  let Types = [Uint32, Sint32, Bool];
 }
 
 def Cast: Opcode {
-  let Types = [BoolTypeClass, Sint32TypeClass];
+  let Types = [FromCastTypeClass, ToCastTypeClass];
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -117,6 +117,7 @@
   case CK_NullToPointer:
 return this->Visit(SubExpr);
 
+  case CK_IntegralToBoolean:
   case CK_IntegralCast: {
 Optional FromT = classify(SubExpr->getType());
 Optional ToT = classify(CE->getType());
@@ -132,19 +133,6 @@
   case CK_ToVoid:
 return discard(SubExpr);
 
-  case CK_IntegralToBoolean:
-// Compare integral from Subexpr with 0
-if (Optional T = classify(SubExpr->getType())) {
-  if (!this->Visit(SubExpr))
-return false;
-
-  if (!this->emitConst(SubExpr, 0))
-return false;
-
-  return this->emitNE(*T, SubExpr);
-}
-return false;
-
   default:
 assert(false && "Cast not implemented");
   }
Index: clang/lib/AST/Interp/Boolean.h
===
--- clang/lib/AST/Interp/Boolean.h
+++ clang/lib/AST/Interp/Boolean.h
@@ -50,6 +50,7 @@
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
   explicit operator int() const { return V; }
+  explicit operator bool() const { return V; }
 
   APSInt toAPSInt() const {
 return APSInt(APInt(1, static_cast(V), false), true);
@@ -85,9 +86,10 @@
   static Boolean min(unsigned NumBits) { return Boolean(false); }
   static Boolean max(unsigned NumBits) { return Boolean(true); }
 
-  template 
-  static std::enable_if_t::value, Boolean> from(T Value) {
-return Boolean(Value != 0);
+  template  static Boolean from(T Value) {
+if constexpr (std::is_integral::value)
+  return Boolean(Value != 0);
+return Boolean(static_cast(Value) != 0);
   }
 
   template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132568: [clang][Sema] check default argument promotions for printf

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
inclyc updated this revision to Diff 455898.
inclyc added a comment.

Update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/FormatString.h
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/format-strings-freebsd.c
  clang/test/Sema/format-strings-scanf.c
  clang/test/Sema/format-strings.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -819,13 +819,7 @@
 
   Unclear type relationship between a format specifier and its argument
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf";>N2562
-  
-Partial
-  Clang supports diagnostics checking format specifier validity, but
-  does not yet account for all of the changes in this paper, especially
-  regarding length modifiers like h and hh.
-
-  
+  Clang 16
 
 
 
Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -830,3 +830,58 @@
   printf_arg2("foo", "%s string %i\n", "aaa", 123);
   printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
 }
+
+void test_promotion(void) {
+  // Default argument promotions for *printf in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  char c;
+  short ss;
+  unsigned short us;
+
+  printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+  printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+  // %ld %lld %llx
+  printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+  printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+  printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+  printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+  printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+  printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+
+  // ill formed spec for floats
+  printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+
+  // for %hhd and `short` they are compatible by promotions but more likely misuse
+  printf("%hd", ss); // no-warning
+  printf("%hhd", ss); // expected-warning{{format specifies type 'char' but the argument has type 'short'}}
+  printf("%hu", us); // no-warning
+  printf("%hhu", ss); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
+
+  // floats & integers are not compatible
+  printf("%f", i); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+  printf("%f", sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+  printf("%f", uc); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned char'}}
+  printf("%f", c); // expected-warning{{format specifies type 'double' but the argument has type 'char'}}
+  printf("%f", ss); // expected-warning{{format specifies type 'double' but the argument has type 'short'}}
+  printf("%f", us); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned short'}}
+
+  // character literals
+  // In C language engineering practice, printing a character literal with %hhd or %d is common, but %hd may be misuse.
+  printf("%hhu", 'a'); // no-warning
+  printf("%hhd", 'a'); // no-warning
+  printf("%hd", 'a'); // expected-warning{{format specifies type 'short' but the argument has type 'char'}}
+  printf("%hu", 'a'); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'char'}}
+  printf("%d", 'a'); // no-warning
+  printf("%u", 'a'); // no-warning
+
+
+  // pointers
+  printf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+}
Index: clang/test/Sema/format-strings-scanf.c
===
--- clang/test/Sema/format-strings-scanf.c
+++ clang/test/Sema/for

[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:433
+def ToCastTypeClass : TypeClass {
+  let Types = [Sint32, Bool];
 }

tbaeder wrote:
> erichkeane wrote:
> > tbaeder wrote:
> > > erichkeane wrote:
> > > > Not sure how this works... but is this ONLY int-32 to bool and vice 
> > > > versa?  The implementation of any of the other integrals should be 
> > > > trivial.
> > > Yes exactly, the primitive types are:
> > > ```
> > > def Bool : Type;
> > > def Sint8 : Type;
> > > def Uint8 : Type;
> > > def Sint16 : Type;
> > > def Uint16 : Type;
> > > def Sint32 : Type;
> > > def Uint32 : Type;
> > > def Sint64 : Type;
> > > def Uint64 : Type;
> > > def Ptr : Type;
> > > ```
> > So why not fill them in here as well? 
> I think I can add `Uint32` as well, but for the others I can't write tests 
> yet since I don't have their literals implemented yet.
Got it, thanks!


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

https://reviews.llvm.org/D132739

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


[clang] 01bebed - [Basic] Drop header-only dependency from Basic to Lex

2022-08-26 Thread Benjamin Kramer via cfe-commits

Author: Benjamin Kramer
Date: 2022-08-26T16:03:22+02:00
New Revision: 01bebedaf09bcb7b4f00b8d37a121e68316f9f59

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

LOG: [Basic] Drop header-only dependency from Basic to Lex

It's still a bit weird for IdentifierTable to depend on Lex diagnostics,
but we can get away with including the enum info that's in Basic already.

Added: 


Modified: 
clang/include/clang/Basic/IdentifierTable.h
clang/lib/Basic/IdentifierTable.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/IdentifierTable.h 
b/clang/include/clang/Basic/IdentifierTable.h
index 1e89aa5f670f..f98ea48f952f 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -15,9 +15,9 @@
 #ifndef LLVM_CLANG_BASIC_IDENTIFIERTABLE_H
 #define LLVM_CLANG_BASIC_IDENTIFIERTABLE_H
 
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/TokenKinds.h"
-#include "clang/Lex/LexDiagnostic.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"

diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index 5d413a8da6d3..3940d4bc5de3 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/Specifiers.h"



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


[clang] 8360174 - Fix the Sphinx build bot

2022-08-26 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-26T10:04:57-04:00
New Revision: 8360174fb1a2d0ed759bb0a97708add44deddf92

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

LOG: Fix the Sphinx build bot

This addresses an accidental break from
41667a8b9b624e282e7c08fadf7091223728d1c1

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 75f7f7f293f4..3f7eec1effd4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -110,8 +110,8 @@ Improvements to Clang's diagnostics
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
 
- Non-comprehensive list of changes in this release
- -
+Non-comprehensive list of changes in this release
+-
 
 
 Non-comprehensive list of changes in this release



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


[PATCH] D119147: [AIX][clang][driver] Check the command string to the linker for exportlist opts

2022-08-26 Thread David Tenty via Phabricator via cfe-commits
daltenty accepted this revision.
daltenty added a comment.

LGTM, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147

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


[PATCH] D132607: [OffloadPackager] Add ability to extract mages from other file types

2022-08-26 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp:17-21
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IRReader/IRReader.h"
+#include "llvm/Object/Archive.h"
+#include "llvm/Object/ArchiveWriter.h"

Are these include files necessary? I do not see code changes that need new 
include files. Or they were missing before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132607

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


[PATCH] D132607: [OffloadPackager] Add ability to extract mages from other file types

2022-08-26 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp:17-21
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IRReader/IRReader.h"
+#include "llvm/Object/Archive.h"
+#include "llvm/Object/ArchiveWriter.h"

yaxunl wrote:
> Are these include files necessary? I do not see code changes that need new 
> include files. Or they were missing before?
You're right, they were included previously but I since rolled that logic into 
D132689. I'll remove them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132607

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


[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

2022-08-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 455906.
pengfei added a comment.

Address Yuanke's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512bf16intrin.h
  clang/lib/Headers/avx512vlbf16intrin.h
  clang/test/CodeGen/X86/avx512bf16-builtins.c
  clang/test/CodeGen/X86/avx512bf16-error.c
  clang/test/CodeGen/X86/avx512vlbf16-builtins.c
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrFragmentsSIMD.td
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/CodeGen/X86/avx512bf16-intrinsics.ll
  llvm/test/CodeGen/X86/avx512bf16-vl-intrinsics.ll
  llvm/test/CodeGen/X86/stack-folding-avx512bf16.ll

Index: llvm/test/CodeGen/X86/stack-folding-avx512bf16.ll
===
--- llvm/test/CodeGen/X86/stack-folding-avx512bf16.ll
+++ llvm/test/CodeGen/X86/stack-folding-avx512bf16.ll
@@ -9,7 +9,7 @@
 ; By including a nop call with sideeffects we can force a partial register spill of the
 ; relevant registers and check that the reload is correctly folded into the instruction.
 
-define <32 x i16> @stack_fold_cvtne2ps2bf16(<16 x float> %a0, <16 x float> %a1) {
+define <32 x bfloat> @stack_fold_cvtne2ps2bf16(<16 x float> %a0, <16 x float> %a1) {
 ; CHECK-LABEL: stack_fold_cvtne2ps2bf16:
 ; CHECK:   # %bb.0:
 ; CHECK-NEXT:vmovups %zmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 64-byte Spill
@@ -19,12 +19,12 @@
 ; CHECK-NEXT:vcvtne2ps2bf16 {{[-0-9]+}}(%r{{[sb]}}p), %zmm0, %zmm0 # 64-byte Folded Reload
 ; CHECK-NEXT:retq
   %1 = tail call <2 x i64> asm sideeffect "nop", "=x,~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{xmm16},~{xmm17},~{xmm18},~{xmm19},~{xmm20},~{xmm21},~{xmm22},~{xmm23},~{xmm24},~{xmm25},~{xmm26},~{xmm27},~{xmm28},~{xmm29},~{xmm30},~{xmm31},~{flags}"()
-  %2 = call <32 x i16> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %a0, <16 x float> %a1)
-  ret <32 x i16> %2
+  %2 = call <32 x bfloat> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %a0, <16 x float> %a1)
+  ret <32 x bfloat> %2
 }
-declare <32 x i16> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float>, <16 x float>)
+declare <32 x bfloat> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float>, <16 x float>)
 
-define <32 x i16> @stack_fold_cvtne2ps2bf16_mask(<16 x float> %a0, <16 x float> %a1, ptr %passthru, i32 %U) {
+define <32 x bfloat> @stack_fold_cvtne2ps2bf16_mask(<16 x float> %a0, <16 x float> %a1, ptr %passthru, i32 %U) {
 ; CHECK-LABEL: stack_fold_cvtne2ps2bf16_mask:
 ; CHECK:   # %bb.0:
 ; CHECK-NEXT:vmovups %zmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 64-byte Spill
@@ -37,15 +37,15 @@
 ; CHECK-NEXT:vmovaps %zmm2, %zmm0
 ; CHECK-NEXT:retq
   %1 = tail call <2 x i64> asm sideeffect "nop", "=x,~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{xmm16},~{xmm17},~{xmm18},~{xmm19},~{xmm20},~{xmm21},~{xmm22},~{xmm23},~{xmm24},~{xmm25},~{xmm26},~{xmm27},~{xmm28},~{xmm29},~{xmm30},~{xmm31},~{flags}"()
-  %2 = call <32 x i16> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %a0, <16 x float> %a1)
+  %2 = call <32 x bfloat> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %a0, <16 x float> %a1)
   %3 = bitcast i32 %U to <32 x i1>
   ; load needed to keep the operation from being scheduled above the asm block
-  %4 = load <32 x i16>, ptr %passthru
-  %5 = select <32 x i1> %3, <32 x i16> %2, <32 x i16> %4
-  ret <32 x i16> %5
+  %4 = load <32 x bfloat>, ptr %passthru
+  %5 = select <32 x i1> %3, <32 x bfloat> %2, <32 x bfloat> %4
+  ret <32 x bfloat> %5
 }
 
-define <32 x i16> @stack_fold_cvtne2ps2bf16_maskz(<16 x float> %a0, <16 x float> %a1, i32 %U) {
+define <32 x bfloat> @stack_fold_cvtne2ps2bf16_maskz(<16 x float> %a0, <16 x float> %a1, i32 %U) {
 ; CHECK-LABEL: stack_fold_cvtne2ps2bf16_maskz:
 ; CHECK:   # %bb.0:
 ; CHECK-NEXT:vmovups %zmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 64-byte Spill
@@ -56,13 +56,13 @@
 ; CHECK-NEXT:vcvtne2ps2bf16 {{[-0-9]+}}(%r{{[sb]}}p), %zmm0, %zmm0 {%k1} {z} # 64-byte Folded Reload
 ; CHECK-NEXT:retq
   %1 = tail call <2 x i64> asm sideeffect "nop", "=x,~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{xmm16},~{xmm17},~{xmm18},~{xmm19},~{xmm20},~{xmm21},~{xmm22},~{xmm23},~{xmm24},~{xmm25},~{xmm26},~{xmm27},~{xmm28},~{xmm29},~{xmm30},~{xmm31},~{flags}"()
-  %2 = call <32 x i16> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %a0, <16 x float> %a1)
+  %2 = call <32 x bfloat> @llvm.x86.avx512bf16.cvtne2ps2bf16.512(<16 x float> %

[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

2022-08-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2472
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});

LuoYuanke wrote:
> This looks irrelative to the patch.
The use of `__bf16` in intrinsics leads to new lit fails due to no mangling 
support on Windows. But I can do it in a separate patch.



Comment at: clang/test/CodeGen/X86/avx512bf16-builtins.c:7
 
-float test_mm_cvtsbh_ss(__bfloat16 A) {
-  // CHECK-LABEL: @test_mm_cvtsbh_ss
-  // CHECK: zext i16 %{{.*}} to i32
-  // CHECK: shl i32 %{{.*}}, 16
-  // CHECK: bitcast i32 %{{.*}} to float
+float test_mm_cvtsbh_ss(__bf16 A) {
+  // CHECK: fpext bfloat %{{.*}} to float

LuoYuanke wrote:
> Add a test case for `__bfloat16` to test compatibility?
GCC folks prefer to not providing `__bfloat16`, but I'd like to deprecate it 
first. So we don't need test for it.



Comment at: llvm/include/llvm/IR/IntrinsicsX86.td:4904
   ClangBuiltin<"__builtin_ia32_cvtne2ps2bf16_128">,
-  Intrinsic<[llvm_v8i16_ty], [llvm_v4f32_ty, llvm_v4f32_ty],
+  Intrinsic<[llvm_v8bf16_ty], [llvm_v4f32_ty, llvm_v4f32_ty],
   [IntrNoMem]>;

LuoYuanke wrote:
> Probably need to upgrade the old intrinsics to new version for IR 
> compatibility or we can keep IR unchanged and just generate bitcast from 
> bfloat16 to i16.
Good suggestion!



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:2180
+  if (!Subtarget.useSoftFloat() && Subtarget.hasBF16()) {
+addRegisterClass(MVT::bf16, &X86::FR16XRegClass);
+addRegisterClass(MVT::v8bf16, &X86::VR128XRegClass);

LuoYuanke wrote:
> Not sure about this. Does it make bf16 legal type?
Good catch! I made it legal to lower `BUILD_VECTOR`. But yes, it results in the 
scalar lowering failing with AVX512BF16.
I fixed the problem by adding customized code. It works for both scalar 
lowering and AVX512BF16 intrinsics lowering now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

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


[PATCH] D132742: [X86][BF16] Add type mangling for Windows

2022-08-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei created this revision.
pengfei added reviewers: FreddyYe, LuoYuanke, craig.topper, RKSimon, skan.
Herald added a subscriber: StephenFan.
Herald added a project: All.
pengfei 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/D132742

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGen/X86/bfloat-mangle.cpp


Index: clang/test/CodeGen/X86/bfloat-mangle.cpp
===
--- clang/test/CodeGen/X86/bfloat-mangle.cpp
+++ clang/test/CodeGen/X86/bfloat-mangle.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple i386-windows-msvc -target-feature +sse2 -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=WINDOWS
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
 
-// CHECK: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// LINUX: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// WINDOWS: define {{.*}}void @"?foo@@YAXU__bf16@__clang@@@Z"(bfloat noundef 
%b)
 void foo(__bf16 b) {}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2469,6 +2469,10 @@
   Out << "$halff@";
 break;
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});
+break;
+
 #define SVE_TYPE(Name, Id, SingletonId) \
   case BuiltinType::Id:
 #include "clang/Basic/AArch64SVEACLETypes.def"
@@ -2501,7 +2505,6 @@
   case BuiltinType::SatUShortFract:
   case BuiltinType::SatUFract:
   case BuiltinType::SatULongFract:
-  case BuiltinType::BFloat16:
   case BuiltinType::Ibm128:
   case BuiltinType::Float128: {
 DiagnosticsEngine &Diags = Context.getDiags();


Index: clang/test/CodeGen/X86/bfloat-mangle.cpp
===
--- clang/test/CodeGen/X86/bfloat-mangle.cpp
+++ clang/test/CodeGen/X86/bfloat-mangle.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple i386-windows-msvc -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
 
-// CHECK: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// LINUX: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// WINDOWS: define {{.*}}void @"?foo@@YAXU__bf16@__clang@@@Z"(bfloat noundef %b)
 void foo(__bf16 b) {}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2469,6 +2469,10 @@
   Out << "$halff@";
 break;
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});
+break;
+
 #define SVE_TYPE(Name, Id, SingletonId) \
   case BuiltinType::Id:
 #include "clang/Basic/AArch64SVEACLETypes.def"
@@ -2501,7 +2505,6 @@
   case BuiltinType::SatUShortFract:
   case BuiltinType::SatUFract:
   case BuiltinType::SatULongFract:
-  case BuiltinType::BFloat16:
   case BuiltinType::Ibm128:
   case BuiltinType::Float128: {
 DiagnosticsEngine &Diags = Context.getDiags();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] be19952 - Fix the lldb test bots

2022-08-26 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-26T11:19:42-04:00
New Revision: be199527205dc8a8c7febc057ad6be90fac15547

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

LOG: Fix the lldb test bots

This addresses an accidental change in behavior from
41667a8b9b624e282e7c08fadf7091223728d1c1 to get the bots back to green.
However, I think there's an issue with LLDB assuming it's valid to
enable support for keywords in language modes that don't support the
keyword (as other parts of Clang are not expecting to be able to do
that).

This should fix (and others):
https://lab.llvm.org/buildbot/#/builders/68/builds/38374

Added: 


Modified: 
clang/include/clang/Basic/TokenKinds.def
clang/lib/Basic/IdentifierTable.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TokenKinds.def 
b/clang/include/clang/Basic/TokenKinds.def
index 729442a79f05..f6a16cc019de 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -330,7 +330,7 @@ KEYWORD(__objc_no   , KEYALL)
 
 // C++ 2.11p1: Keywords.
 KEYWORD(asm , KEYCXX|KEYGNU)
-KEYWORD(bool, KEYCXX|KEYOPENCLC|KEYOPENCLCXX|KEYC2X)
+KEYWORD(bool, BOOLSUPPORT|KEYC2X)
 KEYWORD(catch   , KEYCXX)
 KEYWORD(class   , KEYCXX)
 KEYWORD(const_cast  , KEYCXX)
@@ -338,7 +338,7 @@ KEYWORD(delete  , KEYCXX)
 KEYWORD(dynamic_cast, KEYCXX)
 KEYWORD(explicit, KEYCXX)
 KEYWORD(export  , KEYCXX)
-KEYWORD(false   , KEYCXX|KEYOPENCLC|KEYOPENCLCXX|KEYC2X)
+KEYWORD(false   , BOOLSUPPORT|KEYC2X)
 KEYWORD(friend  , KEYCXX)
 KEYWORD(mutable , KEYCXX)
 KEYWORD(namespace   , KEYCXX)
@@ -352,7 +352,7 @@ KEYWORD(static_cast , KEYCXX)
 KEYWORD(template, KEYCXX)
 KEYWORD(this, KEYCXX)
 KEYWORD(throw   , KEYCXX)
-KEYWORD(true, KEYCXX|KEYOPENCLC|KEYOPENCLCXX|KEYC2X)
+KEYWORD(true, BOOLSUPPORT|KEYC2X)
 KEYWORD(try , KEYCXX)
 KEYWORD(typename, KEYCXX)
 KEYWORD(typeid  , KEYCXX)

diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index 3940d4bc5de3..2035ca2f7ce1 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -89,25 +89,26 @@ namespace {
 KEYCXX11  = 0x4,
 KEYGNU= 0x8,
 KEYMS = 0x10,
-KEYALTIVEC= 0x20,
-KEYNOCXX  = 0x40,
-KEYBORLAND= 0x80,
-KEYOPENCLC= 0x100,
-KEYC2X= 0x200,
-KEYNOMS18 = 0x400,
-KEYNOOPENCL   = 0x800,
-WCHARSUPPORT  = 0x1000,
-HALFSUPPORT   = 0x2000,
-CHAR8SUPPORT  = 0x4000,
-KEYOBJC   = 0x8000,
-KEYZVECTOR= 0x1,
-KEYCOROUTINES = 0x2,
-KEYMODULES= 0x4,
-KEYCXX20  = 0x8,
-KEYOPENCLCXX  = 0x10,
-KEYMSCOMPAT   = 0x20,
-KEYSYCL   = 0x40,
-KEYCUDA   = 0x80,
+BOOLSUPPORT   = 0x20,
+KEYALTIVEC= 0x40,
+KEYNOCXX  = 0x80,
+KEYBORLAND= 0x100,
+KEYOPENCLC= 0x200,
+KEYC2X= 0x400,
+KEYNOMS18 = 0x800,
+KEYNOOPENCL   = 0x1000,
+WCHARSUPPORT  = 0x2000,
+HALFSUPPORT   = 0x4000,
+CHAR8SUPPORT  = 0x8000,
+KEYOBJC   = 0x1,
+KEYZVECTOR= 0x2,
+KEYCOROUTINES = 0x4,
+KEYMODULES= 0x8,
+KEYCXX20  = 0x10,
+KEYOPENCLCXX  = 0x20,
+KEYMSCOMPAT   = 0x40,
+KEYSYCL   = 0x80,
+KEYCUDA   = 0x100,
 KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
 KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
@@ -161,6 +162,9 @@ static KeywordStatus getKeywordStatusHelper(const 
LangOptions &LangOpts,
 return LangOpts.GNUKeywords ? KS_Extension : KS_Unknown;
   case KEYMS:
 return LangOpts.MicrosoftExt ? KS_Extension : KS_Unknown;
+  case BOOLSUPPORT:
+if (LangOpts.Bool)  return KS_Enabled;
+return !LangOpts.CPlusPlus ? KS_Future : KS_Unknown;
   case KEYALTIVEC:
 return LangOpts.AltiVec ? KS_Enabled : KS_Unknown;
   case KEYBORLAND:



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


[PATCH] D132236: [analyzer] Fix liveness of LazyCompoundVals

2022-08-26 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D132236#3747652 , @NoQ wrote:

> Nice catch but I don't think this is the right solution.
>
> Symbol liveness corresponds to a concrete runtime property of the program: 
> can the program obtain this value by executing further runtime instructions? 
> The program cannot obtain a pointer to variable `n`, or even a pointer to a 
> region `*n`, just by looking at the data in a struct that was copied from 
> region `*n` previously. Therefore neither `n` nor `*n` should not be marked 
> live just because they are mentioned in a base region of a lazy compound 
> value. In particular, memory leaks should be reported against `n` if the last 
> place it's mentioned is a base region of a lazy compound value, as the 
> program cannot possibly free that memory. You can most likely write a 
> MallocChecker test for that, which will become a false negative after your 
> patch.
>
> For the same reason, for any region `R`, if `reg` or `derived` is 
> live, does not mean `R` is live. //The program cannot guess a memory address 
> by only looking at a value loaded from that address.//
>
> If you look at how a simpler example works:
>
>   void clang_analyzer_warnIfReached();
>   
>   void foo(int **n) {
> int *n2 = *n;
> if (!**n) {
>   if (*n2) {
> clang_analyzer_warnIfReached();
>   }
> }
>   }
>
> You will notice that it does not try to keep VarRegion `n` or even the symbol 
> `reg_$0` alive. It is *sufficient* to keep `reg_$1 Element{SymRegion{reg_$0},0 S64b,int *}>` alive in order to keep 
> the test passing, and it does match the actual definition of live symbol 
> (`reg_$0` can no longer be retrieved by the program but `reg_$1` can).

Nice explanation, thank you very much! I'll need some time to digest it to the 
full extent.
It makes sense so far, but I imagine it will become clear when I continue the 
investigation.

> The original code tries to do the same for lazy compound values. I suspect 
> that you'll find the actual bug when you descend into 
> `getInterestingValues()`.
>
> What looks fishy about `getInterestingValues()` is that it assumes that the 
> amount of interesting values is finite. This sounds incredibly wrong to me. 
> If a lazy compound value contains any pointer symbol `$p`, then all values in 
> the following infinite series are interesting:
>
>   $p,  *$p,  **$p,  ***$p,  ...

Well, there two places where I suspected this bug. The place I changed and the 
other was `getInterestingValues()`. The first looked more probable, hence we 
sticked to that.
We'll continue the investigation with the `getInterestingValues()` next time.

> So I think the correct solution would be to maintain a list of 
> explicitly-live compound values. Just like we already maintain a list of 
> explicitly live symbols - `TheLiving`, and explicitly live regions - 
> `RegionRoots`; neither of these lists enumerates all live symbols or regions 
> as there are infinitely many of them, but each of these lists enumerates the 
> ones that we've learned to be definitely live, and they're sufficient to 
> determine liveness of any other symbol through recursive descent into the 
> identity of that symbol. So with a list of live compound value regions, we 
> can ask a question like "is this symbol stored in any of the known-live 
> compound values?" to determine that symbol's liveness. This will keep the 
> stored symbol alive without making the lazy compound value's base region 
> alive.

Okay, I'll continue from here. Thanks for the

In D132236#3748017 , @NoQ wrote:

> So to be clear, I think your solution is probably an ok stop-gap solution. 
> False negatives aren't that bad, I'm more worried that existing leak reports 
> may become less understandable because they'll be reported later than 
> necessary, which may obscure the reason why we think they're finally leaking. 
> But I really want us to agree on how this facility is supposed to work, and 
> explore the perfect solution, before implementing an imperfect solution.

Unfortunately, we found many FPs introduced by this change; so this should be 
definitely blocked.

> Maybe we should introduce "weak region roots" that aren't necessarily live 
> themselves but anything derived from them (any `SymbolRegionValue` or 
> `SymbolDerived` for arbitrary sub-region `R'` of a weak-region-root 
> `R`) is live. This could be pretty straightforward.

So, what you are suggesting here is an alternative solution to the one you 
already proposed in your last comment?

For transparency, we, unfortunately, reached the end of our timebox, and I'll 
work on something else for a couple of weeks.
I plan to come back to this in ~3 weeks if everything works well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132236

___
cfe-commi

[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 455917.
cor3ntin added a comment.

Formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp

Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++2b -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
 
 int nop() { return 0; }
 
@@ -13,6 +13,22 @@
 ++j;// CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
+
+// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
+constexpr int check_consteval(int i) {
+if consteval {
+  i++;
+}
+if !consteval {
+  i++;
+}
+if consteval {
+return 42;
+} else {
+return i;
+}
+}
+
 // CHECK-LABEL: main:
 int main() {// CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
   int i = 0;
@@ -50,6 +66,10 @@
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
   i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
+  // GH-57377
+  constexpr int c_i = check_consteval(0);
+  check_consteval(i);
+
   return 0;
 }
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1377,19 +1377,23 @@
 
 // Extend into the condition before we propagate through it below - this is
 // needed to handle macros that generate the "if" but not the condition.
-extendRegion(S->getCond());
+if (!S->isConsteval())
+  extendRegion(S->getCond());
 
 Counter ParentCount = getRegion().getCounter();
 Counter ThenCount = getRegionCounter(S);
 
-// Emitting a counter for the condition makes it easier to interpret the
-// counter for the body when looking at the coverage.
-propagateCounts(ParentCount, S->getCond());
+if (!S->isConsteval()) {
+  // Emitting a counter for the condition makes it easier to interpret the
+  // counter for the body when looking at the coverage.
+  propagateCounts(ParentCount, S->getCond());
 
-// The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
-if (Gap)
-  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+  // The 'then' count applies to the area immediately after the condition.
+  Optional Gap =
+  findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+  if (Gap)
+fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+}
 
 extendRegion(S->getThen());
 Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1402,9 @@
 if (const Stmt *Else = S->getElse()) {
   bool ThenHasTerminateStmt = HasTerminateStmt;
   HasTerminateStmt = false;
-
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  Optional Gap =
+  findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);
@@ -1416,9 +1420,11 @@
   GapRegionCounter = OutCount;
 }
 
-// Create Branch Region around condition.
-createBranchRegion(S->getCond(), ThenCount,
-   subtractCounters(ParentCount, ThenCount));
+if (!S->isConsteval()) {
+  // Create Branch Region around condition.
+  createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
+}
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -65,6 +65,9 @@
 - Fix a crash when evaluating a multi-dimensional array's array filler
   expression is element-dependent. This fixes
   `Issue 

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-26 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 455920.
usaxena95 marked 11 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -196,7 +196,7 @@
   ElementsAre(SourceAnnotations.range("empty")));
 }
 
-TEST(FoldingRanges, All) {
+TEST(FoldingRanges, ASTAll) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -265,7 +265,7 @@
   }
 }
 
-TEST(FoldingRangesPseudoParser, All) {
+TEST(FoldingRanges, PseudoParserWithoutLineFoldings) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -359,13 +359,67 @@
   };
   for (const char *Test : Tests) {
 auto T = Annotations(Test);
-EXPECT_THAT(
-gatherFoldingRanges(llvm::cantFail(getFoldingRanges(T.code().str(,
-UnorderedElementsAreArray(T.ranges()))
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(
+T.code().str(), /*LineFoldingsOnly=*/false))),
+UnorderedElementsAreArray(T.ranges()))
 << Test;
   }
 }
 
+TEST(FoldingRanges, PseudoParserLineFoldingsOnly) {
+  const char *Tests[] = {
+  R"cpp(
+void func(int a) {[[
+a++;]]
+}
+  )cpp",
+  R"cpp(
+// Always exclude last line for brackets.
+void func(int a) {[[
+  if(a == 1) {[[
+a++;]]
+  } else if (a == 2){[[
+a--;]]
+  } else {  // No folding for 2 line bracketed ranges.
+  }]]
+}
+  )cpp",
+  R"cpp(
+[[/* comment
+* comment]]
+*/
+
+/* No folding for this comment.
+*/
+
+// No folding for this comment.
+
+[[// 2 single line comment.
+// 2 single line comment.]]
+
+[[// >=2 line comments.
+// >=2 line comments.
+// >=2 line comments.]]
+  )cpp",
+
+  };
+  auto StripColumns = [](const std::vector &Ranges) {
+std::vector Res;
+for (Range R : Ranges) {
+  R.start.character = R.end.character = 0;
+  Res.push_back(R);
+}
+return Res;
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+EXPECT_THAT(
+StripColumns(gatherFoldingRanges(llvm::cantFail(
+getFoldingRanges(T.code().str(), /*LineFoldingsOnly=*/true,
+UnorderedElementsAreArray(StripColumns(T.ranges(
+<< Test;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -33,7 +33,7 @@
 /// Returns a list of ranges whose contents might be collapsible in an editor.
 /// This version uses the pseudoparser which does not require the AST.
 llvm::Expected>
-getFoldingRanges(const std::string &Code);
+getFoldingRanges(const std::string &Code, bool LineFoldingOnly);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -179,7 +179,7 @@
 // statement bodies).
 // Related issue: https://github.com/clangd/clangd/issues/310
 llvm::Expected>
-getFoldingRanges(const std::string &Code) {
+getFoldingRanges(const std::string &Code, bool LineFoldingOnly) {
   auto OrigStream = pseudo::lex(Code, clang::pseudo::genericLangOpts());
 
   auto DirectiveStructure = pseudo::DirectiveTree::parse(OrigStream);
@@ -192,15 +192,17 @@
   pseudo::pairBrackets(ParseableStream);
 
   std::vector Result;
-  auto ToFoldingRange = [](Position Start, Position End,
-   llvm::StringLiteral Kind) {
+  auto AddFoldingRange = [&](Position Start, Position End,
+ llvm::StringLiteral Kind) {
+if (Start.line >= End.line)
+  return;
 FoldingRange FR;
 FR.startLine = Start.line;
-FR.startCharacter = Start.character;
 FR.endLine = End.line;
+FR.startCharacter = Start.character;
 FR.endCharacter = End.character;
 FR.kind = Kind.str();

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-26 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:391
+/* No folding for this comment.
+*/ int b_token=0;
+  )cpp",

kadircet wrote:
> it'd be nice to have a test case like:
> ```
> template <[[typename foo, class bar]]> struct baz {[[]]};
> ```
> 
> which isn't useful for line-only folding clients, but something we should be 
> able to support going forward.
I don't understand. We do not return any single-line ranges.
I think lines are never long enough to have helpful foldings (atleast in 
formatted code). It would only clutter  IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

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


[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the fix!




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1407
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  auto Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)

cor3ntin wrote:
> aaron.ballman wrote:
> > 1) Pick a name which doesn't shadow
> > 2) Spell out the type instead of using `auto`
> `auto Gap...` was preexisting, I changed both instances. Both variables are 
> in their own non nested scopes though.
Yeah, I somehow missed these were in different scopes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

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


[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

2022-08-26 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

In D132186#3750925 , @iamarchit123 
wrote:

> @paulkirth  this change was done under the intuition that marking hot 
> functions noinline may hurt performance. This change till now hasn't been 
> tested for performance improvements and so the points that you raise that 
> functions marked noinline are marked not for hotness/performance but rather 
> correctness is something that I was unaware of. Since I don't have access to 
> big services currently and can't test this change, so I may be unable to 
> defend this change. @modimo feel free if you think folks can come back and 
> test this change for effectiveness or completely redo this if the above 
> change is still insufficient.
>
> I agree until it's shown/proven that there is a serious performance win this 
> change may not be useful. Thanks for the review.

I have seen a few cases where noinline was used for performance, in addition to 
other cases like avoiding too much stack growth. I've also seen it used without 
any comment whatsoever. So I think it would be good to make it easier to 
identify cases where we are blocked from inlining at hot callsites because of 
the attribute.

It is a little different than misexpect though in that the expect hints are 
pretty much only for performance, so it is more useful to be able to issue a 
strong warning that can be turned into an error if they are wrong. And also 
there was no way to report the misuse of expects earlier, unlike inlining where 
we already had the remarks plumbing.

I haven't looked through the patch in detail, but is it possible to use your 
changes to emit a better missed opt remark from the inliner for these cases (I 
assume we will already emit a -Rpass-missed=inline for the noinline attribute 
case, just not highlighting that it is hot and would have been inlined for 
performance reasons otherwise)? I suppose one main reason for adding a warning 
is that the missed inline remarks can be really noisy and not really useful to 
the user vs a compiler optimization engineer doing inliner/compiler tuning, and 
therefore a warning would make it easier to turn on more widely as user 
feedback that can/should be addressed in user code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132186

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


[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

2022-08-26 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa updated this revision to Diff 455918.
yihanaa added a comment.

Thanks John and Erich for your comments.
In this update:

Check args in Sema, and both in C and C++, emit an error if the 1st arg of 
__builtin_assume_aligned is volatile qualified or 1st arg not a pointer type.

Remove getSubExprAsWritten in emitAlignmentAssumption, and Emit a cast if 1st 
arg not void * type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-cannot-volatile.c
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-not-pointer.c
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c

Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -21,8 +21,3 @@
   // CHECK: call void @__ubsan_handle_alignment_assumption(
   return __builtin_assume_aligned(x, 1);
 }
-
-// CHECK-LABEL: ignore_volatiles
-void *ignore_volatiles(volatile void * x) {
-  return __builtin_assume_aligned(x, 1);
-}
Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 31, i32 35 }, {{.*}}* @[[CHAR]] }
+
+void *caller(void) {
+  char str[] = "";
+  // CHECK:   define{{.*}}
+  // CHECK-NEXT:  entry:
+  // CHECK-NEXT:%[[STR:.*]] = alloca [1 x i8], align 1
+  // CHECK-NEXT:%[[BITCAST:.*]] = bitcast [1 x i8]* %[[STR]] to i8*
+  // CHECK-NEXT:call void @llvm.memset.p0i8.i64(i8* align 1 %[[BITCAST]], i8 0, i64 1, i1 false)
+  // CHECK-NEXT:%[[ARRAYDECAY:.*]] = getelementptr inbounds [1 x i8], [1 x i8]* %[[STR]], i64 0, i64 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64
+  // CHECK-SANITIZE-NEXT:   %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 0
+  // CHECK-SANITIZE-NEXT:   %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:   br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:   call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-TRAP-NEXT:  call void @llvm.ubsantrap(i8 23){{.*}}, !nosanitize
+  // CHECK-SANITIZE-UNREACHABLE-NEXT:   unreachable, !nosanitize
+  // CHECK-SANITIZE:  [[CONT]]:
+  // CHECK-NEXT:call void @llvm.assume(i1 true) [ "alig

[PATCH] D132607: [OffloadPackager] Add ability to extract mages from other file types

2022-08-26 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 455929.
jhuber6 added a comment.

Removing unused headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132607

Files:
  clang/test/Driver/offload-packager.c
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp


Index: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
===
--- clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
+++ clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
@@ -14,8 +14,7 @@
 
 #include "clang/Basic/Version.h"
 
-#include "llvm/Object/Binary.h"
-#include "llvm/Object/ObjectFile.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Object/OffloadBinary.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileOutputBuffer.h"
@@ -123,29 +122,6 @@
   return Error::success();
 }
 
-static Expected>>
-extractOffloadFiles(MemoryBufferRef Contents) {
-  if (identify_magic(Contents.getBuffer()) != file_magic::offload_binary)
-return createStringError(inconvertibleErrorCode(),
- "Input buffer not an offloading binary");
-  SmallVector> Binaries;
-  uint64_t Offset = 0;
-  // There could be multiple offloading binaries stored at this section.
-  while (Offset < Contents.getBuffer().size()) {
-std::unique_ptr Buffer =
-MemoryBuffer::getMemBuffer(Contents.getBuffer().drop_front(Offset), "",
-   /*RequiresNullTerminator*/ false);
-auto BinaryOrErr = OffloadBinary::create(*Buffer);
-if (!BinaryOrErr)
-  return BinaryOrErr.takeError();
-
-Offset += (*BinaryOrErr)->getSize();
-Binaries.emplace_back(std::move(*BinaryOrErr));
-  }
-
-  return std::move(Binaries);
-}
-
 static Error unbundleImages() {
   ErrorOr> BufferOrErr =
   MemoryBuffer::getFileOrSTDIN(InputFile);
@@ -159,9 +135,9 @@
 Buffer = MemoryBuffer::getMemBufferCopy(Buffer->getBuffer(),
 Buffer->getBufferIdentifier());
 
-  auto BinariesOrErr = extractOffloadFiles(*Buffer);
-  if (!BinariesOrErr)
-return BinariesOrErr.takeError();
+  SmallVector Binaries;
+  if (Error Err = extractOffloadBinaries(*Buffer, Binaries))
+return Err;
 
   // Try to extract each device image specified by the user from the input 
file.
   for (StringRef Image : DeviceImages) {
@@ -169,8 +145,8 @@
 StringSaver Saver(Alloc);
 auto Args = getImageArguments(Image, Saver);
 
-for (uint64_t I = 0, E = BinariesOrErr->size(); I != E; ++I) {
-  const auto &Binary = (*BinariesOrErr)[I];
+for (uint64_t I = 0, E = Binaries.size(); I != E; ++I) {
+  const auto *Binary = Binaries[I].getBinary();
   // We handle the 'file' and 'kind' identifiers differently.
   bool Match = llvm::all_of(Args, [&](auto &Arg) {
 const auto [Key, Value] = Arg;
Index: clang/test/Driver/offload-packager.c
===
--- clang/test/Driver/offload-packager.c
+++ clang/test/Driver/offload-packager.c
@@ -29,3 +29,25 @@
 // RUN: diff *-amdgcn-amd-amdhsa-gfx908.2.o %S/Inputs/dummy-elf.o; rm 
*-amdgcn-amd-amdhsa-gfx908.2.o
 // RUN: diff *-amdgcn-amd-amdhsa-gfx90a.3.o %S/Inputs/dummy-elf.o; rm 
*-amdgcn-amd-amdhsa-gfx90a.3.o
 // RUN: not diff *-amdgcn-amd-amdhsa-gfx90c.4.o %S/Inputs/dummy-elf.o
+
+// Check that we can extract from an ELF object file
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   
--image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908
 \
+// RUN:   
--image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o 
-fembed-offload-object=%t.out
+// RUN: clang-offload-packager %t.out \
+// RUN:   
--image=file=%t-sm_70.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
+// RUN:   
--image=file=%t-gfx908.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908
+// RUN: diff %t-sm_70.o %S/Inputs/dummy-elf.o
+// RUN: diff %t-gfx908.o %S/Inputs/dummy-elf.o
+
+// Check that we can extract from a bitcode file
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   
--image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908
 \
+// RUN:   
--image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-llvm -o %t.bc 
-fembed-offload-object=%t.out
+// RUN: clang-offload-packager %t.out \
+// RUN:   
--image=file=%t-sm_70.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
+// RUN:   
--image=file=%t-gfx908.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908
+// RUN: diff %t-sm_70.o %S/Inputs/dummy-elf.o
+// RUN: diff %t-gfx908.o %S/Inputs/dummy-elf.o


Index: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
===

[clang] 463e30f - [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot via cfe-commits

Author: Corentin Jabot
Date: 2022-08-26T17:46:53+02:00
New Revision: 463e30f51fd07921f32c6630afb3f8dd18d6d2f5

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

LOG: [Clang] Fix crash in coverage of if consteval.

Clang crashes when encountering an `if consteval` statement.
This is the minimum fix not to crash.
The fix is consistent with the current behavior of if constexpr,
which does generate coverage data for the discarded branches.
This is of course not correct and a better solution is
needed for both if constexpr and if consteval.
See https://github.com/llvm/llvm-project/issues/54419.

Fixes #57377

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/if.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3f7eec1effd46..b6d1dbedb2c59 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -78,6 +78,10 @@ Bug Fixes
 - ``-Wtautological-compare`` missed warnings for tautological comparisons
   involving a negative integer literal. This fixes
   `Issue 42918 `_.
+- Fix a crash when generating code coverage information for an
+  ``if consteval`` statement. This fixes
+  `Issue 57377 `_.
+
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0fe084b628dab..836aabf80179d 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1377,19 +1377,23 @@ struct CounterCoverageMappingBuilder
 
 // Extend into the condition before we propagate through it below - this is
 // needed to handle macros that generate the "if" but not the condition.
-extendRegion(S->getCond());
+if (!S->isConsteval())
+  extendRegion(S->getCond());
 
 Counter ParentCount = getRegion().getCounter();
 Counter ThenCount = getRegionCounter(S);
 
-// Emitting a counter for the condition makes it easier to interpret the
-// counter for the body when looking at the coverage.
-propagateCounts(ParentCount, S->getCond());
+if (!S->isConsteval()) {
+  // Emitting a counter for the condition makes it easier to interpret the
+  // counter for the body when looking at the coverage.
+  propagateCounts(ParentCount, S->getCond());
 
-// The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
-if (Gap)
-  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+  // The 'then' count applies to the area immediately after the condition.
+  Optional Gap =
+  findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+  if (Gap)
+fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+}
 
 extendRegion(S->getThen());
 Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1402,9 @@ struct CounterCoverageMappingBuilder
 if (const Stmt *Else = S->getElse()) {
   bool ThenHasTerminateStmt = HasTerminateStmt;
   HasTerminateStmt = false;
-
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  Optional Gap =
+  findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);
@@ -1416,9 +1420,11 @@ struct CounterCoverageMappingBuilder
   GapRegionCounter = OutCount;
 }
 
-// Create Branch Region around condition.
-createBranchRegion(S->getCond(), ThenCount,
-   subtractCounters(ParentCount, ThenCount));
+if (!S->isConsteval()) {
+  // Create Branch Region around condition.
+  createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
+}
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {

diff  --git a/clang/test/CoverageMapping/if.cpp 
b/clang/test/CoverageMapping/if.cpp
index 5b705cd46ab72..de3554c100b96 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp 
%s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-c

[PATCH] D132723: [Clang] Fix crash in coverage of if consteval.

2022-08-26 Thread Corentin Jabot 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 rG463e30f51fd0: [Clang] Fix crash in coverage of if consteval. 
(authored by cor3ntin).

Changed prior to commit:
  https://reviews.llvm.org/D132723?vs=455917&id=455930#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132723

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp

Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++2b -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
 
 int nop() { return 0; }
 
@@ -13,6 +13,22 @@
 ++j;// CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
+
+// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
+constexpr int check_consteval(int i) {
+if consteval {
+  i++;
+}
+if !consteval {
+  i++;
+}
+if consteval {
+return 42;
+} else {
+return i;
+}
+}
+
 // CHECK-LABEL: main:
 int main() {// CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
   int i = 0;
@@ -50,6 +66,10 @@
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
   i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
+  // GH-57377
+  constexpr int c_i = check_consteval(0);
+  check_consteval(i);
+
   return 0;
 }
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1377,19 +1377,23 @@
 
 // Extend into the condition before we propagate through it below - this is
 // needed to handle macros that generate the "if" but not the condition.
-extendRegion(S->getCond());
+if (!S->isConsteval())
+  extendRegion(S->getCond());
 
 Counter ParentCount = getRegion().getCounter();
 Counter ThenCount = getRegionCounter(S);
 
-// Emitting a counter for the condition makes it easier to interpret the
-// counter for the body when looking at the coverage.
-propagateCounts(ParentCount, S->getCond());
+if (!S->isConsteval()) {
+  // Emitting a counter for the condition makes it easier to interpret the
+  // counter for the body when looking at the coverage.
+  propagateCounts(ParentCount, S->getCond());
 
-// The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
-if (Gap)
-  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+  // The 'then' count applies to the area immediately after the condition.
+  Optional Gap =
+  findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+  if (Gap)
+fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
+}
 
 extendRegion(S->getThen());
 Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1402,9 @@
 if (const Stmt *Else = S->getElse()) {
   bool ThenHasTerminateStmt = HasTerminateStmt;
   HasTerminateStmt = false;
-
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
+  Optional Gap =
+  findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);
@@ -1416,9 +1420,11 @@
   GapRegionCounter = OutCount;
 }
 
-// Create Branch Region around condition.
-createBranchRegion(S->getCond(), ThenCount,
-   subtractCounters(ParentCount, ThenCount));
+if (!S->isConsteval()) {
+  // Create Branch Region around condition.
+  createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
+}
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
Index: clang/docs/ReleaseNotes.rst
=

[PATCH] D132745: [clang] Fix ambiguous use of `report_fatal_error`.

2022-08-26 Thread weiyi via Phabricator via cfe-commits
wyt created this revision.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`report_fatal_error` is overloaded on `StringRef` and `Twine &`, therefore 
passing a `std::string` argument leads to ambiguity as it is convertible to 
either type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132745

Files:
  clang/lib/Basic/SanitizerSpecialCaseList.cpp


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(llvm::StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(llvm::StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

2022-08-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik updated this revision to Diff 455935.
shafik marked an inline comment as done.
shafik added a comment.

- Change to order of operations in assert to put off more costly check
- Add release note


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

https://reviews.llvm.org/D132712

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/member-expr.cpp


Index: clang/test/SemaCXX/member-expr.cpp
===
--- clang/test/SemaCXX/member-expr.cpp
+++ clang/test/SemaCXX/member-expr.cpp
@@ -228,3 +228,19 @@
 .i;  // expected-error {{member reference type 'S *' is a pointer; did 
you mean to use '->'}}
   }
 }
+
+namespace LookupTemplateNameAssert {
+void f() {}
+
+typedef int at[];
+const at& f2(){}
+
+void g() {
+  f().junk(); // expected-error {{member reference base type 'void' is 
not a structure or union}}
+// expected-error@-1 {{expected '(' for function-style cast or type 
construction}}
+// expected-error@-2 {{expected expression}}
+  f2().junk(); // expected-error {{member reference base type 'const at' 
(aka 'const int[]') is not a structure or union}}
+// expected-error@-1 {{expected '(' for function-style cast or type 
construction}}
+// expected-error@-2 {{expected expression}}
+}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -399,6 +399,7 @@
 LookupCtx = computeDeclContext(ObjectType);
 IsDependent = !LookupCtx && ObjectType->isDependentType();
 assert((IsDependent || !ObjectType->isIncompleteType() ||
+!ObjectType->getAs() ||
 ObjectType->castAs()->isBeingDefined()) &&
"Caller should have completed object type");
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -81,6 +81,9 @@
 - Fix a crash when generating code coverage information for an
   ``if consteval`` statement. This fixes
   `Issue 57377 `_.
+- Fix assert that triggers a crash during template name lookup when a type was
+  incomplete but was not also a TagType. This fixes
+  `Issue 57387 `_.
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaCXX/member-expr.cpp
===
--- clang/test/SemaCXX/member-expr.cpp
+++ clang/test/SemaCXX/member-expr.cpp
@@ -228,3 +228,19 @@
 .i;  // expected-error {{member reference type 'S *' is a pointer; did you mean to use '->'}}
   }
 }
+
+namespace LookupTemplateNameAssert {
+void f() {}
+
+typedef int at[];
+const at& f2(){}
+
+void g() {
+  f().junk(); // expected-error {{member reference base type 'void' is not a structure or union}}
+// expected-error@-1 {{expected '(' for function-style cast or type construction}}
+// expected-error@-2 {{expected expression}}
+  f2().junk(); // expected-error {{member reference base type 'const at' (aka 'const int[]') is not a structure or union}}
+// expected-error@-1 {{expected '(' for function-style cast or type construction}}
+// expected-error@-2 {{expected expression}}
+}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -399,6 +399,7 @@
 LookupCtx = computeDeclContext(ObjectType);
 IsDependent = !LookupCtx && ObjectType->isDependentType();
 assert((IsDependent || !ObjectType->isIncompleteType() ||
+!ObjectType->getAs() ||
 ObjectType->castAs()->isBeingDefined()) &&
"Caller should have completed object type");
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -81,6 +81,9 @@
 - Fix a crash when generating code coverage information for an
   ``if consteval`` statement. This fixes
   `Issue 57377 `_.
+- Fix assert that triggers a crash during template name lookup when a type was
+  incomplete but was not also a TagType. This fixes
+  `Issue 57387 `_.
 
 
 Improvements to Clang's diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] a0ecb4a - [HLSL] Move DXIL validation version out of ModuleFlags

2022-08-26 Thread Xiang Li via cfe-commits

Author: Xiang Li
Date: 2022-08-26T09:20:45-07:00
New Revision: a0ecb4a2991d6163c16c29bee8cd63720ee7d3d7

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

LOG: [HLSL] Move DXIL validation version out of ModuleFlags

Put DXIL validation version into separate NamedMetadata to avoid update 
ModuleFlags.

Currently DXIL validation version is saved in ModuleFlags in clang codeGen.
Then in DirectX backend, the data will be extracted from ModuleFlags and cause 
rebuild of ModuleFlags.
This patch will build NamedMetadata for DXIL validation version and remove the 
code to rebuild ModuleFlags.

Reviewed By: beanz

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

Added: 


Modified: 
clang/lib/CodeGen/CGHLSLRuntime.cpp
clang/test/CodeGenHLSL/validator_version.hlsl
llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
llvm/test/CodeGen/DirectX/dxil_ver.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index fa7bab9121b7a..9ca3b64996edd 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -42,16 +42,18 @@ void addDxilValVersion(StringRef ValVersionStr, 
llvm::Module &M) {
   IRBuilder<> B(M.getContext());
   MDNode *Val = MDNode::get(Ctx, {ConstantAsMetadata::get(B.getInt32(Major)),
   ConstantAsMetadata::get(B.getInt32(Minor))});
-  StringRef DxilValKey = "dx.valver";
-  M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, 
Val);
+  StringRef DXILValKey = "dx.valver";
+  auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
+  DXILValMD->addOperand(Val);
 }
 } // namespace
 
 void CGHLSLRuntime::finishCodeGen() {
   auto &TargetOpts = CGM.getTarget().getTargetOpts();
-
   llvm::Module &M = CGM.getModule();
-  addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
+  Triple T(M.getTargetTriple());
+  if (T.getArch() == Triple::ArchType::dxil)
+addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) 
{

diff  --git a/clang/test/CodeGenHLSL/validator_version.hlsl 
b/clang/test/CodeGenHLSL/validator_version.hlsl
index eee83bd9677be..426918a7221c4 100644
--- a/clang/test/CodeGenHLSL/validator_version.hlsl
+++ b/clang/test/CodeGenHLSL/validator_version.hlsl
@@ -1,8 +1,11 @@
 // RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -S -emit-llvm 
-xhlsl -validator-version 1.1 -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple spirv32 -S -emit-llvm -xhlsl -validator-version 
1.1 -o - %s | FileCheck %s --check-prefix=NOT_DXIL
 
-// CHECK:!"dx.valver", ![[valver:[0-9]+]]}
+// CHECK:!dx.valver = !{![[valver:[0-9]+]]}
 // CHECK:![[valver]] = !{i32 1, i32 1}
 
+// NOT_DXIL-NOT:!dx.valver
+
 float bar(float a, float b);
 
 float foo(float a, float b) {

diff  --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp 
b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index 634ead98a6ae7..85f1b0784cda3 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -58,32 +58,6 @@ static VersionTuple loadDXILValidatorVersion(MDNode 
*ValVerMD) {
   return VersionTuple(Major, Minor);
 }
 
-static void cleanModuleFlags(Module &M) {
-  constexpr StringLiteral DeadKeys[] = {ValVerKey};
-  // Collect DeadKeys in ModuleFlags.
-  StringSet<> DeadKeySet;
-  for (auto &Key : DeadKeys) {
-if (M.getModuleFlag(Key))
-  DeadKeySet.insert(Key);
-  }
-  if (DeadKeySet.empty())
-return;
-
-  SmallVector ModuleFlags;
-  M.getModuleFlagsMetadata(ModuleFlags);
-  NamedMDNode *MDFlags = M.getModuleFlagsMetadata();
-  MDFlags->eraseFromParent();
-  // Add ModuleFlag which not dead.
-  for (auto &Flag : ModuleFlags) {
-StringRef Key = Flag.Key->getString();
-if (DeadKeySet.contains(Key))
-  continue;
-M.addModuleFlag(Flag.Behavior, Key, Flag.Val);
-  }
-}
-
-static void cleanModule(Module &M) { cleanModuleFlags(M); }
-
 namespace {
 class DXILTranslateMetadata : public ModulePass {
 public:
@@ -101,13 +75,12 @@ class DXILTranslateMetadata : public ModulePass {
 } // namespace
 
 bool DXILTranslateMetadata::runOnModule(Module &M) {
-  if (MDNode *ValVerMD = cast_or_null(M.getModuleFlag(ValVerKey))) {
-auto ValVer = loadDXILValidatorVersion(ValVerMD);
+  if (NamedMDNode *ValVerMD = M.getNamedMetadata(ValVerKey)) {
+VersionTuple ValVer = loadDXILValidatorVersion(ValVerMD->getOperand(0));
 if (!ValVer.empty())
   ValidatorVer = ValVer;
   }
   emitDXILValidatorVersion(M, ValidatorVer);
-  cleanModule(M);
   return false;
 }
 

diff  --git a/llvm/test/CodeGen/DirectX/dxil_ver.ll 
b/llvm/test/CodeGen/DirectX/dxil_ver.ll
index dcc9b6a797f6a..e9923a3abce02 100644
--- a/l

[PATCH] D130207: [HLSL] Move DXIL validation version out of ModuleFlags

2022-08-26 Thread Xiang Li via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0ecb4a2991d: [HLSL] Move DXIL validation version out of 
ModuleFlags (authored by python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130207

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/test/CodeGenHLSL/validator_version.hlsl
  llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
  llvm/test/CodeGen/DirectX/dxil_ver.ll

Index: llvm/test/CodeGen/DirectX/dxil_ver.ll
===
--- llvm/test/CodeGen/DirectX/dxil_ver.ll
+++ llvm/test/CodeGen/DirectX/dxil_ver.ll
@@ -3,18 +3,18 @@
 target triple = "dxil-pc-shadermodel6.3-library"
 
 ; Make sure dx.valver metadata is generated.
-; CHECK:!dx.valver = !{![[valver:[0-9]+]]}
+; CHECK-DAG:!dx.valver = !{![[valver:[0-9]+]]}
 ; Make sure module flags still exist and only have 1 operand left.
-; CHECK:!llvm.module.flags = !{{{![0-9]}}}
+; CHECK-DAG:!llvm.module.flags = !{{{![0-9]}}}
 ; Make sure validator version is 1.1.
-; CHECK:![[valver]] = !{i32 1, i32 1}
+; CHECK-DAG:![[valver]] = !{i32 1, i32 1}
 ; Make sure wchar_size still exist.
-; CHECK:!{i32 1, !"wchar_size", i32 4}
+; CHECK-DAG:!{i32 1, !"wchar_size", i32 4}
 
-!llvm.module.flags = !{!0, !1}
-!llvm.ident = !{!3}
+!llvm.module.flags = !{!0}
+!dx.valver = !{!1}
+!llvm.ident = !{!2}
 
 !0 = !{i32 1, !"wchar_size", i32 4}
-!1 = !{i32 6, !"dx.valver", !2}
-!2 = !{i32 1, i32 1}
-!3 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project 71de12113a0661649ecb2f533fba4a2818a1ad68)"}
+!1 = !{i32 1, i32 1}
+!2 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project 71de12113a0661649ecb2f533fba4a2818a1ad68)"}
Index: llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
===
--- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -58,32 +58,6 @@
   return VersionTuple(Major, Minor);
 }
 
-static void cleanModuleFlags(Module &M) {
-  constexpr StringLiteral DeadKeys[] = {ValVerKey};
-  // Collect DeadKeys in ModuleFlags.
-  StringSet<> DeadKeySet;
-  for (auto &Key : DeadKeys) {
-if (M.getModuleFlag(Key))
-  DeadKeySet.insert(Key);
-  }
-  if (DeadKeySet.empty())
-return;
-
-  SmallVector ModuleFlags;
-  M.getModuleFlagsMetadata(ModuleFlags);
-  NamedMDNode *MDFlags = M.getModuleFlagsMetadata();
-  MDFlags->eraseFromParent();
-  // Add ModuleFlag which not dead.
-  for (auto &Flag : ModuleFlags) {
-StringRef Key = Flag.Key->getString();
-if (DeadKeySet.contains(Key))
-  continue;
-M.addModuleFlag(Flag.Behavior, Key, Flag.Val);
-  }
-}
-
-static void cleanModule(Module &M) { cleanModuleFlags(M); }
-
 namespace {
 class DXILTranslateMetadata : public ModulePass {
 public:
@@ -101,13 +75,12 @@
 } // namespace
 
 bool DXILTranslateMetadata::runOnModule(Module &M) {
-  if (MDNode *ValVerMD = cast_or_null(M.getModuleFlag(ValVerKey))) {
-auto ValVer = loadDXILValidatorVersion(ValVerMD);
+  if (NamedMDNode *ValVerMD = M.getNamedMetadata(ValVerKey)) {
+VersionTuple ValVer = loadDXILValidatorVersion(ValVerMD->getOperand(0));
 if (!ValVer.empty())
   ValidatorVer = ValVer;
   }
   emitDXILValidatorVersion(M, ValidatorVer);
-  cleanModule(M);
   return false;
 }
 
Index: clang/test/CodeGenHLSL/validator_version.hlsl
===
--- clang/test/CodeGenHLSL/validator_version.hlsl
+++ clang/test/CodeGenHLSL/validator_version.hlsl
@@ -1,8 +1,11 @@
 // RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -S -emit-llvm -xhlsl -validator-version 1.1 -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple spirv32 -S -emit-llvm -xhlsl -validator-version 1.1 -o - %s | FileCheck %s --check-prefix=NOT_DXIL
 
-// CHECK:!"dx.valver", ![[valver:[0-9]+]]}
+// CHECK:!dx.valver = !{![[valver:[0-9]+]]}
 // CHECK:![[valver]] = !{i32 1, i32 1}
 
+// NOT_DXIL-NOT:!dx.valver
+
 float bar(float a, float b);
 
 float foo(float a, float b) {
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -42,16 +42,18 @@
   IRBuilder<> B(M.getContext());
   MDNode *Val = MDNode::get(Ctx, {ConstantAsMetadata::get(B.getInt32(Major)),
   ConstantAsMetadata::get(B.getInt32(Minor))});
-  StringRef DxilValKey = "dx.valver";
-  M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val);
+  StringRef DXILValKey = "dx.valver";
+  auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
+  DXILValMD->addOperand(Val);
 }
 } // namespace
 
 void CGHLSLRuntime::finishCodeGen() {
   auto &TargetOpts = CGM.getTarget().getTargetOpts();
-
   llvm::Module &M = CGM.getModule();
-  addDxilValVersion(TargetOpts.DxilValidato

[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

2022-08-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.

LGTM.


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

https://reviews.llvm.org/D132712

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


  1   2   >