[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626
+  VisitDeclaratorDecl(D);
+  Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName());
+  Record.push_back(D->getIdentifierNamespace());
+

mizvekov wrote:
> ChuanqiXu wrote:
> > ChuanqiXu wrote:
> > > ChuanqiXu wrote:
> > > > mizvekov wrote:
> > > > > ChuanqiXu wrote:
> > > > > > I still don't get the reason for the move. What's the benefit? Or 
> > > > > > why is it necessary?
> > > > > Yeah, now the type can reference the template decl, so without moving 
> > > > > this, it can happen during import of the type that we try to read 
> > > > > this function template bits without having imported them yet.
> > > > Oh, I guess I met the problem before (D129748 ) and I made a workaround 
> > > > for it (https://reviews.llvm.org/D130331). If I understood right, the 
> > > > patch will solve that problem. I'll check it out later.
> > > > 
> > > > (This kind of code move looks dangerous you know and I'll take a double 
> > > > check)
> > > After looking into the detailed change for the serialization part, I feel 
> > > it is a not-so-good workaround indeed.. It looks like we need a better 
> > > method to delay reading the type in the serializer. And I'm looking at 
> > > it. @mizvekov would you like to rebase the series of patches to the main 
> > > branch so that I can test it actually.
> > Or would it be simpler to rebase and squash them into a draft revision?
> I had given this some thought, and it made sense to me that we should deal 
> with the template bits first, since these are closer to the introducer for 
> these declarations, and so that it would be harder to have a dependence the 
> other way around.
> 
> But I would like to hear your thoughts on this after you have taken a better 
> look.
> I am working on a bunch of things right now, I should be able to rebase this 
> on the next few days, but otherwise
> I last rebased about 4 days ago, so you can also check that out at 
> https://github.com/mizvekov/llvm-project/tree/resugar
> That link has the whole stack, you probably should check out just the commit 
> for this patch, as you are probably going to encounter issues with the 
> resugarer if you try it on substantial code bases.
> It will carry other changes with it, but I think those should be safe.
I won't say it is bad to deal with template bits first. I just feel it is a 
workaround to avoid the circular dependent problem in deserialization. Or in 
another word, here the method works due to you put some decls* in the template 
parameter types. And we avoid the circular dependent problem by adjusting the 
order we deserializes. The reasons why I don't feel it is good include:
(1) Although we touched template function decl and template var decl, we don't 
touched template var decl. I guess it'll be a problem now or later.
(2) The solution here can't solve the similar circular dependent problem I 
sawed in attributes. So the method only workarounds some resulting of the same 
problem.

Or in one shorter explanation, it should be greater to solve the root problems. 
I have an idea and I am going to to do a proof-of-concept implementation first 
since I feel like nobody are happy about an unimplementable idea. Generally I 
don't like to block patches due to such reasons since it is completely not your 
fault but I guess it may be better to wait some time. Since if we want to allow 
workarounds first and clear the workarounds, things will be harder. If you want 
a timeline, I guess 2 months may be reasonable choices. I mean if I can't make 
it in 2 months and other reviewers feel this is good (what I am seeing), I feel 
bad to block this. (But if we're more patient, it'll be better). How do you 
think about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[clang] 7dc0734 - [msan] Insert simplification passes after instrumentation

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2022-09-09T00:33:04-07:00
New Revision: 7dc0734567217236a61fa2ed3b3909ac25925ce5

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

LOG: [msan] Insert simplification passes after instrumentation

This resolves TODO from D96406.
InstCombine issue is fixed with D133394.

Save 4.5% of .text on CTMark.

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 64b21ef608bb1..f4a4f1cd22ab9 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
@@ -72,14 +73,16 @@
 #include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemProfiler.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
-#include "llvm/Transforms/Instrumentation/SanitizerCoverage.h"
 #include "llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h"
+#include "llvm/Transforms/Instrumentation/SanitizerCoverage.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Scalar/EarlyCSE.h"
 #include "llvm/Transforms/Scalar/GVN.h"
+#include "llvm/Transforms/Scalar/JumpThreading.h"
 #include "llvm/Transforms/Scalar/LowerMatrixIntrinsics.h"
+#include "llvm/Transforms/Scalar/NewGVN.h"
 #include "llvm/Transforms/Utils.h"
 #include "llvm/Transforms/Utils/CanonicalizeAliases.h"
 #include "llvm/Transforms/Utils/Debugify.h"
@@ -97,6 +100,7 @@ using namespace llvm;
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt RunNewGVN;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -661,16 +665,21 @@ static void addSanitizers(const Triple &TargetTriple,
CodeGenOpts.SanitizeMemoryParamRetval);
 MPM.addPass(MemorySanitizerPass(options));
 if (Level != OptimizationLevel::O0) {
-  // MemorySanitizer inserts complex instrumentation that mostly
-  // follows the logic of the original code, but operates on
-  // "shadow" values. It can benefit from re-running some
-  // general purpose optimization passes.
-  MPM.addPass(createModuleToFunctionPassAdaptor(EarlyCSEPass()));
-  // TODO: Consider add more passes like in
-  // addGeneralOptsForMemorySanitizer. EarlyCSEPass makes visible
-  // 
diff erence on size. It's not clear if the rest is still
-  // usefull. InstCombinePass breakes
-  // compiler-rt/test/msan/select_origin.cpp.
+  // MemorySanitizer inserts complex instrumentation that mostly 
follows
+  // the logic of the original code, but operates on "shadow" values. 
It
+  // can benefit from re-running some general purpose optimization
+  // passes.
+  MPM.addPass(RecomputeGlobalsAAPass());
+  FunctionPassManager FPM;
+  FPM.addPass(EarlyCSEPass(true /* Enable mem-ssa. */));
+  FPM.addPass(InstCombinePass());
+  FPM.addPass(JumpThreadingPass());
+  if (RunNewGVN)
+FPM.addPass(NewGVNPass());
+  else
+FPM.addPass(GVNPass());
+  FPM.addPass(InstCombinePass());
+  MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
 }
   }
 };



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

[PATCH] D133555: [pseudo] Eliminate a false parse of the template parameter "class T".

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

Implements the temp.param#2  rule with 
the guard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133555

Files:
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/type-template-parameter.cpp


Index: clang-tools-extra/pseudo/test/cxx/type-template-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/type-template-parameter.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't misparse the "class abc" as an unnamed non-type 
template-parameter of class T.
+template
+struct Foo {};
+
+// CHECK:  template-parameter-list~type-parameter := type-parameter-key 
IDENTIFIER
+// CHECK-NEXT: ├─type-parameter-key~CLASS :=
+// CHECK-NEXT: └─IDENTIFIER :=
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -663,7 +663,7 @@
 constraint-logical-and-expression := primary-expression
 constraint-logical-and-expression := constraint-logical-and-expression && 
primary-expression
 template-parameter := type-parameter
-template-parameter := parameter-declaration
+template-parameter := parameter-declaration [guard]
 type-parameter := type-parameter-key ..._opt IDENTIFIER_opt
 type-parameter := type-parameter-key IDENTIFIER_opt = type-id
 type-parameter := type-constraint ..._opt IDENTIFIER_opt
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -13,6 +13,7 @@
 #include "clang-pseudo/grammar/LRTable.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Debug.h"
 #include 
@@ -182,6 +183,25 @@
   return true;
 }
 
+bool isNotClassIdentifierFormTemplateParameter(
+const GuardParams &TemplateParameter) {
+  const ForestNode *ParameterDeclaration = TemplateParameter.RHS[0];
+  assert(ParameterDeclaration->symbol() == cxx::Symbol::parameter_declaration);
+  llvm::SmallVector Tokens;
+  // FIXME: This is an awkward way to get the end of the covered token range,
+  // pass it in the GuardParams?
+  Token::Index Start = ParameterDeclaration->startTokenIndex();
+  if (TemplateParameter.Tokens.tokens()[Start].Kind != clang::tok::kw_class)
+return true;
+  Token::Index Last = Start;
+  for (auto &Child : ParameterDeclaration->descendants()) {
+if (Child.kind() == ForestNode::Terminal)
+  Last = std::max(Last, Child.startTokenIndex());
+  }
+  return !(Last - Start == 1 && TemplateParameter.Tokens.tokens()[Last].Kind ==
+clang::tok::identifier);
+}
+
 // Whether this e.g. decl-specifier contains an "exclusive" type such as a 
class
 // name, and thus can't combine with a second exclusive type.
 //
@@ -353,6 +373,17 @@

decl_specifier_seq__L_SQUARE__identifier_list__R_SQUARE__initializer__SEMI,
specifiesStructuredBinding},
 
+  // C++ [temp.param#2]:
+  //   ...A template-parameter of the form class identifier is a 
type-parameter.
+  //
+  //   template void f(T t);
+  // Here, the template f has a type-parameter called T, rather than an
+  // unnamed non-type template-parameter of class T.
+  {
+rule::template_parameter::parameter_declaration,
+isNotClassIdentifierFormTemplateParameter,
+  },
+
   // The grammar distinguishes (only) user-defined vs plain string 
literals,
   // where the clang lexer distinguishes (only) encoding types.
   {rule::user_defined_string_literal_chunk::STRING_LITERAL,


Index: clang-tools-extra/pseudo/test/cxx/type-template-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/type-template-parameter.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't misparse the "class abc" as an unnamed non-type template-parameter of class T.
+template
+struct Foo {};
+
+// CHECK:  template-parameter-list~type-parameter := type-parameter-key IDENTIFIER
+// CHECK-NEXT: ├─type-parameter-key~CLASS :=
+// CHECK-NEXT: └─IDENTIFIER :=
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/c

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  struct Style {
enum E {
  Yes,
  No,
};
unsigned OverEmptyLines;
  }

I don't understand the need for `state` as a struct could have multiple options 
(as enums) each enum should have a state that means "Leave"


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] D133565: [pseudo] Print a random text code for each ambiguous symbol in --print-statistics

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

I found it useful when diagnosing/investigating ambiguous cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133565

Files:
  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
@@ -18,6 +18,7 @@
 #include "clang-pseudo/grammar/LRGraph.h"
 #include "clang-pseudo/grammar/LRTable.h"
 #include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/CommandLine.h"
@@ -185,14 +186,32 @@
   llvm::outs() << "GSS bytes: " << GSS.bytes()
<< " nodes: " << GSS.nodesCreated() << "\n";
 
+  llvm::DenseMap AmbiguousSamples;
   for (auto &P : {std::make_pair("Ambiguous", ForestNode::Ambiguous),
   std::make_pair("Opaque", ForestNode::Opaque)}) {
 clang::pseudo::NodeStats Stats(
-Root, [&](const auto &N) { return N.kind() == P.second; });
+Root, [&](const clang::pseudo::ForestNode &N) {
+  if (N.kind() == ForestNode::Ambiguous) {
+if (AmbiguousSamples.count(N.symbol()) == 0) {
+  Token::Index Start, Last;
+  Start = Last = N.startTokenIndex();
+  for (auto &It : N.descendants()) {
+if (It.kind() == ForestNode::Terminal)
+  Last = std::max(Last, It.startTokenIndex());
+  }
+  std::string TextCode;
+  for (const auto &T :
+   ParseableStream->tokens().slice(Start, Last - Start + 
1))
+TextCode += T.text();
+  AmbiguousSamples.insert({N.symbol(), TextCode});
+}
+  }
+  return N.kind() == P.second;
+});
 llvm::outs() << "\n" << Stats.Total << " " << P.first << " nodes:\n";
 for (const auto &S : Stats.BySymbol)
-  llvm::outs() << llvm::formatv("  {0,3} {1}\n", S.second,
-Lang.G.symbolName(S.first));
+  llvm::outs() << llvm::formatv("  {0,3} {1}, sample code: {2}\n", 
S.second,
+Lang.G.symbolName(S.first), 
AmbiguousSamples[S.first]);
   }
 
   // Metrics for how imprecise parsing was.


Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -18,6 +18,7 @@
 #include "clang-pseudo/grammar/LRGraph.h"
 #include "clang-pseudo/grammar/LRTable.h"
 #include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/CommandLine.h"
@@ -185,14 +186,32 @@
   llvm::outs() << "GSS bytes: " << GSS.bytes()
<< " nodes: " << GSS.nodesCreated() << "\n";
 
+  llvm::DenseMap AmbiguousSamples;
   for (auto &P : {std::make_pair("Ambiguous", ForestNode::Ambiguous),
   std::make_pair("Opaque", ForestNode::Opaque)}) {
 clang::pseudo::NodeStats Stats(
-Root, [&](const auto &N) { return N.kind() == P.second; });
+Root, [&](const clang::pseudo::ForestNode &N) {
+  if (N.kind() == ForestNode::Ambiguous) {
+if (AmbiguousSamples.count(N.symbol()) == 0) {
+  Token::Index Start, Last;
+  Start = Last = N.startTokenIndex();
+  for (auto &It : N.descendants()) {
+if (It.kind() == ForestNode::Terminal)
+  Last = std::max(Last, It.startTokenIndex());
+  }
+  std::string TextCode;
+  for (const auto &T :
+   ParseableStream->tokens().slice(Start, Last - Start + 1))
+TextCode += T.text();
+  AmbiguousSamples.insert({N.symbol(), TextCode});
+}
+  }
+  return N.kind() == P.second;
+});
 llvm::outs() << "\n" << Stats.Total << " " << P.first << " nodes:\n";
 for (const auto &S : Stats.BySymbol)
-  llvm::outs() << llvm::formatv("  {0,3} {1}\n", S.second,
-Lang.G.symbolName(S.first));
+  llvm::outs() << llvm::formatv("  {0,3} {1}, sample code: {2}\n", S.second,
+Lang.G.symbolName(S.first), AmbiguousSamples[S.first]);
   }

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> But you could set MaxEmptyLinesToKeep to 3 and aligning comments to over 2 
> empty lines.

Correct! lets assume MaxEmptyLinesToKeep  = 3

if this case is valid in that case

  int a;// Foo
  int longer foo;   // Foo
  int verylong foo; // Foo

then so is this

  int a;// Foo
  
  int longer foo;   // Foo
  
  int verylong foo; // Foo

and so is this...

  int a;// Foo
  
  
  int longer foo;   // Foo
  
  
  int verylong foo; // Foo

but not this

  int a;  // Foo
  
  int longer foo; // Foo
  
  
   
  int verylong foo; // Foo

Regardless of how many lines I am willing or not willing to keep, I know it 
feels orthogonal, but its actually independent (so don't use it as my setting), 
or you could manipulate code in a way I don't want changed (and they people 
will complain)

Its one of those cases there it feels like they can be the same, but the answer 
is should they? or are we making an assumption about what people want rather 
than giving them the control.

I agree if OverEmptyLines > MaxEmptyLinesToKeep  then it feels kinda odd (but 
what about lines that are // clang-format off'd should we count those?


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

[clang] 9424497 - [Clang] Use virtual FS in processing config files

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2022-09-09T16:28:51+07:00
New Revision: 9424497e43aff088e014d65fd952ec557e28e6cf

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

LOG: [Clang] Use virtual FS in processing config files

Clang has support of virtual file system for the purpose of testing, but
treatment of config files did not use it. This change enables VFS in it
as well.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/Driver.cpp
clang/unittests/Driver/ToolChainTest.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 155eababa81e6..4804856bc8f5c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -93,6 +93,8 @@ Bug Fixes
   `Issue 57516 `_
 - Fix ``__builtin_assume_aligned`` crash when the 1st arg is array type. This 
fixes
   `Issue 57169 `_
+- Clang configuration files are now read through the virtual file system
+  rather than the physical one, if these are 
diff erent.
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ca8e0e5240e1d..217236f459a50 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -911,7 +911,8 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation 
&C,
 /// by Dirs.
 ///
 static bool searchForFile(SmallVectorImpl &FilePath,
-  ArrayRef Dirs, StringRef FileName) {
+  ArrayRef Dirs, StringRef FileName,
+  llvm::vfs::FileSystem &FS) {
   SmallString<128> WPath;
   for (const StringRef &Dir : Dirs) {
 if (Dir.empty())
@@ -919,7 +920,8 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 WPath.clear();
 llvm::sys::path::append(WPath, Dir, FileName);
 llvm::sys::path::native(WPath);
-if (llvm::sys::fs::is_regular_file(WPath)) {
+auto Status = FS.status(WPath);
+if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) 
{
   FilePath = std::move(WPath);
   return true;
 }
@@ -930,7 +932,7 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 bool Driver::readConfigFile(StringRef FileName) {
   // Try reading the given file.
   SmallVector NewCfgArgs;
-  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs)) {
+  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs, getVFS())) {
 Diag(diag::err_drv_cannot_read_config_file) << FileName;
 return true;
   }
@@ -970,7 +972,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_system_dir_EQ));
-  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
+  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
 SystemConfigDir.clear();
   else
 SystemConfigDir = static_cast(CfgDir);
@@ -979,7 +981,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_user_dir_EQ));
-  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
+  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
 UserConfigDir.clear();
   else
 UserConfigDir = static_cast(CfgDir);
@@ -1004,13 +1006,16 @@ bool Driver::loadConfigFile() {
   // If argument contains directory separator, treat it as a path to
   // configuration file.
   if (llvm::sys::path::has_parent_path(CfgFileName)) {
-SmallString<128> CfgFilePath;
-if (llvm::sys::path::is_relative(CfgFileName))
-  llvm::sys::fs::current_path(CfgFilePath);
-llvm::sys::path::append(CfgFilePath, CfgFileName);
-if (!llvm::sys::fs::is_regular_file(CfgFilePath)) {
-  Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
-  return true;
+SmallString<128> CfgFilePath(CfgFileName);
+if (llvm::sys::path::is_relative(CfgFilePath)) {
+  if (getVFS().makeAbsolute(CfgFilePath))
+return true;
+  auto Status = getVFS().status(CfgFilePath);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::regular_file) {
+Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+return true;
+  }
 }
 return readConfigFile(CfgFilePath);
   }
@@ -1069,17 +1074,19 @@ bool Driver::loadConfigFile() {
   // Try to find config file. First try file with corrected architecture.
   llvm::SmallString<128> CfgFilePath;
   

[PATCH] D132867: [Clang] Use virtual FS in processing config files

[Serge Pavlov 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 rG9424497e43af: [Clang] Use virtual FS in processing config 
files (authored by sepavloff).

Changed prior to commit:
  https://reviews.llvm.org/D132867?vs=458205&id=458993#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132867

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1264,10 +1264,17 @@
 // always have an absolute path deduced from the containing file.
 SmallString<128> CurrDir;
 if (llvm::sys::path::is_relative(FName)) {
-  if (!CurrentDir)
-llvm::sys::fs::current_path(CurrDir);
-  else
+  if (!CurrentDir) {
+if (auto CWD = FS.getCurrentWorkingDirectory()) {
+  CurrDir = *CWD;
+} else {
+  // TODO: The error should be propagated up the stack.
+  llvm::consumeError(llvm::errorCodeToError(CWD.getError()));
+  return false;
+}
+  } else {
 CurrDir = *CurrentDir;
+  }
   llvm::sys::path::append(CurrDir, FName);
   FName = CurrDir.c_str();
 }
@@ -1357,24 +1364,26 @@
 }
 
 bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
-SmallVectorImpl &Argv) {
+SmallVectorImpl &Argv,
+llvm::vfs::FileSystem &FS) {
   SmallString<128> AbsPath;
   if (sys::path::is_relative(CfgFile)) {
-llvm::sys::fs::current_path(AbsPath);
-llvm::sys::path::append(AbsPath, CfgFile);
+AbsPath.assign(CfgFile);
+if (std::error_code EC = FS.makeAbsolute(AbsPath))
+  return false;
 CfgFile = AbsPath.str();
   }
-  if (llvm::Error Err = ExpandResponseFile(
-  CfgFile, Saver, cl::tokenizeConfigFile, Argv,
-  /*MarkEOLs=*/false, /*RelativeNames=*/true, /*ExpandBasePath=*/true,
-  *llvm::vfs::getRealFileSystem())) {
+  if (llvm::Error Err =
+  ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
+ /*MarkEOLs=*/false, /*RelativeNames=*/true,
+ /*ExpandBasePath=*/true, FS)) {
 // TODO: The error should be propagated up the stack.
 llvm::consumeError(std::move(Err));
 return false;
   }
   return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv,
  /*MarkEOLs=*/false, /*RelativeNames=*/true,
- /*ExpandBasePath=*/true, llvm::None);
+ /*ExpandBasePath=*/true, llvm::None, FS);
 }
 
 static void initCommonOptions();
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2078,7 +2078,8 @@
 /// current config file.
 ///
 bool readConfigFile(StringRef CfgFileName, StringSaver &Saver,
-SmallVectorImpl &Argv);
+SmallVectorImpl &Argv,
+llvm::vfs::FileSystem &FS);
 
 /// Expand response files on a command line recursively using the given
 /// StringSaver and tokenization strategy.  Argv should contain the command line
@@ -2099,7 +2100,7 @@
 /// the current response file.
 /// \param [in] FS File system used for all file access when running the tool.
 /// \param [in] CurrentDir Path used to resolve relative rsp files. If set to
-/// None, process' cwd is used instead.
+/// None, the file system current directory is used instead.
 /// \return true if all @files were expanded successfully or there were none.
 bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
  SmallVectorImpl &Argv, bool MarkEOLs,
Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -484,4 +484,61 @@
   }
 }
 
+TEST(ToolChainTest, ConfigFileSearch) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr FS(
+  new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+  const char *TestRoot = "C:\\";
+#else
+  const char *TestRoot = "/";
+#endif
+  FS->setCurrentWorkingDirectory(TestRoot);
+
+  FS->addFile(
+  "/opt/sdk/root.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/opt/sdk/platf

[clang] 55e1441 - Revert "[Clang] Use virtual FS in processing config files"

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2022-09-09T16:43:15+07:00
New Revision: 55e1441f7b5d01a37fc46eb1711f03ee69845316

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

LOG: Revert "[Clang] Use virtual FS in processing config files"

This reverts commit 9424497e43aff088e014d65fd952ec557e28e6cf.
Some buildbots failed, reverted for investigation.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/Driver.cpp
clang/unittests/Driver/ToolChainTest.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4804856bc8f5c..155eababa81e6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -93,8 +93,6 @@ Bug Fixes
   `Issue 57516 `_
 - Fix ``__builtin_assume_aligned`` crash when the 1st arg is array type. This 
fixes
   `Issue 57169 `_
-- Clang configuration files are now read through the virtual file system
-  rather than the physical one, if these are 
diff erent.
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 217236f459a50..ca8e0e5240e1d 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -911,8 +911,7 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation 
&C,
 /// by Dirs.
 ///
 static bool searchForFile(SmallVectorImpl &FilePath,
-  ArrayRef Dirs, StringRef FileName,
-  llvm::vfs::FileSystem &FS) {
+  ArrayRef Dirs, StringRef FileName) {
   SmallString<128> WPath;
   for (const StringRef &Dir : Dirs) {
 if (Dir.empty())
@@ -920,8 +919,7 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 WPath.clear();
 llvm::sys::path::append(WPath, Dir, FileName);
 llvm::sys::path::native(WPath);
-auto Status = FS.status(WPath);
-if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) 
{
+if (llvm::sys::fs::is_regular_file(WPath)) {
   FilePath = std::move(WPath);
   return true;
 }
@@ -932,7 +930,7 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 bool Driver::readConfigFile(StringRef FileName) {
   // Try reading the given file.
   SmallVector NewCfgArgs;
-  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs, getVFS())) {
+  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs)) {
 Diag(diag::err_drv_cannot_read_config_file) << FileName;
 return true;
   }
@@ -972,7 +970,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_system_dir_EQ));
-  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
+  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
 SystemConfigDir.clear();
   else
 SystemConfigDir = static_cast(CfgDir);
@@ -981,7 +979,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_user_dir_EQ));
-  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
+  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
 UserConfigDir.clear();
   else
 UserConfigDir = static_cast(CfgDir);
@@ -1006,16 +1004,13 @@ bool Driver::loadConfigFile() {
   // If argument contains directory separator, treat it as a path to
   // configuration file.
   if (llvm::sys::path::has_parent_path(CfgFileName)) {
-SmallString<128> CfgFilePath(CfgFileName);
-if (llvm::sys::path::is_relative(CfgFilePath)) {
-  if (getVFS().makeAbsolute(CfgFilePath))
-return true;
-  auto Status = getVFS().status(CfgFilePath);
-  if (!Status ||
-  Status->getType() != llvm::sys::fs::file_type::regular_file) {
-Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
-return true;
-  }
+SmallString<128> CfgFilePath;
+if (llvm::sys::path::is_relative(CfgFileName))
+  llvm::sys::fs::current_path(CfgFilePath);
+llvm::sys::path::append(CfgFilePath, CfgFileName);
+if (!llvm::sys::fs::is_regular_file(CfgFilePath)) {
+  Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+  return true;
 }
 return readConfigFile(CfgFilePath);
   }
@@ -1074,19 +1069,17 @@ bool Driver::loadConfigFile() {
   // Try to find config file. First try file with corrected architecture.
   llvm::SmallString<128> CfgFilePath;
   if (!FixedConfigFile.empty()) {
-if (searchForFile(CfgFilePath, CfgFileSearchDirs, Fi

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 459004.
t.p.northover marked an inline comment as done.
t.p.northover added a comment.

Switched to `enum class`.

> You may want to split the patch, with refactoring as the first, and the 
> Mach-O specific change as the second one.

I've got it split into two commits locally now. It's pretty much as you'd 
expect: first one NFC with no test changes, the second adds the test change and 
changes the return statement in `MachO::getDefaultUnwindTableLevel` from an 
unconditional `Asynchronous`.


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

https://reviews.llvm.org/D131153

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/CrossWindows.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/lib/Driver/ToolChains/NetBSD.h
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/test/Driver/clang-translation.c

Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -77,7 +77,11 @@
 
 // RUN: %clang -target arm64-apple-ios10 -### -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-APPLE %s
-// ARM64-APPLE: -funwind-tables=2
+// ARM64-APPLE: -funwind-tables=1
+
+// RUN: %clang -target arm64-apple-ios10 -funwind-tables -### -S %s -arch arm64 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM64-APPLE-UNWIND %s
+// ARM64-APPLE-UNWIND: -funwind-tables=1
 
 // RUN: %clang -target arm64-apple-ios10 -### -ffreestanding -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-FREESTANDING-APPLE %s
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -82,7 +82,8 @@
   std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
 FileType Type = ToolChain::FT_Static) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override;
 
   LangOptions::StackProtectorMode
   GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -363,11 +363,12 @@
 
 bool OpenBSD::HasNativeLLVMSupport() const { return true; }
 
-bool OpenBSD::IsUnwindTablesDefault(const ArgList &Args) const {
-switch (getArch()) {
-  case llvm::Triple::arm:
-return false;
-  default:
-return true;
-}
+ToolChain::UnwindTableLevel
+OpenBSD::getDefaultUnwindTableLevel(const ArgList &Args) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+return UnwindTableLevel::None;
+  default:
+return UnwindTableLevel::Asynchronous;
+  }
 }
Index: clang/lib/Driver/ToolChains/NetBSD.h
===
--- clang/lib/Driver/ToolChains/NetBSD.h
+++ clang/lib/Driver/ToolChains/NetBSD.h
@@ -65,8 +65,9 @@
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
-return true;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override {
+return UnwindTableLevel::Asynchronous;
   }
 
   llvm::ExceptionHandling GetExceptionModel(
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -66,7 +66,8 @@
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
   bool isPIEDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefaultForced() const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: aaron.ballman.
ilya-biryukov added a comment.

In D133029#3766058 , @shafik wrote:

> This old cfe-dev thread might be relevant: 
> https://lists.llvm.org/pipermail/cfe-dev/2018-June/058338.html

Thanks, the thread is definitely relevant.

> It appears that folks take advantage of this working a lot. It sounds like 
> your saying w/ constexpr std::vector this would be ill-formed in a constant 
> expression context now. Which won't effect most of the existing code I think?

Most of the code is not affected, but some code is and large codebases will 
definitely see quite some breakages.
Note that this warning diagnoses is very pedantic in terms of implementing the 
rules from the standard. It fires in many places than don't actually break in 
libc++ and C++20 mode. On the flip side, it is really easy to implement.

In D133029#3778230 , @dblaikie wrote:

> any chance of generalizing this beyond a special case for `std::vector` - an 
> attribute we could add to any class template (perhaps particular template 
> parameters to the class template) to document the requirement? (is 
> std::vector the only type in the standard library that has this property, or 
> only the one people trip over most often)

It should be easy to generalize, e.g. we could introduce an attribute to mark 
classes that want this behavior, something along the lines of:

  // Option 1. Less general, easy, small overhead.
  template  struct [[member_access_requires_complete(T)]] my_array {};
  // Option 2. More general, might be high-overhead depending on the actual 
expression.
  template  struct [[member_access_requires(sizeof (T))]] my_array {}; 
// checks for validity of expression on every member access?

I am not sure this carries it weight for that particular use-case and there are 
other things to consider, e.g. how to report error messages.
What is definitely useful and cheap is adding these warnings for more STL 
containers that have this requirement.

We discussed this with @aaron.ballman in the last Clang C++ Language WG meeting 
and one idea that popped up is implementing a warning in Clang, but never 
actually surfacing it in the compiler, only in `clang-tidy`. I would be very 
comfortable making a `clang-tidy` warning like this as the bar is much lower 
than the compiler warnings.
My proposal would be to allow making warnings that are disabled by default and 
even with `-Weverything`, the only way to enable them would be to pass 
`-W` explicitly or run a corresponding `clang-tidy` check. 
Alternatively, there will be no way to enable the warning in the compiler at 
all and the clang-tidy would be the only way to surface it.

Obviously, these use-cases should be rare as most things can be implemented 
directly in `clang-tidy`, but in that particular case we need access to the 
compiler state in the middle of processing the TU, something that `clang-tidy` 
APIs do not provide. We should also be careful to make sure any warnings like 
this do not hurt performance of the compiler.

@aaron.ballman does that look reasonable? I am about to try prototyping this. 
Are there any open questions we might want to agree on beforehand?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133029

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

[clang] 7b9fae0 - [Clang] Use virtual FS in processing config files

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2022-09-09T18:24:45+07:00
New Revision: 7b9fae05b4d0d3184ffc340e90d06a75e3cba2de

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

LOG: [Clang] Use virtual FS in processing config files

Clang has support of virtual file system for the purpose of testing, but
treatment of config files did not use it. This change enables VFS in it
as well.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/Driver.cpp
clang/unittests/Driver/ToolChainTest.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineTest.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 155eababa81e6..4804856bc8f5c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -93,6 +93,8 @@ Bug Fixes
   `Issue 57516 `_
 - Fix ``__builtin_assume_aligned`` crash when the 1st arg is array type. This 
fixes
   `Issue 57169 `_
+- Clang configuration files are now read through the virtual file system
+  rather than the physical one, if these are 
diff erent.
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ca8e0e5240e1d..217236f459a50 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -911,7 +911,8 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation 
&C,
 /// by Dirs.
 ///
 static bool searchForFile(SmallVectorImpl &FilePath,
-  ArrayRef Dirs, StringRef FileName) {
+  ArrayRef Dirs, StringRef FileName,
+  llvm::vfs::FileSystem &FS) {
   SmallString<128> WPath;
   for (const StringRef &Dir : Dirs) {
 if (Dir.empty())
@@ -919,7 +920,8 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 WPath.clear();
 llvm::sys::path::append(WPath, Dir, FileName);
 llvm::sys::path::native(WPath);
-if (llvm::sys::fs::is_regular_file(WPath)) {
+auto Status = FS.status(WPath);
+if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) 
{
   FilePath = std::move(WPath);
   return true;
 }
@@ -930,7 +932,7 @@ static bool searchForFile(SmallVectorImpl &FilePath,
 bool Driver::readConfigFile(StringRef FileName) {
   // Try reading the given file.
   SmallVector NewCfgArgs;
-  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs)) {
+  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs, getVFS())) {
 Diag(diag::err_drv_cannot_read_config_file) << FileName;
 return true;
   }
@@ -970,7 +972,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_system_dir_EQ));
-  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
+  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
 SystemConfigDir.clear();
   else
 SystemConfigDir = static_cast(CfgDir);
@@ -979,7 +981,7 @@ bool Driver::loadConfigFile() {
   SmallString<128> CfgDir;
   CfgDir.append(
   CLOptions->getLastArgValue(options::OPT_config_user_dir_EQ));
-  if (CfgDir.empty() || llvm::sys::fs::make_absolute(CfgDir))
+  if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
 UserConfigDir.clear();
   else
 UserConfigDir = static_cast(CfgDir);
@@ -1004,13 +1006,16 @@ bool Driver::loadConfigFile() {
   // If argument contains directory separator, treat it as a path to
   // configuration file.
   if (llvm::sys::path::has_parent_path(CfgFileName)) {
-SmallString<128> CfgFilePath;
-if (llvm::sys::path::is_relative(CfgFileName))
-  llvm::sys::fs::current_path(CfgFilePath);
-llvm::sys::path::append(CfgFilePath, CfgFileName);
-if (!llvm::sys::fs::is_regular_file(CfgFilePath)) {
-  Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
-  return true;
+SmallString<128> CfgFilePath(CfgFileName);
+if (llvm::sys::path::is_relative(CfgFilePath)) {
+  if (getVFS().makeAbsolute(CfgFilePath))
+return true;
+  auto Status = getVFS().status(CfgFilePath);
+  if (!Status ||
+  Status->getType() != llvm::sys::fs::file_type::regular_file) {
+Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+return true;
+  }
 }
 return readConfigFile(CfgFilePath);
   }
@@ -1069,17 +1074,19 @@ bool Driver::loadConfigFile() {
   // Try to find config file. First try file with corrected architect

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks very much for your comments @rjmccall @rsmith , I've take a look at 
D45383 , I believe that user code isn't 
allowed to declare  __builtin_*, but seems `intrin0.inl.h` is a system header 
on windows, should we keep compatibility(like `__va_start` in D45383 
) with it? or breaking it, but I'd like some 
further guidance. can you help me make that decision? I think you guys are 
experts in this area and have more experience than me, I'll see if I can 
accomplish soon! (maybe I should submit a new RFC patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

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

[PATCH] D133415: [clangd] Fix non-idempotent cases of canonicalRenameDecl()

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM to unbreak clangd. Agree that a more thorough look at this is needed.
Maybe add a bug to track this?




Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:1521
   R"cpp(
 template  class Foo { virtual void [[m]](); };
 class Bar : Foo { void [[^m]]() override; };

Quick question to help better understand our current behavior. Not requesting 
to change anything here, just wanted to make sure what we're doing now.

If we run the rename inside the primary template itself, are we going to rename 
the use in `Bar`?
I suspect the answer is "yes" because it's in the same file, so we get it from 
the AST and not from the index. Just to make sure.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133415

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

[PATCH] D133354: [Clang]: Diagnose deprecated copy operations also in MSVC compatibility mode

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

For more context, this was initially raised in 
https://discourse.llvm.org/t/why-is-the-deprecated-copy-warning-suppressed-in-msvc-compatibility-mode/65085/1

I'm not aware of any reason for the special microsoft mode case, but it's very 
possible that I'm missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133354

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80
+ParsedAttributes Attrs(AttrFactory);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+

beanz wrote:
> python3kgae wrote:
> > aaron.ballman wrote:
> > > Just double-checking, but this allows `[[]]` style attributes as well as 
> > > `[]` style attributes, but not `__attribute__` or `__declspec` style 
> > > attributes, is that intended?
> > That is what dxc currently support.
> > It may change in the future. But for now, only [[]] and [] are supported.
> Well... worth noting, HLSL doesn't actually support C++11 attributes, but 
> that is almost certainly going to change in the near future, so we might as 
> well support them from the start in Clang.
Ah, that is good to know @beanz -- we should think carefully about this 
situation because there are some tradeoffs to consider.

1) It's pretty weird to support half of the Microsoft attribute syntax (and the 
half we barely have any attribute support for, at that). Is there a reason to 
not support `__declspec` as well? (For example, are there concerns about things 
like using those attributes to do dllexport or specify a COMDAT section, etc?) 
In fact, if we're going to support vendor attributes like 
`[[clang::overloadable]]`, it's a bit weird that we then prohibit the same 
attribute from being spelled `__attribute__((overloadable))`, is there a 
particular reason to not extend to all attributes?
2) Supporting `[]` style attributes means that attribute order is important. We 
cannot use `MaybeParseAttributes()` to parse attribute specifiers in any order 
because `[]` causes ambiguities under some circumstances. So you're stuck with 
the order you have -- `[[]]` attributes first, `[]` attributes second. Is that 
ordering reasonable?

And for the patch itself -- there are no test cases demonstrating what happens 
when using attributes on things within one of these buffers. I expect many 
things to be quite reasonable, like using `[[deprecated]]`, but are the 
attributes which impact codegen reasonable as well? (Like naked functions, 
returns twice, disable tail calls, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

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

[PATCH] D132977: [HLSL] Call global constructors inside entry

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132977

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

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTDumper.cpp:205
+  if (const Decl *D = dyn_cast(this))
+D->dump();
+}

shafik wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > One thing to note is that the 'else' case here is a little 
> > > > > uninformative.  See 
> > > > > https://clang.llvm.org/doxygen/DeclBase_8cpp_source.html#l00915 for 
> > > > > some similar logic here (though not sure we should be emulating that).
> > > > > 
> > > > > More, I wonder if there is SOME message here that should be dumped 
> > > > > for 'else'.
> > > > Looking at what inherits from `DeclContext`, is there use of it which 
> > > > is *not* a `Decl`? I couldn't find a use where it's not also a `Decl`.
> > > I've DEFINITELY run into it in the debugger before, but I have no idea 
> > > WHAT case that is. It is sometimes just "DeclContext is an invalid 
> > > pointer" kinda thing, so it might be worth-while to have SOME output 
> > > besides "print nothing", particularly when debugging.
> > IIRC, the case this comes up in is when the object is only partially 
> > constructed, and so I agree that having an `else` clause here would be 
> > useful -- because this interface is predominately used from a debugger, it 
> > has to deal with special "impossible" situations a bit more carefully.
> So looking at the other dump member functions, all of them seem to assume we 
> have a valid `DeclContext` and so they do not have any else either.
> 
> So maybe we want to think about making the rest a bit smarter as well?
> So maybe we want to think about making the rest a bit smarter as well?

What do you have in mind?

I think it's plausible to be in a partially constructed state in terms of when 
ctors are called, but I'm far less worried about people calling a dump method 
in the middle of the member inits happening.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133499

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

[PATCH] D133405: [Linux] Hack around Linux/sparc

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D133405#3779190 , @MaskRay wrote:

> So, sparc64 gcc does seem to define the macro by default. (I am using Debian 
> testing and can't tell whether it's distro setting or upstream default) 
> ¯\_(ツ)_/¯
>
>   % sparc64-linux-gnu-gcc -E -dM -xc /dev/null -nostdinc | grep NO_INLINE
>   #define __NO_INLINE__ 1

True, as does `clang`.  However, both define it only at `-O0`.  This is common 
upstream behaviour in both cases.

The following reduced testcase demonstrates the issue:

  $ cat vfprintf-ldbl.c 
  struct _IO_FILE;
  typedef struct _IO_FILE FILE;
  extern FILE *stdout;
  typedef __builtin_va_list __gnuc_va_list;
  extern int vfprintf (FILE *__restrict __s, const char *__restrict __format,
 __gnuc_va_list __arg);
  extern int __inline __attribute__ ((__gnu_inline__))
  vprintf (const char *__restrict __fmt, __gnuc_va_list __arg)
  {
return vfprintf (stdout, __fmt, __arg);
  }
  extern __typeof (vfprintf) vfprintf __asm ("" "_nldbl" "vfprintf");

makes `clang` choke

  $ clang -c vfprintf-ldbl.c
  vfprintf-ldbl.c:12:28: error: cannot apply asm label to function after its 
first use
  extern __typeof (vfprintf) vfprintf __asm ("" "_nldbl" "vfprintf");
 ^   ~~
  1 error generated.

while gcc compiles this without issues.

> This needs a test in `clang/test/Preprocessor/init.c` (find `SPARC`) with a 
> comment linking to the canonical place to discuss the glibc problem 
> (@zatrazz).

Done now and tested on `sparcv9-sun-solaris2.11`.  Will also test on 
`sparc64-unknown-linux-gnu` once the currently running release build is 
finished (this box is **slow**).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133405

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

[PATCH] D133405: [Linux] Hack around Linux/sparc

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 459017.
ro added a comment.

Add comment, tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133405

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -874,6 +874,9 @@
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=sparc-none-none < /dev/null | FileCheck -match-full-lines -check-prefix 
SPARC -check-prefix SPARC-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=sparc-rtems-elf < /dev/null | FileCheck -match-full-lines -check-prefix 
SPARC -check-prefix SPARC-DEFAULT %s
+// Check that clang defines __NO_INLINE__ unconditionally (even at -O) to
+// work around Issue #47994.
+// RUN: %clang_cc1 -E -dM -triple=sparc-unknown-linux-gnu -O < /dev/null | 
FileCheck -match-full-lines -check-prefix SPARC-LINUX %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=sparc-none-netbsd < /dev/null | FileCheck -match-full-lines 
-check-prefix SPARC -check-prefix SPARC-NETOPENBSD %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=sparc-none-none < /dev/null | FileCheck -match-full-lines -check-prefix 
SPARC -check-prefix SPARC-DEFAULT -check-prefix SPARC-DEFAULT-CXX %s
 //
@@ -997,6 +1000,7 @@
 // SPARC:#define __LONG_LONG_MAX__ 9223372036854775807LL
 // SPARC:#define __LONG_MAX__ 2147483647L
 // SPARC-NOT:#define __LP64__
+// SPARC-LINUX:#define __NO_INLINE__ 1
 // SPARC:#define __POINTER_WIDTH__ 32
 // SPARC-DEFAULT:#define __PTRDIFF_TYPE__ int
 // SPARC-NETOPENBSD:#define __PTRDIFF_TYPE__ long int
@@ -1380,6 +1384,11 @@
 // SPARCV9:#define __SIZEOF_POINTER__ 8
 // SPARCV9:#define __UINTPTR_TYPE__ long unsigned int
 //
+// Check that clang defines __NO_INLINE__ unconditionally (even at -O) to
+// work around Issue #47994.
+// RUN: %clang_cc1 -E -dM -triple=sparc64-unknown-linux-gnu -O < /dev/null | 
FileCheck -match-full-lines -check-prefix SPARC64-LINUX %s
+// SPARC64-LINUX:#define __NO_INLINE__ 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=sparc64-none-openbsd < 
/dev/null | FileCheck -match-full-lines -check-prefix SPARC64-OBSD %s
 // SPARC64-OBSD:#define __INT64_TYPE__ long long int
 // SPARC64-OBSD:#define __INTMAX_C_SUFFIX__ LL
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -388,6 +388,9 @@
 } else {
 Builder.defineMacro("__gnu_linux__");
 }
+// Work around Issue #47994 until glibc PR build/27558 is fixed.
+if (Triple.isSPARC())
+  Builder.defineMacro("__NO_INLINE__");
 if (Opts.POSIXThreads)
   Builder.defineMacro("_REENTRANT");
 if (Opts.CPlusPlus)


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -874,6 +874,9 @@
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-none-none < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-rtems-elf < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-DEFAULT %s
+// Check that clang defines __NO_INLINE__ unconditionally (even at -O) to
+// work around Issue #47994.
+// RUN: %clang_cc1 -E -dM -triple=sparc-unknown-linux-gnu -O < /dev/null | FileCheck -match-full-lines -check-prefix SPARC-LINUX %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-none-netbsd < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-NETOPENBSD %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-none-none < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-DEFAULT -check-prefix SPARC-DEFAULT-CXX %s
 //
@@ -997,6 +1000,7 @@
 // SPARC:#define __LONG_LONG_MAX__ 9223372036854775807LL
 // SPARC:#define __LONG_MAX__ 2147483647L
 // SPARC-NOT:#define __LP64__
+// SPARC-LINUX:#define __NO_INLINE__ 1
 // SPARC:#define __POINTER_WIDTH__ 32
 // SPARC-DEFAULT:#define __PTRDIFF_TYPE__ int
 // SPARC-NETOPENBSD:#define __PTRDIFF_TYPE__ long int
@@ -1380,6 +1384,11 @@
 // SPARCV9:#define __SIZEOF_POINTER__ 8
 // SPARCV9:#define __UINTPTR_TYPE__ long unsigned int
 //
+// Check that clang defines __NO_INLINE__ unconditionally (even at -O) to
+// work around Issue #47994.
+// RUN: %clang_cc1 -E -dM -triple=sparc64-unknown-linux-gnu -O < /dev/null | FileCheck -match-full-lines -check-prefix SPARC64-LINUX %s
+// SPARC64-LINUX:#define __NO_INLINE__ 1
+//
 // RUN: %clang_cc1 -E 

[PATCH] D133457: Add Clang driver flags equivalent to cl's /MD, /MT, /MDd, /MTd.

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2209
+def fms_runtime_lib_EQ : Joined<["-"], "fms-runtime-lib=">, Group,
+  Flags<[NoXarchOption, CoreOption]>, HelpText<"Select Windows run-time 
library">;
 defm delayed_template_parsing : BoolFOption<"delayed-template-parsing",

Could this also list the allowed options? And maybe it could also have a 
DocBrief that explains how to use it?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6480
 
+  // Process Windows runtime flags (equivalent to cl flags /MD, /MDd, /MT, 
/MTd)
+  if (Triple.isOSWindows()) {

Could we somehow re-use the logic in Clang::AddClangCLArgs()? Perhaps the main 
switch from that could be extracted to a function that could be called from 
here?



Comment at: clang/test/Driver/cl-runtime-flags.c:99
+
+// Check for clang versions of the /MD and /MT flags.
+

If we want these to behave as the /MD and /MT flags, maybe we can re-use the 
tests for those, so e.g. we'd do:


```
// RUN: %clang_cl -### /MD -- %s 2>&1 | FileCheck -check-prefix=CHECK-MD %s
// RUN: %clang -### -target ... -fms-runtime-lib=dll -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-MD %s
// CHECK-MD-NOT: "-D_DEBUG"
// CHECK-MD: "-D_MT"
// CHECK-MD: "-D_DLL"
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133457

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

Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)

[Aaron Ballman via cfe-commits]

On Thu, Sep 8, 2022 at 12:37 PM David Blaikie  wrote:
>
> Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes
> across the LLVM project for a while now, most, at least I think, are
> quite welcome (things like switching to range-based-for, std
> algorithms over llvm ones, llvm algorithms over manually written
> loops, etc). But yeah, there's some threshold below which the churn
> might not be worth the benefit - especially if the change doesn't come
> along with tooling to enforce the invariant is maintained in the
> future (if it's easy to make mistakes like this one - we'll regress it
> and need to do cleanup again in the future)

Thanks for speaking up, because I also waffled a bit on whether I
called this out or not. :-)

> Also, for this particular one, I wonder if in some cases this sort of
> automatic transformation isn't ideal - if something is a pointer, but
> that's an implementation detail, rather than an intentional feature of
> an API (eg: the pointer-ness might be hidden behind a typedef and used
> as an opaque handle, without any dereferencing, etc)

Agreed.

> I think it'd be really good to have some discussion on discourse about
> if/how some of these cleanups could be formed into a way to
> enforce/encourage the invariant to be maintained going forward -
> clang-tidy (assuming that's the basis for the tooling Kazu's using to
> make these changes in the first place) integration into the LLVM build
> in some way, etc.

I think that's a good idea! We want to encourage cleanups, but we
don't want to encourage churn, and I think it's somewhat subjective
where to draw that line. Having some more community awareness around
that would be beneficial. I'm especially interested in how we balance
between making incremental style improvements to the project and
keeping our git blame logs useful. I'm seeing a lot more git blames
that require several steps to get to an interesting commit because of
the number of NFCs and reverts/recommits. Unfortunately, the tooling
around viewing git blames of large files (like Clang tends to have)
makes these sorts of commits surprisingly painful when you do have to
dig to see where changes came from. (So I find myself having far less
concern when TransGCAttrs.cpp (~350LoC) gets a cleanup like this
compared to SemaExpr.cpp (~21kLoC), which suggests to me we should
maybe strongly consider splitting more of these massive files up so
that churn is less painful.)

> & yeah, adding the `const` too if/when that's relevant would've
> improved the quality/value/justification for a cleanup change like
> this.

It also would have matched our coding style. (We document it somewhat
poorly as an example showing "observe" and "change", but reviewers who
call for cleanups with auto are pretty consistent about asking to make
qualifiers explicit.)

~Aaron

>
> On Sun, Sep 4, 2022 at 5:58 AM Aaron Ballman via cfe-commits
>  wrote:
> >
> > FWIW, sweeping NFC changes like this cause a fair amount of code churn
> > (which makes tools like git blame a bit harder to use) for a
> > relatively small improvement to code readability, which is why our
> > developer policy asks that you "Avoid committing formatting- or
> > whitespace-only changes outside of code you plan to make subsequent
> > changes to." In the future, I'd caution against doing such large-scale
> > sweeping NFC changes outside of areas you're actively working on
> > unless there's some wider discussion with the community first. That
> > said, all of your changes are all improvements, so thank you for them!
> >
> > Some of the changes you made would have ideally made it more clear
> > when the deduced type is a pointer to a const object instead of hiding
> > the qualifier behind the deduction. I've pointed out a couple such
> > places below, but don't feel obligated to go back and change anything
> > unless you're going to be touching other code in the area.
> >
> > ~Aaron
> >
> >
> > On Sun, Sep 4, 2022 at 2:27 AM Kazu Hirata via cfe-commits
> >  wrote:
> > >
> > >
> > > Author: Kazu Hirata
> > > Date: 2022-09-03T23:27:27-07:00
> > > New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
> > >
> > > URL: 
> > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
> > > DIFF: 
> > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff
> > >
> > > LOG: [clang] Qualify auto in range-based for loops (NFC)
> > >
> > > Added:
> > >
> > >
> > > Modified:
> > > clang/lib/ARCMigrate/ObjCMT.cpp
> > > clang/lib/ARCMigrate/TransGCAttrs.cpp
> > > clang/lib/AST/ASTContext.cpp
> > > clang/lib/AST/ASTImporter.cpp
> > > clang/lib/AST/Decl.cpp
> > > clang/lib/AST/DeclObjC.cpp
> > > clang/lib/AST/ODRHash.cpp
> > > clang/lib/AST/OpenMPClause.cpp
> > > clang/lib/AST/StmtProfile.cpp
> > > clang/lib/AST/Type.cpp
> > > clang/lib/Analysis/CFG.cpp
> > > clang/lib/Analysis/ThreadSafetyCommon.cpp
> > > clang/lib/CodeGen

[PATCH] D133570: [Clang] changing behavior of constant array emission [AsmPrinter] changing Size from unsigned to uint64_t

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
OfekShochat requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

fixes issue with emitting partially initialized constant arrays larger than 
2^32.
issue #57353 on github.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133570

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp


Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3164,7 +3164,7 @@
  const Constant *BaseCV, uint64_t Offset,
  AsmPrinter::AliasMapTy *AliasList) {
   // Print the fields in successive locations. Pad to align if needed!
-  unsigned Size = DL.getTypeAllocSize(CS->getType());
+  uint64_t Size = DL.getTypeAllocSize(CS->getType());
   const StructLayout *Layout = DL.getStructLayout(CS->getType());
   uint64_t SizeSoFar = 0;
   for (unsigned I = 0, E = CS->getNumOperands(); I != E; ++I) {
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -1222,29 +1222,18 @@
   llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
 auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
 assert(CAT && "can't emit array init for non-constant-bound array");
-unsigned NumInitElements = ILE->getNumInits();
-unsigned NumElements = CAT->getSize().getZExtValue();
+uint64_t NumInitElements = ILE->getNumInits();
+uint64_t NumElements = CAT->getSize().getZExtValue();
+printf("hi\n");
 
 // Initialising an array requires us to automatically
 // initialise any elements that have not been initialised explicitly
-unsigned NumInitableElts = std::min(NumInitElements, NumElements);
+uint64_t NumInitableElts = std::min(NumInitElements, NumElements);
 
 QualType EltType = CAT->getElementType();
 
-// Initialize remaining array elements.
-llvm::Constant *fillC = nullptr;
-if (Expr *filler = ILE->getArrayFiller()) {
-  fillC = Emitter.tryEmitAbstractForMemory(filler, EltType);
-  if (!fillC)
-return nullptr;
-}
-
-// Copy initializer elements.
-SmallVector Elts;
-if (fillC && fillC->isNullValue())
-  Elts.reserve(NumInitableElts + 1);
-else
-  Elts.reserve(NumElements);
+SmallVector Inits;
+Inits.reserve(NumInitableElts + 1);
 
 llvm::Type *CommonElementType = nullptr;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
@@ -1256,13 +1245,33 @@
 CommonElementType = C->getType();
   else if (C->getType() != CommonElementType)
 CommonElementType = nullptr;
-  Elts.push_back(C);
+  Inits.push_back(C);
 }
 
-llvm::ArrayType *Desired =
+uint64_t TrailingZeroes = NumElements - NumInitElements;
+
+// If all elements have the same type, just emit an array constant.
+if (CommonElementType && TrailingZeroes == 0)
+  return llvm::ConstantArray::get(
+  llvm::ArrayType::get(CommonElementType, NumElements), Inits);
+
+llvm::ArrayType *DesiredType =
 cast(CGM.getTypes().ConvertType(ILE->getType()));
-return EmitArrayConstant(CGM, Desired, CommonElementType, NumElements, 
Elts,
- fillC);
+
+auto *FillerType =
+CommonElementType ? CommonElementType : DesiredType->getElementType();
+FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
+
+Inits.push_back(llvm::ConstantAggregateZero::get(FillerType));
+
+llvm::SmallVector Types;
+for (llvm::Constant *Elt : Inits)
+  Types.push_back(Elt->getType());
+
+llvm::StructType *SType =
+llvm::StructType::get(CGM.getLLVMContext(), Types, true);
+
+return llvm::ConstantStruct::get(SType, Inits);
   }
 
   llvm::Constant *EmitRecordInitialization(InitListExpr *ILE, QualType T) {


Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3164,7 +3164,7 @@
  const Constant *BaseCV, uint64_t Offset,
  AsmPrinter::AliasMapTy *AliasList) {
   // Print the fields in successive locations. Pad to align if needed!
-  unsigned Size = DL.getTypeAllocSize(CS->getType());
+  uint64_t Size = DL.getTypeAllocSize(CS->getType());
   const StructLayout *Layout = DL.getStructLayout(CS->getType());
   uint64_t SizeSoFar = 0;
   for (unsigned I = 0, E = CS->getNumOperands(); I != E; ++I) {
Index: clang/lib/CodeGen/CGExprConstant.c

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, owenpan, curdeius.
MyDeveloperDay added a project: clang-format.
Herald added a project: All.
MyDeveloperDay requested review of this revision.
Herald added a project: clang.

As highlighted by https://github.com/llvm/llvm-project/issues/57609

Give more control when using the AllowShortCaseLabelsOnASingleLine to not put 
fallthrough code onto the same line

  switch (i) {
  case 0: g(); [[fallthrough]];
  case 1: f(); break;
  }

whether that fallthrough is marked with the attribute or implicit

  switch (i) {
  case 0:
  case 1: return i;
  case 2:
  case 3: i *= 2; break;
  }

in these cases don't make them short

  switch (i) {
  case 0:
g();
[[fallthrough]];
  case 1:
f();
break;
  }



  switch (i) {
  case 0:
  case 1:
return i;
  case 2:
  case 3:
i *= 2;
break;
  }

Fixes: 57609


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133571

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2882,7 +2882,7 @@
 
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   verifyFormat("switch (a) {\n"
"case 1: x = 1; break;\n"
"case 2: return;\n"
@@ -3011,7 +3011,7 @@
"}",
Style);
   Style.ColumnLimit = 80;
-  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   Style.IndentCaseLabels = true;
   EXPECT_EQ("switch (n) {\n"
 "  default /*comments*/:\n"
@@ -3026,7 +3026,7 @@
"  return false;\n"
"}",
Style));
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
@@ -3051,6 +3051,55 @@
"  }\n"
"}",
Style));
+
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_NoFallThrough;
+  /*
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "case 1: x = 1; break;\n"
+   "case 2: return;\n"
+   "case 3:\n"
+   "case 4:\n"
+   "case 5: return;\n"
+   "case 6: // comment\n"
+   "  return;\n"
+   "case 7:\n"
+   "  // comment\n"
+   "  return;\n"
+   "case 8:\n"
+   "  x = 8; // comment\n"
+   "  break;\n"
+   "default: y = 1; break;\n"
+   "}",
+   Style);
+  */
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1:\n"
+   "x = 1;\n"
+   "break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3:\n"
+   "x = 3;\n"
+   "[[fallthrough]];\n"
+   "  case 4:\n"
+   "x = 4;\n"
+   "break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1: x = 1; break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3: x = 3; [[fallthrough]];\n"
+   "  case 4: x = 4; break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsLabels) {
@@ -20148,7 +20197,6 @@
   CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -20509,6 +20557,20 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Never",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Always",

[PATCH] D133570: #57353 on github

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a comment.

its my first time contributing. this is probably very bad and goes against a 
lot of guidelines, but I wouldnt resist to fix them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 459024.
MyDeveloperDay added a comment.

self review


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

https://reviews.llvm.org/D133571

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2882,7 +2882,7 @@
 
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   verifyFormat("switch (a) {\n"
"case 1: x = 1; break;\n"
"case 2: return;\n"
@@ -3011,7 +3011,7 @@
"}",
Style);
   Style.ColumnLimit = 80;
-  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   Style.IndentCaseLabels = true;
   EXPECT_EQ("switch (n) {\n"
 "  default /*comments*/:\n"
@@ -3026,7 +3026,7 @@
"  return false;\n"
"}",
Style));
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
@@ -3051,6 +3051,55 @@
"  }\n"
"}",
Style));
+
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_NoFallThrough;
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 1: x = 1; break;\n"
+   "  case 2: return;\n"
+   "  case 3:\n"
+   "  case 4:\n"
+   "  case 5:\n"
+   "return;\n"
+   "  case 6: // comment\n"
+   "return;\n"
+   "  case 7:\n"
+   "// comment\n"
+   "return;\n"
+   "  case 8:\n"
+   "x = 8; // comment\n"
+   "break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
+
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1:\n"
+   "x = 1;\n"
+   "break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3:\n"
+   "x = 3;\n"
+   "[[fallthrough]];\n"
+   "  case 4:\n"
+   "x = 4;\n"
+   "break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1: x = 1; break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3: x = 3; [[fallthrough]];\n"
+   "  case 4: x = 4; break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsLabels) {
@@ -20148,7 +20197,6 @@
   CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -20509,6 +20557,20 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Never",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Always",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: NoFallThrough",
+  AllowShortCaseLabelsOnASingleLine,
+  FormatStyle::SCLS_NoFallThrough);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: false",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: true",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+
   Style.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Both;
   CHECK_PARSE("SpaceAroundPointerQualifiers: Default",
   SpaceAroundPointerQualifiers, FormatStyle::SAPQ_Default);
Index:

[PATCH] D133354: [Clang]: Diagnose deprecated copy operations also in MSVC compatibility mode

[Julius via Phabricator via cfe-commits]

ningvin added a comment.

In D133354#3779123 , @dblaikie wrote:

> That works, you can also take the hash with the `rG` prefix and use that: 
> rGd577fbbd1c9d6dab193d530fcd807efc3b3bc9ad 
>  and 
> that works too.

Didn't know that, thanks!

In D133354#3779765 , @hans wrote:

> For more context, this was initially raised in 
> https://discourse.llvm.org/t/why-is-the-deprecated-copy-warning-suppressed-in-msvc-compatibility-mode/65085/1

Oh yes sorry, I should have linked to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133354

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

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: erichkeane, ldionne.
aaron.ballman added a comment.

CC @erichkeane for attribute questions, and CC @ldionne for STL questions and 
design/usability of the proposed attribute

In D133029#3779663 , @ilya-biryukov 
wrote:

> In D133029#3766058 , @shafik wrote:
>
>> This old cfe-dev thread might be relevant: 
>> https://lists.llvm.org/pipermail/cfe-dev/2018-June/058338.html
>
> Thanks, the thread is definitely relevant.
>
>> It appears that folks take advantage of this working a lot. It sounds like 
>> your saying w/ constexpr std::vector this would be ill-formed in a constant 
>> expression context now. Which won't effect most of the existing code I think?
>
> Most of the code is not affected, but some code is and large codebases will 
> definitely see quite some breakages.
> Note that this warning diagnoses is very pedantic in terms of implementing 
> the rules from the standard. It fires in many places than don't actually 
> break in libc++ and C++20 mode. On the flip side, it is really easy to 
> implement.

I assume the standard rules you're talking about are: 
http://eel.is/c++draft/containers#vector.overview-4 (and other instances of 
similar wording for other containers)? If so, shouldn't this requirement be 
addressed by the STL implementation and not the compiler?

> In D133029#3778230 , @dblaikie 
> wrote:
>
>> any chance of generalizing this beyond a special case for `std::vector` - an 
>> attribute we could add to any class template (perhaps particular template 
>> parameters to the class template) to document the requirement? (is 
>> std::vector the only type in the standard library that has this property, or 
>> only the one people trip over most often)
>
> It should be easy to generalize, e.g. we could introduce an attribute to mark 
> classes that want this behavior, something along the lines of:
>
>   // Option 1. Less general, easy, small overhead.
>   template  struct [[member_access_requires_complete(T)]] my_array 
> {};
>   // Option 2. More general, might be high-overhead depending on the actual 
> expression.
>   template  struct [[member_access_requires(sizeof (T))]] my_array 
> {}; // checks for validity of expression on every member access?
>
> I am not sure this carries it weight for that particular use-case and there 
> are other things to consider, e.g. how to report error messages.
> What is definitely useful and cheap is adding these warnings for more STL 
> containers that have this requirement.
>
> We discussed this with @aaron.ballman in the last Clang C++ Language WG 
> meeting and one idea that popped up is implementing a warning in Clang, but 
> never actually surfacing it in the compiler, only in `clang-tidy`. I would be 
> very comfortable making a `clang-tidy` warning like this as the bar is much 
> lower than the compiler warnings.
> My proposal would be to allow making warnings that are disabled by default 
> and even with `-Weverything`, the only way to enable them would be to pass 
> `-W` explicitly or run a corresponding `clang-tidy` check. 
> Alternatively, there will be no way to enable the warning in the compiler at 
> all and the clang-tidy would be the only way to surface it.
>
> Obviously, these use-cases should be rare as most things can be implemented 
> directly in `clang-tidy`, but in that particular case we need access to the 
> compiler state in the middle of processing the TU, something that 
> `clang-tidy` APIs do not provide. We should also be careful to make sure any 
> warnings like this do not hurt performance of the compiler.
>
> @aaron.ballman does that look reasonable? I am about to try prototyping this. 
> Are there any open questions we might want to agree on beforehand?

An attribute is an interesting idea, but I'm not certain it's fully necessary; 
I think it's effectively a slightly nicer form of: 
https://godbolt.org/z/ha6Tb1s4M

The reason why I'm wondering whether libc++ should handle this is: the 
requirements are library requirements, not compiler requirements, so I think 
the library should decide whether to enforce this rule or not. An attribute is 
an interesting idea in so much as it may help the library authors to implement 
this more easily, but the downside to the attribute is that it won't help 
libc++ out too much because they still need to work with other compilers that 
don't support the attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133029

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

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:935
+**AllowShortCaseLabelsOnASingleLine** (``ShortCaseLabelStyle``) 
:versionbadge:`clang-format 3.6`
+  Determine if Short case labels will be contracted to a single line.
+





Comment at: clang/docs/ClangFormatStyleOptions.rst:3596
 
-  QualifierOrder: ['inline', 'static', 'type', 'const']
+  QualifierOrder: ['inline', 'static' , 'type', 'const']
 

Unrelated change.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:643-657
+bool NoFallThrough = Style.AllowShortCaseLabelsOnASingleLine ==
+ FormatStyle::SCLS_NoFallThrough;
+// If the last thing on the line before was just case X:  then don't merge.
+if (NoFallThrough && PreviousLine && PreviousLine->Last &&
+PreviousLine->Last->is(tok::colon))
+  return 0;
+// Check if the last thing on the line before was an attribute and if so

Can we merge the conditions like this?


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

https://reviews.llvm.org/D133571

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 459028.
zahiraam marked an inline comment as done.

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

https://reviews.llvm.org/D123630

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/Driver/fp-contract.c

Index: clang/test/Driver/fp-contract.c
===
--- /dev/null
+++ clang/test/Driver/fp-contract.c
@@ -0,0 +1,114 @@
+// Test that -ffp-contract is set to the right value when combined with
+// the options -ffast-math and -fno-fast-math.
+
+// RUN: %clang -### -ffast-math -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// CHECK-FPC-FAST: "-ffp-contract=fast"
+
+// RUN: %clang -### -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// CHECK-FPC-ON:   "-ffp-contract=on"
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+// CHECK-FPC-OFF:  "-ffp-contract=off"
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=off \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clan

[PATCH] D133574: [C2x] reject type definitions in offsetof

[YingChi Long via Phabricator via cfe-commits]

inclyc created this revision.
Herald added a project: All.
inclyc updated this revision to Diff 459034.
inclyc added a comment.
inclyc added reviewers: aaron.ballman, clang-language-wg.
inclyc published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Apply clang-format


https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm made very
clear that it is an UB having type definitions with in offsetof. After
this patch clang will reject any type definitions in __builtin_offsetof.

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

I'm not sure this is a correct bug fix, because adding a new bool state
to Parser just for supporting this seems ugly. I've tried creating a new
DeclaratorContext, though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133574

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/declarators.c
  clang/test/Sema/offsetof.c

Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -70,3 +70,16 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int test_definition(void) {
+  return __builtin_offsetof(struct A // expected-error{{'struct A' cannot be defined in '__builtin_offsetof'}} 
+  { 
+int a;
+struct B // expected-error{{'struct B' cannot be defined in '__builtin_offsetof'}}
+{
+  int c;
+  int d;
+} x;
+  }, a);
+}
Index: clang/test/Parser/declarators.c
===
--- clang/test/Parser/declarators.c
+++ clang/test/Parser/declarators.c
@@ -80,10 +80,6 @@
 struct test10 { int a; } static test10x;
 struct test11 { int a; } const test11x;
 
-// PR6216
-void test12(void) {
-  (void)__builtin_offsetof(struct { char c; int i; }, i);
-}
 
 // rdar://7608537
 struct test13 { int a; } (test13x);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16254,7 +16254,7 @@
  SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, bool IsWithinOffsetOf) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -17027,6 +17027,12 @@
cast_or_null(PrevDecl));
   }
 
+  if (IsWithinOffsetOf && TUK == TUK_Definition) {
+Diag(New->getLocation(), diag::err_type_defined_in_offsetof)
+<< Context.getTagDeclType(New);
+Invalid = true;
+  }
+
   // C++11 [dcl.type]p3:
   //   A type-specifier-seq shall not define a class or enumeration [...].
   if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2579,7 +2579,9 @@
   }
   case tok::kw___builtin_offsetof: {
 SourceLocation TypeLoc = Tok.getLocation();
+InBuiltInOffsetOfBase = true;
 TypeResult Ty = ParseTypeName();
+InBuiltInOffsetOfBase = false;
 if (Ty.isInvalid()) {
   SkipUntil(tok::r_paren, StopAtSemi);
   return ExprError();
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2030,7 +2030,7 @@
 DSC == DeclSpecContext::DSC_type_specifier,
 DSC == DeclSpecContext::DSC_template_param ||
 DSC == DeclSpecContext::DSC_template_type_arg,
-&SkipBody);
+&SkipBody, InBuiltInOffsetOfBase);
 
 // If ActOnTag said the type was dependent, try again with the
 // less common call.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3273,7 +3273,8 @@
  bool &IsDependent, SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody = nullptr);
+ SkipBodyInfo *SkipBody = nullptr,
+ boo

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

Just my .02, but I am conflicted between:

1. Simply not doing anything -- the diagnostic users get when they violate the 
requirement currently is probably not that bad? I did see this breakage a bit 
in our internal code base as well, but it was easy to fix and there were not 
many instances.
2. Adding the attribute that was suggested and using it in libc++. On compilers 
that don't support the attribute, we'd simply be less pedantic.

The one thing I'd rather not do is `static_assert(__is_complete<_Tp>::value)` 
in all the `std::vector` member functions, IMO that adds complexity and reduces 
readability for a really marginal gain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133029

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 459037.
inclyc added a comment.

Add release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/declarators.c
  clang/test/Sema/offsetof.c

Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -70,3 +70,16 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int test_definition(void) {
+  return __builtin_offsetof(struct A // expected-error{{'struct A' cannot be defined in '__builtin_offsetof'}} 
+  { 
+int a;
+struct B // expected-error{{'struct B' cannot be defined in '__builtin_offsetof'}}
+{
+  int c;
+  int d;
+} x;
+  }, a);
+}
Index: clang/test/Parser/declarators.c
===
--- clang/test/Parser/declarators.c
+++ clang/test/Parser/declarators.c
@@ -80,10 +80,6 @@
 struct test10 { int a; } static test10x;
 struct test11 { int a; } const test11x;
 
-// PR6216
-void test12(void) {
-  (void)__builtin_offsetof(struct { char c; int i; }, i);
-}
 
 // rdar://7608537
 struct test13 { int a; } (test13x);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16254,7 +16254,7 @@
  SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, bool IsWithinOffsetOf) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -17027,6 +17027,12 @@
cast_or_null(PrevDecl));
   }
 
+  if (IsWithinOffsetOf && TUK == TUK_Definition) {
+Diag(New->getLocation(), diag::err_type_defined_in_offsetof)
+<< Context.getTagDeclType(New);
+Invalid = true;
+  }
+
   // C++11 [dcl.type]p3:
   //   A type-specifier-seq shall not define a class or enumeration [...].
   if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2579,7 +2579,9 @@
   }
   case tok::kw___builtin_offsetof: {
 SourceLocation TypeLoc = Tok.getLocation();
+InBuiltInOffsetOfBase = true;
 TypeResult Ty = ParseTypeName();
+InBuiltInOffsetOfBase = false;
 if (Ty.isInvalid()) {
   SkipUntil(tok::r_paren, StopAtSemi);
   return ExprError();
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2030,7 +2030,7 @@
 DSC == DeclSpecContext::DSC_type_specifier,
 DSC == DeclSpecContext::DSC_template_param ||
 DSC == DeclSpecContext::DSC_template_type_arg,
-&SkipBody);
+&SkipBody, InBuiltInOffsetOfBase);
 
 // If ActOnTag said the type was dependent, try again with the
 // less common call.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3273,7 +3273,8 @@
  bool &IsDependent, SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody = nullptr);
+ SkipBodyInfo *SkipBody = nullptr,
+ bool IsWithinOffsetOf = false);
 
   Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
 unsigned TagSpec, SourceLocation TagLoc,
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -246,6 +246,9 @@
   /// function call.
   bool CalledSignatureHelp = false;
 
+  /// Parsing a type within __builtin_offsetof.
+  bool InBuiltInOffsetOfBase = false;
+
   /// The "depth" of the template parameters c

[PATCH] D133574: [C2x] reject type definitions in offsetof

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 459039.
inclyc added a comment.

Use double backquotes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/declarators.c
  clang/test/Sema/offsetof.c

Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -70,3 +70,16 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int test_definition(void) {
+  return __builtin_offsetof(struct A // expected-error{{'struct A' cannot be defined in '__builtin_offsetof'}} 
+  { 
+int a;
+struct B // expected-error{{'struct B' cannot be defined in '__builtin_offsetof'}}
+{
+  int c;
+  int d;
+} x;
+  }, a);
+}
Index: clang/test/Parser/declarators.c
===
--- clang/test/Parser/declarators.c
+++ clang/test/Parser/declarators.c
@@ -80,10 +80,6 @@
 struct test10 { int a; } static test10x;
 struct test11 { int a; } const test11x;
 
-// PR6216
-void test12(void) {
-  (void)__builtin_offsetof(struct { char c; int i; }, i);
-}
 
 // rdar://7608537
 struct test13 { int a; } (test13x);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16254,7 +16254,7 @@
  SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, bool IsWithinOffsetOf) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -17027,6 +17027,12 @@
cast_or_null(PrevDecl));
   }
 
+  if (IsWithinOffsetOf && TUK == TUK_Definition) {
+Diag(New->getLocation(), diag::err_type_defined_in_offsetof)
+<< Context.getTagDeclType(New);
+Invalid = true;
+  }
+
   // C++11 [dcl.type]p3:
   //   A type-specifier-seq shall not define a class or enumeration [...].
   if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2579,7 +2579,9 @@
   }
   case tok::kw___builtin_offsetof: {
 SourceLocation TypeLoc = Tok.getLocation();
+InBuiltInOffsetOfBase = true;
 TypeResult Ty = ParseTypeName();
+InBuiltInOffsetOfBase = false;
 if (Ty.isInvalid()) {
   SkipUntil(tok::r_paren, StopAtSemi);
   return ExprError();
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2030,7 +2030,7 @@
 DSC == DeclSpecContext::DSC_type_specifier,
 DSC == DeclSpecContext::DSC_template_param ||
 DSC == DeclSpecContext::DSC_template_type_arg,
-&SkipBody);
+&SkipBody, InBuiltInOffsetOfBase);
 
 // If ActOnTag said the type was dependent, try again with the
 // less common call.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3273,7 +3273,8 @@
  bool &IsDependent, SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody = nullptr);
+ SkipBodyInfo *SkipBody = nullptr,
+ bool IsWithinOffsetOf = false);
 
   Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
 unsigned TagSpec, SourceLocation TagLoc,
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -246,6 +246,9 @@
   /// function call.
   bool CalledSignatureHelp = false;
 
+  /// Parsing a type within __builtin_offsetof.
+  bool InBuiltInOffsetOfBase = false;
+
   /// The "depth" of the template paramete

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

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

In D129755#3777038 , @aaronpuchert 
wrote:

> I was under the impression that we've already switched to C++17, but the 
> Windows pre-submit build failed with:
>
>   
> C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2107):
>  error C2429: language feature 'init-statements in if/switch' requires 
> compiler flag '/std:c++17'
>   
> C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2120):
>  error C2429: language feature 'init-statements in if/switch' requires 
> compiler flag '/std:c++17'
>   
> C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2418):
>  error C2429: language feature 'init-statements in if/switch' requires 
> compiler flag '/std:c++17'
>
> Perhaps I should move the init statements out again?

No, I've filed https://github.com/google/llvm-premerge-checks/issues/416 to 
find out what's going on with the Windows precommit CI bot.

The changes LGTM, though it'd still be great if @delesley had some time to put 
a second set of eyes on the changes. If we don't hear from him by Tue, I think 
we should go ahead and land this. Please be sure to add a release note for the 
changes!




Comment at: clang/lib/Analysis/ThreadSafetyCommon.cpp:339-342
+if (const Expr *SelfArg = dyn_cast(Ctx->SelfArg))
+  return translate(SelfArg, Ctx->Prev);
+else
+  return cast(Ctx->SelfArg);

aaronpuchert wrote:
> aaron.ballman wrote:
> > 
> No doubt referring to 
> https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return. I 
> was trying to emphasize the dual nature of `llvm::PointerUnion til::SExpr *>`, but I can certainly also drop the `else`.
I don't have a strong opinion.



Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4230-4236
+void testDeferredTemporary() {
+  SelfLockDeferred(); // expected-warning {{releasing mutex '.mu_' 
that was not held}}
+}
+
+void testDeferredTemporary2() {
+  SelfLockDeferred2(); // expected-warning {{releasing mutex '' 
that was not held}}
+}

aaronpuchert wrote:
> Here we're printing ``, because we don't have a name for this 
> object.
Ah thank you, this example makes a lot of sense. I think printing `` 
is quite reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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

[PATCH] D133570: #57353 on github

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a comment.

trying to fix the problem. probably going to split this into two patches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133570: #57353 on github

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added a comment.

In D133570#3779856 , @OfekShochat 
wrote:

> its my first time contributing. this is probably very bad and goes against a 
> lot of guidelines, but I wouldnt resist to fix this

Welcome, and thank you for the patch! I'm going to add a few more reviewers who 
typically look at codegen changes, but I do have some high-level suggestions. 
Can you edit the title of the review (there's an "edit revision" link in the 
upper-right of the review) so that there's a short description of what is being 
fixed in the title? The title is what most reviewers see in our inbox/review 
list, so knowing what the patch is about helps reviewers immensely (also, we 
use the title of the review as the title of the commit message). Also, we ask 
that all changes to the code base come with test coverage showing what has been 
fixed, so please add some test coverage (most likely to `clang/test/CodeGen/`). 
Finally, we like to add information to our release notes about bug fixes so 
users know about the improvements, so please add a short release note (to 
clang/docs/ReleaseNotes.rst).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133570: #57353 on github

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a comment.

hello! thank you so much, Ill do that now, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[clang] d3c54a1 - [HLSL] Call global constructors inside entry

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-09-09T09:01:28-05:00
New Revision: d3c54a172d48a2e3a0c2740c300a1d7dbdf4e292

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

LOG: [HLSL] Call global constructors inside entry

HLSL doesn't have a runtime loader model that supports global
construction by a loader or runtime initializer. To allow us to leverage
global constructors with minimal code generation impact we put calls to
the global constructors inside the generated entry function.

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

Added: 
clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
clang/test/CodeGenHLSL/GlobalConstructorLib.hlsl
clang/test/CodeGenHLSL/GlobalConstructors.hlsl
clang/test/SemaHLSL/GlobalConstructors.hlsl

Modified: 
clang/docs/HLSL/EntryFunctions.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGHLSLRuntime.cpp
clang/lib/CodeGen/CGHLSLRuntime.h
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/docs/HLSL/EntryFunctions.rst 
b/clang/docs/HLSL/EntryFunctions.rst
index 0e547aab420c8..8591814777388 100644
--- a/clang/docs/HLSL/EntryFunctions.rst
+++ b/clang/docs/HLSL/EntryFunctions.rst
@@ -37,10 +37,16 @@ linkage following normal function code generation.
 
 The actual exported entry function which can be called by the GPU driver is a
 ``void(void)`` function that isn't name mangled. In code generation we generate
-the unmangled entry function, instantiations of the parameters with their
-semantic values populated, and a call to the user-defined function. After the
-call instruction the return value (if any) is saved using a target-appropriate
-intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+the unmangled entry function to serve as the actual shader entry. The shader
+entry function is annotated with the ``hlsl.shader`` function attribute
+identifying the entry's pipeline stage.
+
+The body of the unmangled entry function contains first a call to execute 
global
+constructors, then instantiations of the user-defined entry parameters with
+their semantic values populated, and a call to the user-defined function.
+After the call instruction the return value (if any) is saved using a
+target-appropriate intrinsic for storing outputs (for DirectX, the
+``llvm.dx.store.output``). Global destructors are not supported in HLSL.
 
 .. note::
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a59eeda85bc03..76e18c9deff46 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11655,6 +11655,8 @@ def err_hlsl_attribute_param_mismatch : Error<"%0 
attribute parameters do not ma
 def err_hlsl_missing_semantic_annotation : Error<
   "semantic annotations must be present for all parameters of an entry "
   "function or patch constant function">;
+def err_hlsl_init_priority_unsupported : Error<
+  "initializer priorities are not supported in HLSL">;
 
 def err_hlsl_pointers_unsupported : Error<
   "%select{pointers|references}0 are unsupported in HLSL">;

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 9ca3b64996edd..2f0c00169af69 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -54,6 +54,8 @@ void CGHLSLRuntime::finishCodeGen() {
   Triple T(M.getTargetTriple());
   if (T.getArch() == Triple::ArchType::dxil)
 addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
+
+  generateGlobalCtorCalls();
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) 
{
@@ -143,3 +145,40 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl 
*FD,
   // FIXME: Handle codegen for return type semantics.
   B.CreateRetVoid();
 }
+
+void CGHLSLRuntime::generateGlobalCtorCalls() {
+  llvm::Module &M = CGM.getModule();
+  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+  if (!GlobalCtors)
+return;
+  const auto *CA = dyn_cast(GlobalCtors->getInitializer());
+  if (!CA)
+return;
+  // The global_ctor array elements are a struct [Priority, Fn *, COMDat].
+  // HLSL neither supports priorities or COMDat values, so we will check those
+  // in an assert but not handle them.
+
+  llvm::SmallVector CtorFns;
+  for (const auto &Ctor : CA->operands()) {
+if (isa(Ctor))
+  continue;
+ConstantStruct *CS = cast(Ctor);
+
+assert(cast(CS->getOperand(0))->getValue() == 65535 &&
+   "HLSL doesn't support setting priority for global ctors.");
+assert(isa(CS->getOperand(2)) &&
+   "HLSL doesn't support COMDat for global ctors.");
+CtorFns.push_back(cast(CS->getOperand(1)));
+  }
+
+  // Inse

[PATCH] D132977: [HLSL] Call global constructors inside entry

[Chris Bieneman 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 rGd3c54a172d48: [HLSL] Call global constructors inside entry 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132977

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
  clang/test/CodeGenHLSL/GlobalConstructorLib.hlsl
  clang/test/CodeGenHLSL/GlobalConstructors.hlsl
  clang/test/SemaHLSL/GlobalConstructors.hlsl

Index: clang/test/SemaHLSL/GlobalConstructors.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/GlobalConstructors.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fsyntax-only %s -verify
+
+int i;
+
+struct Pup {
+  Pup() {
+i++;
+  }
+};
+
+// expected-error@+1 {{initializer priorities are not supported in HLSL}}
+Pup __attribute__((init_priority(1))) Fido;
+
+// expected-error@+1 {{initializer priorities are not supported in HLSL}}
+__attribute__((constructor(1))) void call_me_first(void) {
+  i = 12;
+}
+
Index: clang/test/CodeGenHLSL/GlobalConstructors.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/GlobalConstructors.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+RWBuffer Buffer;
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {}
+
+//CHECK:  define void @main()
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   call void @_GLOBAL__sub_I_GlobalConstructors.hlsl()
+//CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+//CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   ret void
Index: clang/test/CodeGenHLSL/GlobalConstructorLib.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/GlobalConstructorLib.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+RWBuffer Buffer;
+
+[shader("compute")]
+[numthreads(1,1,1)]
+void FirstEntry() {}
+
+// CHECK: define void @FirstEntry()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @_GLOBAL__sub_I_GlobalConstructorLib.hlsl()
+
+[shader("compute")]
+[numthreads(1,1,1)]
+void SecondEntry() {}
+
+// CHECK: define void @SecondEntry()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @_GLOBAL__sub_I_GlobalConstructorLib.hlsl()
+// CHECK-NEXT:   call void @"?SecondEntry@@YAXXZ"()
Index: clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+int i;
+
+__attribute__((constructor)) void call_me_first(void) {
+  i = 12;
+}
+
+__attribute__((constructor)) void then_call_me(void) {
+  i = 12;
+}
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {}
+
+//CHECK: define void @main()
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   call void @"?call_me_first@@YAXXZ"()
+//CHECK-NEXT:   call void @"?then_call_me@@YAXXZ"()
+//CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+//CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   ret void
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2328,6 +2328,10 @@
 
 static void handleConstructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   uint32_t priority = ConstructorAttr::DefaultPriority;
+  if (S.getLangOpts().HLSL && AL.getNumArgs()) {
+S.Diag(AL.getLoc(), diag::err_hlsl_init_priority_unsupported);
+return;
+  }
   if (AL.getNumArgs() &&
   !checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority))
 return;
@@ -3729,6 +3733,11 @@
 S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
 return;
   }
+  
+  if (S.getLangOpts().HLSL) {
+S.Diag(AL.getLoc(), diag::err_hlsl_init_priority_unsupported);
+return;
+  }
 
   if (S.getCurFunctionOrMethodDecl()) {
 S.Diag(AL.getLoc(), diag::err_init_priority_object_attr);
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -46,6 +46,7 @@
   virtual ~CGHLSLRuntime() {}
 
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
+  void generateGlobalCtorCalls(

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a comment.

hmm, one problem, seems like nor clang and nor gcc can compile the example in 
the github issue fully, only to assembly. is there something like c -> llir, 
llir -> asm checks? in the issue, it did succeed to compile to either llir and 
asm, just not correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133570#3780071 , @OfekShochat 
wrote:

> hello! thank you so much, Ill do that now, thanks! just a question, what are 
> your guidelines when it comes to commit naming/number etc?

Thanks, the new title is great! I'm not certain I understand your question, but 
our full developer policy can be found at: 
https://llvm.org/docs/DeveloperPolicy.html which may help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133578: [OpenMP] Add generation of SIMD align assumptions to OMPIRBuilder

[Dominik Adamski via Phabricator via cfe-commits]

domada created this revision.
domada added a reviewer: kiranchandramohan.
Herald added subscribers: bzcheeseman, awarzynski, sdasgup3, wenzhicui, wrengr, 
cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, 
rriddle, mehdi_amini, guansong, hiraditya, yaxunl.
Herald added a reviewer: ftynse.
Herald added a project: All.
domada requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, sstefan1, 
stephenneuendorffer, nicolasvasilache.
Herald added a reviewer: dcaballe.
Herald added projects: clang, MLIR, LLVM.

Currently generation of align assumptions for OpenMP simd construct is done 
outside OMPIRBuilder for C code and it is not supported for Fortran.

According to OpenMP 5.0 standard (2.9.3) only pointers and arrays can be 
aligned for C code.

If given aligned variable is pointer, then Clang generates the following set of 
the LLVM IR instructions to support simd align clause:

  ; memory allocation for pointer address:
  %A.addr = alloca ptr, align 8
  ; some LLVM IR code
  ; Alignment instructions (alignment is equal to 32):
  %0 = load ptr, ptr %A.addr, align 8
  call void @llvm.assume(i1 true) [ "align"(ptr %0, i64 32) ]

If given aligned variable is array, then Clang generates the following set of 
the LLVM IR instructions to support simd align clause:

  ; memory allocation for array: 
  %B = alloca [10 x i32], align 16
  ; some LLVM IR code
  ; Alignment instructions (alignment is equal to 32):
  %arraydecay = getelementptr inbounds [10 x i32], ptr %B, i64 0, i64 0
  call void @llvm.assume(i1 true) [ "align"(ptr %arraydecay, i64 32) ]

OMPIRBuilder was modified to support this codegen scheme. Attached unit tests 
check if generation of align assumptions is successful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133578

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -996,7 +996,7 @@
 safelen = builder.getInt64(safelenVar.value());
 
   ompBuilder->applySimd(
-  loopInfo,
+  loopInfo, {}, nullptr,
   loop.if_expr() ? moduleTranslation.lookupValue(loop.if_expr()) : nullptr,
   simdlen, safelen);
 
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -1771,7 +1771,8 @@
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(CLI, /* IfCond */ nullptr, /* Simdlen */ nullptr,
+  OMPBuilder.applySimd(CLI, /* AlignedVars */ {}, /* Alignment */ nullptr,
+   /* IfCond */ nullptr, /* Simdlen */ nullptr,
/* Safelen */ nullptr);
 
   OMPBuilder.finalize();
@@ -1797,13 +1798,137 @@
   }));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimdiDefaultAligned) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  IRBuilder<> Builder(BB);
+  AllocaInst *Alloc1 =
+  Builder.CreateAlloca(Builder.getInt8PtrTy(), Builder.getInt64(1));
+  AllocaInst *Alloc2 = Builder.CreateAlloca(
+  ArrayType::get(Builder.getInt32Ty(), 10), Builder.getInt64(1));
+  SmallVector AlignedVars;
+  auto Int8Ty = Builder.getInt8Ty();
+  Instruction *MallocInstr = CallInst::CreateMalloc(
+  Alloc2, Builder.getInt64Ty(), Int8Ty, ConstantExpr::getSizeOf(Int8Ty),
+  Builder.getInt64(400), nullptr, "");
+  Builder.CreateStore(MallocInstr, Alloc1);
+
+  AlignedVars.push_back(Alloc1);
+  AlignedVars.push_back(Alloc2);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(CLI, AlignedVars, /* Alignment */ nullptr,
+   /* IfCond */ nullptr, /* Simdlen */ nullptr,
+   /* Safelen */ nullptr);
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a comment.

hello again, I added two tests, one for llvm and one for clang. how do I run 
those specifically so I see I didnt do something wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133029#3779970 , @ldionne wrote:

> Just my .02, but I am conflicted between:
>
> 1. Simply not doing anything -- the diagnostic users get when they violate 
> the requirement currently is probably not that bad? I did see this breakage a 
> bit in our internal code base as well, but it was easy to fix and there were 
> not many instances.

I think the issue this is solving is that we are not diagnosing the library 
requirement properly because we would miss cases where the incomplete type 
wasn't actually used by the member function (aka, it wasn't caught by any 
constant expression evaluation requirements in the compiler).

> 2. Adding the attribute that was suggested and using it in libc++. On 
> compilers that don't support the attribute, we'd simply be less pedantic.
>
> The one thing I'd rather not do is `static_assert(__is_complete<_Tp>::value)` 
> in all the `std::vector` member functions, IMO that adds complexity and 
> reduces readability for a really marginal gain.

Because this is a libc++ requirement, I will defer to what you think is best 
here. The only danger I see of not diagnosing this is that users may 
potentially run into a portability issue to another STL, which might make sense 
as a clang-tidy warning, but seems like it'd be pretty low-value as I suspect 
most STL implementations rely on the compiler to issue a diagnostic for the 
incomplete type when it matters and so all the implementations are likely to 
behave roughly the same in terms of behavior. So I'm leaning towards the "do 
nothing" option.

I'm not certain an attribute makes a whole lot of sense given that the 
requirement is pretty specific to the STL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133029

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

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133570#3780171 , @OfekShochat 
wrote:

> hello again, I added two tests, one for llvm and one for clang. how do I run 
> those specifically so I see I didnt do something wrong?

The short answer is: you can run the `check-all` to run the LLVM and Clang 
tests together. For more details, our testing documentation can be found at: 
https://llvm.org/docs/TestingGuide.html though we also have some details at: 
https://clang.llvm.org/hacking.html#testing as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80
+ParsedAttributes Attrs(AttrFactory);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+

aaron.ballman wrote:
> beanz wrote:
> > python3kgae wrote:
> > > aaron.ballman wrote:
> > > > Just double-checking, but this allows `[[]]` style attributes as well 
> > > > as `[]` style attributes, but not `__attribute__` or `__declspec` style 
> > > > attributes, is that intended?
> > > That is what dxc currently support.
> > > It may change in the future. But for now, only [[]] and [] are supported.
> > Well... worth noting, HLSL doesn't actually support C++11 attributes, but 
> > that is almost certainly going to change in the near future, so we might as 
> > well support them from the start in Clang.
> Ah, that is good to know @beanz -- we should think carefully about this 
> situation because there are some tradeoffs to consider.
> 
> 1) It's pretty weird to support half of the Microsoft attribute syntax (and 
> the half we barely have any attribute support for, at that). Is there a 
> reason to not support `__declspec` as well? (For example, are there concerns 
> about things like using those attributes to do dllexport or specify a COMDAT 
> section, etc?) In fact, if we're going to support vendor attributes like 
> `[[clang::overloadable]]`, it's a bit weird that we then prohibit the same 
> attribute from being spelled `__attribute__((overloadable))`, is there a 
> particular reason to not extend to all attributes?
> 2) Supporting `[]` style attributes means that attribute order is important. 
> We cannot use `MaybeParseAttributes()` to parse attribute specifiers in any 
> order because `[]` causes ambiguities under some circumstances. So you're 
> stuck with the order you have -- `[[]]` attributes first, `[]` attributes 
> second. Is that ordering reasonable?
> 
> And for the patch itself -- there are no test cases demonstrating what 
> happens when using attributes on things within one of these buffers. I expect 
> many things to be quite reasonable, like using `[[deprecated]]`, but are the 
> attributes which impact codegen reasonable as well? (Like naked functions, 
> returns twice, disable tail calls, etc)
@aaron.ballman I think those are all good questions. Generally HLSL has used 
Microsoft attribute syntax, and I've started extending the Clang support to be 
more robust, but still have more work to do.

More on this patch, I want to take a step back.

I think @python3kgae copied this code from DXC, but I don't think it is ever 
used. I don't think we have any attributes in the language that are valid with 
cbuffer or tbuffer  subjects. We certainly don't have any attributes 
implemented in clang that would be valid on these targets.

That makes me think we should remove since it should be dead and unreachable 
and untestable code.

Since these HLSL buffer decls are an older (although frequently used) HLSL 
feature, I think our general preference is to not extend new feature support to 
them, and instead to encourage users to use the newer buffer types.

Does that sound reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80
+ParsedAttributes Attrs(AttrFactory);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+

beanz wrote:
> aaron.ballman wrote:
> > beanz wrote:
> > > python3kgae wrote:
> > > > aaron.ballman wrote:
> > > > > Just double-checking, but this allows `[[]]` style attributes as well 
> > > > > as `[]` style attributes, but not `__attribute__` or `__declspec` 
> > > > > style attributes, is that intended?
> > > > That is what dxc currently support.
> > > > It may change in the future. But for now, only [[]] and [] are 
> > > > supported.
> > > Well... worth noting, HLSL doesn't actually support C++11 attributes, but 
> > > that is almost certainly going to change in the near future, so we might 
> > > as well support them from the start in Clang.
> > Ah, that is good to know @beanz -- we should think carefully about this 
> > situation because there are some tradeoffs to consider.
> > 
> > 1) It's pretty weird to support half of the Microsoft attribute syntax (and 
> > the half we barely have any attribute support for, at that). Is there a 
> > reason to not support `__declspec` as well? (For example, are there 
> > concerns about things like using those attributes to do dllexport or 
> > specify a COMDAT section, etc?) In fact, if we're going to support vendor 
> > attributes like `[[clang::overloadable]]`, it's a bit weird that we then 
> > prohibit the same attribute from being spelled 
> > `__attribute__((overloadable))`, is there a particular reason to not extend 
> > to all attributes?
> > 2) Supporting `[]` style attributes means that attribute order is 
> > important. We cannot use `MaybeParseAttributes()` to parse attribute 
> > specifiers in any order because `[]` causes ambiguities under some 
> > circumstances. So you're stuck with the order you have -- `[[]]` attributes 
> > first, `[]` attributes second. Is that ordering reasonable?
> > 
> > And for the patch itself -- there are no test cases demonstrating what 
> > happens when using attributes on things within one of these buffers. I 
> > expect many things to be quite reasonable, like using `[[deprecated]]`, but 
> > are the attributes which impact codegen reasonable as well? (Like naked 
> > functions, returns twice, disable tail calls, etc)
> @aaron.ballman I think those are all good questions. Generally HLSL has used 
> Microsoft attribute syntax, and I've started extending the Clang support to 
> be more robust, but still have more work to do.
> 
> More on this patch, I want to take a step back.
> 
> I think @python3kgae copied this code from DXC, but I don't think it is ever 
> used. I don't think we have any attributes in the language that are valid 
> with cbuffer or tbuffer  subjects. We certainly don't have any attributes 
> implemented in clang that would be valid on these targets.
> 
> That makes me think we should remove since it should be dead and unreachable 
> and untestable code.
> 
> Since these HLSL buffer decls are an older (although frequently used) HLSL 
> feature, I think our general preference is to not extend new feature support 
> to them, and instead to encourage users to use the newer buffer types.
> 
> Does that sound reasonable?
> We certainly don't have any attributes implemented in clang that would be 
> valid on these targets.

Despite knowing nothing about HLSL, I feel like pushing back a little bit here: 
deprecated, nodiscard, maybe_unused, and many others seem like they'd not only 
be valid on the target but perhaps useful to users.

> Does that sound reasonable?

I'm totally fine with that approach; we can debate attributes later. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

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

[PATCH] D133194: rewording note note_constexpr_invalid_cast

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 459064.
Codesbyusman edited the summary of this revision.
Codesbyusman added a comment.

updated the test work for the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133194

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/cast.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx20_2b,cxx2b-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx11_20,cxx20_2b -triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion -Wvla 
+
 
 namespace StaticAssertFoldTest {
 
@@ -11,6 +12,10 @@
 
 }
 
+int array[(long)(char *)0]; // expected-warning {{variable length arrays are a C99 feature}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
+
 typedef decltype(sizeof(char)) size_t;
 
 template constexpr T id(const T &t) { return t; }
@@ -2449,6 +2454,6 @@
 
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
+  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}} 
 }
 }
Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -1,4 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -DConstantCast -Wvla -verify %s 
+
+#ifdef ConstantCast
+int array[(long)(char *)0]; // expected-warning {{variable length array used}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{this conversion is not allowed in a constant expression}}
+#endif
 
 typedef struct { unsigned long bits[(((1) + (64) - 1) / (64))]; } cpumask_t;
 cpumask_t x;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7473,12 +7473,14 @@
   }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 0 << Info.Ctx.getLangOpts().CPlusPlus;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
 if (!Info.Ctx.getLangOpts().CPlusPlus20)
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+  CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 1 << Info.Ctx.getLangOpts().CPlusPlus;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitBuiltinBitCastExpr(const BuiltinBitCastExpr *E) {
@@ -8179,7 +8181,8 @@
   return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
 case CK_LValueBitCast:
-  this->CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  this->CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return false;
   Result.Designator.setInvalid();
@@ -8890,9 +8893,10 @@
 Result.Designator.setInvalid();
 if (SubExpr->getType()->isVoid

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 459067.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  lldb/test/API/commands/expression/rdar42038760/main.c
  lldb/test/API/commands/expression/rdar44436068/main.c

Index: lldb/test/API/commands/expression/rdar44436068/main.c
===
--- lldb/test/API/commands/expression/rdar44436068/main.c
+++ lldb/test/API/commands/expression/rdar44436068/main.c
@@ -3,6 +3,6 @@
 __int128_t n = 1;
 n = n + n;
 return n; //%self.expect("p n", substrs=['(__int128_t) $0 = 2'])
-  //%self.expect("p n + 6", substrs=['(__int128) $1 = 8'])
-  //%self.expect("p n + n", substrs=['(__int128) $2 = 4'])
+  //%self.expect("p n + 6", substrs=['(__int128_t) $1 = 8'])
+  //%self.expect("p n + n", substrs=['(__int128_t) $2 = 4'])
 }
Index: lldb/test/API/commands/expression/rdar42038760/main.c
===
--- lldb/test/API/commands/expression/rdar42038760/main.c
+++ lldb/test/API/commands/expression/rdar42038760/main.c
@@ -10,7 +10,7 @@
   struct S0 l_19;
   l_19.f2 = 419;
   uint32_t l_4037 = 4294967295UL;
-  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(unsigned int) $0 = 358717883'])
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(uint32_t) $0 = 358717883'])
 }
 int main()
 {
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/In

[clang] 28bd794 - [clang-format] NFC remove incorrect whitespace causing documentation issue

[via cfe-commits]

Author: mydeveloperday
Date: 2022-09-09T16:03:48+01:00
New Revision: 28bd7945eabdbde2b1fc071ab2f9b78e6e754a1a

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

LOG: [clang-format] NFC remove incorrect whitespace causing documentation issue

Added: 


Modified: 
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index c1e2fd54cd67..818dbe0c89c7 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1775,7 +1775,7 @@ struct FormatStyle {
 /// Change specifiers/qualifiers to be aligned based on ``QualifierOrder``.
 /// With:
 /// \code{.yaml}
-///   QualifierOrder: ['inline', 'static' , 'type', 'const']
+///   QualifierOrder: ['inline', 'static', 'type', 'const']
 /// \endcode
 ///
 /// \code



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

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov edited the summary of this revision.
mizvekov updated this revision to Diff 459070.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  lldb/test/API/commands/expression/rdar42038760/main.c
  lldb/test/API/commands/expression/rdar44436068/main.c

Index: lldb/test/API/commands/expression/rdar44436068/main.c
===
--- lldb/test/API/commands/expression/rdar44436068/main.c
+++ lldb/test/API/commands/expression/rdar44436068/main.c
@@ -3,6 +3,6 @@
 __int128_t n = 1;
 n = n + n;
 return n; //%self.expect("p n", substrs=['(__int128_t) $0 = 2'])
-  //%self.expect("p n + 6", substrs=['(__int128) $1 = 8'])
-  //%self.expect("p n + n", substrs=['(__int128) $2 = 4'])
+  //%self.expect("p n + 6", substrs=['(__int128_t) $1 = 8'])
+  //%self.expect("p n + n", substrs=['(__int128_t) $2 = 4'])
 }
Index: lldb/test/API/commands/expression/rdar42038760/main.c
===
--- lldb/test/API/commands/expression/rdar42038760/main.c
+++ lldb/test/API/commands/expression/rdar42038760/main.c
@@ -10,7 +10,7 @@
   struct S0 l_19;
   l_19.f2 = 419;
   uint32_t l_4037 = 4294967295UL;
-  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(unsigned int) $0 = 358717883'])
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(uint32_t) $0 = 358717883'])
 }
 int main()
 {
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/uadd-overfl

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80
+ParsedAttributes Attrs(AttrFactory);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+

aaron.ballman wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > beanz wrote:
> > > > python3kgae wrote:
> > > > > aaron.ballman wrote:
> > > > > > Just double-checking, but this allows `[[]]` style attributes as 
> > > > > > well as `[]` style attributes, but not `__attribute__` or 
> > > > > > `__declspec` style attributes, is that intended?
> > > > > That is what dxc currently support.
> > > > > It may change in the future. But for now, only [[]] and [] are 
> > > > > supported.
> > > > Well... worth noting, HLSL doesn't actually support C++11 attributes, 
> > > > but that is almost certainly going to change in the near future, so we 
> > > > might as well support them from the start in Clang.
> > > Ah, that is good to know @beanz -- we should think carefully about this 
> > > situation because there are some tradeoffs to consider.
> > > 
> > > 1) It's pretty weird to support half of the Microsoft attribute syntax 
> > > (and the half we barely have any attribute support for, at that). Is 
> > > there a reason to not support `__declspec` as well? (For example, are 
> > > there concerns about things like using those attributes to do dllexport 
> > > or specify a COMDAT section, etc?) In fact, if we're going to support 
> > > vendor attributes like `[[clang::overloadable]]`, it's a bit weird that 
> > > we then prohibit the same attribute from being spelled 
> > > `__attribute__((overloadable))`, is there a particular reason to not 
> > > extend to all attributes?
> > > 2) Supporting `[]` style attributes means that attribute order is 
> > > important. We cannot use `MaybeParseAttributes()` to parse attribute 
> > > specifiers in any order because `[]` causes ambiguities under some 
> > > circumstances. So you're stuck with the order you have -- `[[]]` 
> > > attributes first, `[]` attributes second. Is that ordering reasonable?
> > > 
> > > And for the patch itself -- there are no test cases demonstrating what 
> > > happens when using attributes on things within one of these buffers. I 
> > > expect many things to be quite reasonable, like using `[[deprecated]]`, 
> > > but are the attributes which impact codegen reasonable as well? (Like 
> > > naked functions, returns twice, disable tail calls, etc)
> > @aaron.ballman I think those are all good questions. Generally HLSL has 
> > used Microsoft attribute syntax, and I've started extending the Clang 
> > support to be more robust, but still have more work to do.
> > 
> > More on this patch, I want to take a step back.
> > 
> > I think @python3kgae copied this code from DXC, but I don't think it is 
> > ever used. I don't think we have any attributes in the language that are 
> > valid with cbuffer or tbuffer  subjects. We certainly don't have any 
> > attributes implemented in clang that would be valid on these targets.
> > 
> > That makes me think we should remove since it should be dead and 
> > unreachable and untestable code.
> > 
> > Since these HLSL buffer decls are an older (although frequently used) HLSL 
> > feature, I think our general preference is to not extend new feature 
> > support to them, and instead to encourage users to use the newer buffer 
> > types.
> > 
> > Does that sound reasonable?
> > We certainly don't have any attributes implemented in clang that would be 
> > valid on these targets.
> 
> Despite knowing nothing about HLSL, I feel like pushing back a little bit 
> here: deprecated, nodiscard, maybe_unused, and many others seem like they'd 
> not only be valid on the target but perhaps useful to users.
> 
> > Does that sound reasonable?
> 
> I'm totally fine with that approach; we can debate attributes later. :-)
> Despite knowing nothing about HLSL, I feel like pushing back a little bit 
> here: deprecated, nodiscard, maybe_unused, and many others seem like they'd 
> not only be valid on the target but perhaps useful to users.

Okay... you got me here. I hadn't considered `deprecated` but can see a use for 
it. I don't think the other two apply, but I'll concede there may be more 
general clang attributes that do have uses.

If we can postpone this discussion though I think we can do some background and 
get a better feeling for what attributes we should and shouldn't support, and 
maybe consider the syntax a bit carefully too.

If I'm reading this correctly the DXC-supported syntax is:

```
cbuffer A { ... } [some_attribute]
```

(note: DXC doesn't really support CXX11 attributes, just the MS syntax)

If this syntax is really unreachable in DXC (which I believe it is), it might 
be better to shift the syntax to be more like C++ class and struct attributes:

```
[[some_attribute]]
cbuffer A {...}
```

I think that would be more familiar and understandable to users, especially as 
buffer 

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 459074.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.

- rebase with fixed doc NFC update
- rework the nofallback condition to prevent repeated checks
- fix grammar checks


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

https://reviews.llvm.org/D133571

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2882,7 +2882,7 @@
 
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   verifyFormat("switch (a) {\n"
"case 1: x = 1; break;\n"
"case 2: return;\n"
@@ -3011,7 +3011,7 @@
"}",
Style);
   Style.ColumnLimit = 80;
-  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   Style.IndentCaseLabels = true;
   EXPECT_EQ("switch (n) {\n"
 "  default /*comments*/:\n"
@@ -3026,7 +3026,7 @@
"  return false;\n"
"}",
Style));
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
@@ -3051,6 +3051,55 @@
"  }\n"
"}",
Style));
+
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_NoFallThrough;
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 1: x = 1; break;\n"
+   "  case 2: return;\n"
+   "  case 3:\n"
+   "  case 4:\n"
+   "  case 5:\n"
+   "return;\n"
+   "  case 6: // comment\n"
+   "return;\n"
+   "  case 7:\n"
+   "// comment\n"
+   "return;\n"
+   "  case 8:\n"
+   "x = 8; // comment\n"
+   "break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
+
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1:\n"
+   "x = 1;\n"
+   "break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3:\n"
+   "x = 3;\n"
+   "[[fallthrough]];\n"
+   "  case 4:\n"
+   "x = 4;\n"
+   "break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  verifyFormat("switch (a)\n"
+   "{\n"
+   "  case 0:\n"
+   "  case 1: x = 1; break;\n"
+   "  case 2: x = 2; break;\n"
+   "  case 3: x = 3; [[fallthrough]];\n"
+   "  case 4: x = 4; break;\n"
+   "  default: y = 1; break;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsLabels) {
@@ -20148,7 +20197,6 @@
   CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -20509,6 +20557,20 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Never",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Always",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: NoFallThrough",
+  AllowShortCaseLabelsOnASingleLine,
+  FormatStyle::SCLS_NoFallThrough);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: false",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: true",
+  AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+
   Style.SpaceAroundPointerQualifiers = Forma

[PATCH] D133088: [Clang] Fix wrong diagnostic for scope identifier with internal linkage

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

gentle ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133088

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

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1091-1150
+QualType fpTy = ComplexTy->castAs()->getElementType();
 IntExpr = S.ImpCastExprToType(IntExpr.get(), fpTy, CK_IntegralToFloating);
 IntExpr = S.ImpCastExprToType(IntExpr.get(), ComplexTy,
   CK_FloatingRealToComplex);
   } else {
 assert(IntTy->isComplexIntegerType());
 IntExpr = S.ImpCastExprToType(IntExpr.get(), ComplexTy,

@shafik let's continue revision of D133522 in this patch, since that one was a 
quick fix that I attempted in order to avoid reverting it. But now that it was 
reverted, I will abandon the other one.

I have highlighted with this comment the areas affected by D133522 so we can 
continue here.

But now I have refactored the whole thing to be simpler / more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

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

[PATCH] D133522: [clang] Fix Complex x Float x Int conversions so it handles type sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov abandoned this revision.
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1135
+QualType RHSElementType =
+RHSComplexType ? RHSComplexType->getElementType() : RHSType;
+QualType ResultType =

shafik wrote:
> Can you collapse this check with the next line? It just looks weird checking 
> `RHSComplexType` twice and having `RHSType` on the opposite side of each one. 
> It took me a while to understand what was going on.
> 
> This comment also applies a few lines down.
I have refactored the whole thing, but at D111509

The present patch was intended to be a quick fix to that one, to avoid 
reverting it, but that is moot now since it ended up reverted and we can't test 
what is in here without the other one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133522

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

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D111509#3778883 , @MaskRay wrote:

> Sorry, I just reverted it:) You may check whether the fix fixes GCC 
> libstdc++-v3/src/c++98/complex_io.cc

I confirm it does not crash on my system with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

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

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin created this revision.
yronglin added reviewers: rjmccall, rsmith.
Herald added a project: All.
yronglin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix __builtin_assume_aligned incorrect type descriptor

example from @rsmith

  struct A { int n; };
  struct B { int n; };
  struct C : A, B {};
  
  void *f(C *c) {
// Incorrectly returns `c` rather than the address of the B base class.
return __builtin_assume_aligned((B*)c, 8);
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133583

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Basic/Builtins.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-redecl.cpp


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << Call->getSourceRange();
 }
 
+/// Checks that a call expression's argument count is in the desired range. 
This
+/// is useful when doing custom type-checking on a variadic function. Returns
+/// true on error.
+static bool checkArgCountRange(Sema &S, CallExpr *Call, unsigned MinArgCount,
+   unsigned MaxArgCount) {
+  return checkArgCountAtLeast(S, Call, MinArgCount) ||
+ checkArgCountAtMost(S, Call, MaxArgCount);
+}
+
 /// Checks that a call expression's argument count is the desired number.
 /// This is useful when doing custom type-checking.  Returns true on error.
 static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) {
@@ -7643,17 +7652,15 @@
 /// Handle __builtin_assume_aligned. This is declared
 /// as (const void*, size_t, ...) and can take one optional constant int arg.
 bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) {
-  if (checkArgCountAtMost(*this, TheCall, 3))
+  if (checkArgCountRange(*this, TheCall, 2, 3))
 return true;
 
   unsigned NumArgs = TheCall->getNumArgs();
   Expr *FirstArg = TheCall->getArg(0);
-  if (auto *CE = dyn_cast(FirstArg))
-FirstArg = CE->getSubExprAsWritten();
-
+  
   {
 ExprResult FirstArgResult =
-DefaultFunctionArrayLvalueConversion(FirstArg, /*Diagnose=*/false);
+DefaultFunctionArrayLvalueConversion(FirstArg);
 if (FirstArgResult.isInvalid())
   return true;
 TheCall->setArg(0, FirstArgResult.get());
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -209,6 +209,7 @@
 
 bool Builtin::Context::canBeRedeclared(unsigned ID) const {
   return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start ||
+ ID == Builtin::BI__builtin_assume_aligned ||
  (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) ||
  isInStdNamespace(ID);
 }
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -546,7 +546,7 @@
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
 BUILTIN(__builtin_stdarg_start, "vA.", "nt")
-BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
+BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nct")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaC

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D133425#3779121 , @dblaikie wrote:

>> One thing I don't understand in the current state of things is why the 
>> diagnostic fires at all inside system headers. I thought warnings in system 
>> headers were discarded?
>
> It doesn't fire if the location of the diagnostic (the place that's using 
> CTAD) is in a system header. But it does fire if that place is outside a 
> system header, but the template that is being used is in a system header (see 
> the SysHeaderObj example above - maybe the naming is confusing? It's a 
> `SysHeaderClass` but the object (`sho`) is not in a system header).

Oh, right, so my confusion was about point-of-definition vs point-of-use.

>> FWIW, my "objection" that we should not silence the warning when users try 
>> using CTAD with arbitrary types in `std::` remains -- I think it would be 
>> making a disservice to users to let them use CTAD with classes that have not 
>> been designed with that in mind. At the end of the day, I think I'm 
>> advocating for individual classes opting-out of the warning, while the patch 
>> as currently formulated forces all classes in system headers to be opted-out 
>> of the warning.
>
> I think the problem is that not all system headers match the style required 
> for this constraint - so in the interests of compatibility with the complex 
> world of existing system headers, it's suitable not to enforce this stylistic 
> constraint (requiring explicit deduction guides even when the default is what 
> the template author intended to allow CTAD) on system headers. With the 
> knowledge that this does introduce false negatives, but that we don't have a 
> strong enough signal to avoid them.

I think this might come down to a difference in opinion here. Personally, I'd 
rather restrict the use of CTAD only to the small subset of classes that are 
meant to work with it, and warn everywhere else. And I'm fine if a system 
library has to jump through a few hoops in order to avoid warning on a class 
that does work well with CTAD. Allowing users to use CTAD on a class is like a 
contract given to them, and the fact that this contract is implicitly assumed 
by the language is often cited as a questionable design decision in C++17. I 
think this warning is useful and if the special markup had been there on the 
few classes that need it (like `lock_guard`), we wouldn't be having that 
discussion at all because I don't think anybody actually wants to use CTAD with 
random types in a system library.

In particular, the warning is useful in general user code, and I don't really 
follow why the fact that a type is authored in a system library would suddenly 
make it more okay to use with CTAD. Types in system libraries are not different 
from user types w.r.t. CTAD -- several types are just not designed with CTAD in 
mind.

Finally, I'd like to note that this warning only triggers if the user 
explicitly passes `-Wctad-maybe-unsupported`. It's not enabled by `-Wall` or 
`-Wextra`. I would argue that users that go out of their way to enable this 
warning probably share my sentiment that CTAD is sometimes harmful, and they 
probably want to be warned about questionable uses even for classes defined in 
a system library.

Anyway, I think I've made my opinion clear and I don't think I have much more 
to bring to the table. I'll ship D133535  
regardless, but if this patch ships, I think we should have some way of 
enabling the warning for types in system headers. For example, folks using 
`-Wctad-maybe-unsupported` should really be warned about using CTAD on a type 
like `std::reverse_iterator`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133425

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

[PATCH] D133354: [Clang]: Diagnose deprecated copy operations also in MSVC compatibility mode

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D133354#3779882 , @ningvin wrote:

> In D133354#3779123 , @dblaikie 
> wrote:
>
>> That works, you can also take the hash with the `rG` prefix and use that: 
>> rGd577fbbd1c9d6dab193d530fcd807efc3b3bc9ad 
>>  and 
>> that works too.
>
> Didn't know that, thanks!
>
> In D133354#3779765 , @hans wrote:
>
>> For more context, this was initially raised in 
>> https://discourse.llvm.org/t/why-is-the-deprecated-copy-warning-suppressed-in-msvc-compatibility-mode/65085/1
>
> Oh yes sorry, I should have linked to that.

ah, *thumbs up* then, though I'll probably leave it to @hansw to review so he's 
aware of when this goes in/will have context in case any fallout is observed on 
the Windows side of things


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133354

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

[clang] 493766e - Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via cfe-commits]

Author: Steven Wu
Date: 2022-09-09T08:57:12-07:00
New Revision: 493766e068474a80a790ac41c667061229d2262d

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

LOG: Frontend: Respect -working-directory when checking if output files can be 
written

Call `FixupRelativePath` when opening output files to ensure that
`-working-directory` is used when checking up front for write failures,
not just when finalizing the files at the end. This also moves the
temporary file into the same directory as the output file.

Reviewed By: benlangmuir

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

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp
clang/test/Frontend/output-paths.c

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index b187bdc044c08..995c94bb6912b 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@ void CompilerInstance::clearOutputFiles(bool EraseFiles) {
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@ CompilerInstance::createOutputFileImpl(StringRef 
OutputPath, bool Binary,
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary 
files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 

diff  --git a/clang/test/Frontend/output-paths.c 
b/clang/test/Frontend/output-paths.c
index 5b0cbb4d77cc4..836fe971de5e8 100644
--- a/clang/test/Frontend/output-paths.c
+++ b/clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file 
'{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o 
%basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1



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

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG493766e06847: Frontend: Respect -working-directory when 
checking if output files can be… (authored by steven_wu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95497

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Frontend/output-paths.c


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file 
'{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o 
%basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary 
files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file '{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o %basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626
+  VisitDeclaratorDecl(D);
+  Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName());
+  Record.push_back(D->getIdentifierNamespace());
+

ChuanqiXu wrote:
> mizvekov wrote:
> > ChuanqiXu wrote:
> > > ChuanqiXu wrote:
> > > > ChuanqiXu wrote:
> > > > > mizvekov wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > I still don't get the reason for the move. What's the benefit? Or 
> > > > > > > why is it necessary?
> > > > > > Yeah, now the type can reference the template decl, so without 
> > > > > > moving this, it can happen during import of the type that we try to 
> > > > > > read this function template bits without having imported them yet.
> > > > > Oh, I guess I met the problem before (D129748 ) and I made a 
> > > > > workaround for it (https://reviews.llvm.org/D130331). If I understood 
> > > > > right, the patch will solve that problem. I'll check it out later.
> > > > > 
> > > > > (This kind of code move looks dangerous you know and I'll take a 
> > > > > double check)
> > > > After looking into the detailed change for the serialization part, I 
> > > > feel it is a not-so-good workaround indeed.. It looks like we need a 
> > > > better method to delay reading the type in the serializer. And I'm 
> > > > looking at it. @mizvekov would you like to rebase the series of patches 
> > > > to the main branch so that I can test it actually.
> > > Or would it be simpler to rebase and squash them into a draft revision?
> > I had given this some thought, and it made sense to me that we should deal 
> > with the template bits first, since these are closer to the introducer for 
> > these declarations, and so that it would be harder to have a dependence the 
> > other way around.
> > 
> > But I would like to hear your thoughts on this after you have taken a 
> > better look.
> > I am working on a bunch of things right now, I should be able to rebase 
> > this on the next few days, but otherwise
> > I last rebased about 4 days ago, so you can also check that out at 
> > https://github.com/mizvekov/llvm-project/tree/resugar
> > That link has the whole stack, you probably should check out just the 
> > commit for this patch, as you are probably going to encounter issues with 
> > the resugarer if you try it on substantial code bases.
> > It will carry other changes with it, but I think those should be safe.
> I won't say it is bad to deal with template bits first. I just feel it is a 
> workaround to avoid the circular dependent problem in deserialization. Or in 
> another word, here the method works due to you put some decls* in the 
> template parameter types. And we avoid the circular dependent problem by 
> adjusting the order we deserializes. The reasons why I don't feel it is good 
> include:
> (1) Although we touched template function decl and template var decl, we 
> don't touched template var decl. I guess it'll be a problem now or later.
> (2) The solution here can't solve the similar circular dependent problem I 
> sawed in attributes. So the method only workarounds some resulting of the 
> same problem.
> 
> Or in one shorter explanation, it should be greater to solve the root 
> problems. I have an idea and I am going to to do a proof-of-concept 
> implementation first since I feel like nobody are happy about an 
> unimplementable idea. Generally I don't like to block patches due to such 
> reasons since it is completely not your fault but I guess it may be better to 
> wait some time. Since if we want to allow workarounds first and clear the 
> workarounds, things will be harder. If you want a timeline, I guess 2 months 
> may be reasonable choices. I mean if I can't make it in 2 months and other 
> reviewers feel this is good (what I am seeing), I feel bad to block this. 
> (But if we're more patient, it'll be better). How do you think about this?
Well we touch FunctionTemplates and VariableTemplates in this patch, because 
they were not doing template first.
For whatever reason, class templates were already doing template first, so no 
need to fix that.

So this patch at least puts that into consistency.

Also, this patch is a pre-requisite for the template resugaring specialization 
project I am working on, and the deadline for the whole project is about two 
months from now.

If I leave merging this patch until the end, it seems impossible that I will 
finish in time, as we will leave field testing this to the very end.

So while I understand the need for a better approach, it is indeed putting me 
in an impossible situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 459090.
python3kgae added a comment.

Remove attribute for first cbuffer commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/IdentifierResolver.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaHLSL.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/HLSL/Inputs/empty.hlsl
  clang/test/AST/HLSL/ast-dump-comment-cbuffe-tbufferr.hlsl
  clang/test/AST/HLSL/cbuffer_tbuffer.hlsl
  clang/test/AST/HLSL/pch_hlsl_buffer.hlsl
  clang/test/ParserHLSL/cb_error.hlsl
  clang/test/ParserHLSL/invalid_inside_cb.hlsl
  clang/test/SemaHLSL/cb_error.hlsl

Index: clang/test/SemaHLSL/cb_error.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/cb_error.hlsl
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-note@+1 {{declared here}}
+cbuffer a {
+  int x;
+};
+
+int foo() {
+  // expected-error@+1 {{'a' does not refer to a value}}
+  return sizeof(a);
+}
+
+// expected-error@+1 {{expected unqualified-id}}
+template  cbuffer a { Ty f; };
+
+// For back-compat reason, it is OK for multiple cbuffer/tbuffer use same name in hlsl.
+// And these cbuffer name only used for reflection, cannot be removed.
+cbuffer A {
+  float A;
+}
+
+cbuffer A {
+  float b;
+}
+
+tbuffer A {
+  float a;
+}
+
+float bar() {
+  // cbuffer/tbuffer name will not conflict with other variables.
+  return A;
+}
+
+cbuffer a {
+  // expected-error@+2 {{unknown type name 'oh'}}
+  // expected-error@+1 {{expected ';' after top level declarator}}
+  oh no!
+  // expected-warning@+1 {{missing terminating ' character}}
+  this isn't even valid HLSL code
+  despite seeming totally reasonable
+  once you understand that HLSL
+  is so flaming weird.
+}
+
+tbuffer B {
+  // expected-error@+1 {{unknown type name 'flaot'}}
+  flaot f;
+}
Index: clang/test/ParserHLSL/invalid_inside_cb.hlsl
===
--- /dev/null
+++ clang/test/ParserHLSL/invalid_inside_cb.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// template not allowed inside cbuffer.
+cbuffer A {
+// expected-error@+2 {{invalid declaration inside cbuffer}}
+template
+T foo(T t) { return t;}
+}
+
+cbuffer A {
+// expected-error@+2 {{invalid declaration inside cbuffer}}
+template
+struct S { float s;};
+}
+
+// typealias not allowed inside cbuffer.
+cbuffer A {
+// expected-error@+2 {{invalid declaration inside cbuffer}}
+// expected-warning@+1 {{alias declarations are a C++11 extension}}
+using F32 = float;
+}
Index: clang/test/ParserHLSL/cb_error.hlsl
===
--- /dev/null
+++ clang/test/ParserHLSL/cb_error.hlsl
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-error@+2 {{expected identifier}}
+// expected-error@+1 {{expected unqualified-id}}
+cbuffer { ... };
+// expected-error@+1 {{expected '{'}}
+cbuffer missing_definition;
+// expected-error@+1 {{expected unqualified-id}}
+int cbuffer;
+// expected-error@+1 {{expected identifier}}
+cbuffer;
+
+// expected-error@+2 {{expected identifier}}
+// expected-error@+1 {{expected unqualified-id}}
+tbuffer { ... };
+// expected-error@+1 {{expected '{'}}
+tbuffer missing_definition;
+// expected-error@+1 {{expected unqualified-id}}
+int tbuffer;
+// expected-error@+1 {{expected identifier}}
+tbuffer;
+
+// expected-error@+1 {{expected unqualified-id}}
+cbuffer A {}, B{}
+
+// cbuffer inside namespace is supported.
+namespace N {
+  cbuffer A {
+float g;
+  }
+}
+
+cbuffer A {
+  // expected-error@+1 {{invalid declaration inside cbuffer}}
+  namespace N {
+  }
+}
+
+cbuffer A {
+  // 

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Hi, should I both fix alignment in this patch or in another separate patch? 
(this seems have different behavior with gcc https://godbolt.org/z/7dvM8zhnh )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

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

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/www/hacking.html:295-296
+  directory will cause the update of the diff to start a CI run. This dummy
+  file will also add the libc++ group to the list of reviewers. The status of
+  the build will be available in Phabricator.
+

ldionne wrote:
> Mordante wrote:
> > philnik wrote:
> > > aaron.ballman wrote:
> > > > I am guessing the dummy file should then be removed before landing the 
> > > > commit and we should document that?
> > > > 
> > > > (FWIW, adding a dummy file feels really unclean as a design -- it's 
> > > > mysterious behavior for new contributors, which the documentation helps 
> > > > with a bit, but also it's a risk for checking in files that are never 
> > > > intended to be in the tree. If there's a way to improve this somehow so 
> > > > we don't need to trick the precommit CI into running, that would be 
> > > > really nice and totally outside of the scope of this review.)
> > > It would be great if just the bootstrapping build could be run on all 
> > > clang reviews. That should show most problems with changes in clang. If 
> > > there are any problems in libc++, fixing them would result in a full CI 
> > > run. Having libc++ run against the patch would probably also be awesome 
> > > as a smoke test for any clang changes.
> > +1 to what @ldionne said.
> > 
> > One of the advantages would be that it's possible to test Clang against the 
> > libc++ code base using different language versions, by defining multiple 
> > test runs using different language versions. (Slightly related to 
> > https://discourse.llvm.org/t/lit-run-a-run-line-multiple-times-with-different-replacements/64932)
> >  @aaron.ballman is looking at extending the Clang pre-commit CI something 
> > worth looking into?
> > 
> > 
> I agree. Let's:
> 
> 1. Setup a `buildkite-pipeline.yml` file that controls the jobs done for 
> Clang. I think right now this is hardcoded somewhere in 
> https://github.com/google/llvm-premerge-checks
> 2. Simply add `LLVM_ENABLE_RUNTIMES=libcxx` to the config that Clang uses so 
> they'll automatically build libc++ with the Clang that was just built anyway. 
> That would run on Clang's own agents that are used for Clang tests, not on 
> the libc++ CI agents. We can adjust if running the libc++ tests causes too 
> much strain on the infrastructure running Clang CI.
> 
> That way, a basic bootstrapping build of libc++ will be run on every Clang 
> change, and TBH I think all of this added documentation for Clang folks to 
> test libc++ changes can simply go away. There really wouldn't be a reason to 
> ever add a `.DELETE_ME` file to libc++ for a Clang patch anymore.
This sounds like a good approach to me. This might be a bit tangential, but if 
`libcxx` is present in `LLVM_ENABLE_RUNTIMES`, could we also have the 
`check-all` target depend on `check-libcxx`? That would help ensure that 
developers run libc++ tests locally before they hit the CI infrastructure. 
Likewise for `compiler-rt` and `libcxxabi` and the `check-runtimes` target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

I've a new patch D133583  , please can you 
guys take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80
+ParsedAttributes Attrs(AttrFactory);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+

beanz wrote:
> aaron.ballman wrote:
> > beanz wrote:
> > > aaron.ballman wrote:
> > > > beanz wrote:
> > > > > python3kgae wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Just double-checking, but this allows `[[]]` style attributes as 
> > > > > > > well as `[]` style attributes, but not `__attribute__` or 
> > > > > > > `__declspec` style attributes, is that intended?
> > > > > > That is what dxc currently support.
> > > > > > It may change in the future. But for now, only [[]] and [] are 
> > > > > > supported.
> > > > > Well... worth noting, HLSL doesn't actually support C++11 attributes, 
> > > > > but that is almost certainly going to change in the near future, so 
> > > > > we might as well support them from the start in Clang.
> > > > Ah, that is good to know @beanz -- we should think carefully about this 
> > > > situation because there are some tradeoffs to consider.
> > > > 
> > > > 1) It's pretty weird to support half of the Microsoft attribute syntax 
> > > > (and the half we barely have any attribute support for, at that). Is 
> > > > there a reason to not support `__declspec` as well? (For example, are 
> > > > there concerns about things like using those attributes to do dllexport 
> > > > or specify a COMDAT section, etc?) In fact, if we're going to support 
> > > > vendor attributes like `[[clang::overloadable]]`, it's a bit weird that 
> > > > we then prohibit the same attribute from being spelled 
> > > > `__attribute__((overloadable))`, is there a particular reason to not 
> > > > extend to all attributes?
> > > > 2) Supporting `[]` style attributes means that attribute order is 
> > > > important. We cannot use `MaybeParseAttributes()` to parse attribute 
> > > > specifiers in any order because `[]` causes ambiguities under some 
> > > > circumstances. So you're stuck with the order you have -- `[[]]` 
> > > > attributes first, `[]` attributes second. Is that ordering reasonable?
> > > > 
> > > > And for the patch itself -- there are no test cases demonstrating what 
> > > > happens when using attributes on things within one of these buffers. I 
> > > > expect many things to be quite reasonable, like using `[[deprecated]]`, 
> > > > but are the attributes which impact codegen reasonable as well? (Like 
> > > > naked functions, returns twice, disable tail calls, etc)
> > > @aaron.ballman I think those are all good questions. Generally HLSL has 
> > > used Microsoft attribute syntax, and I've started extending the Clang 
> > > support to be more robust, but still have more work to do.
> > > 
> > > More on this patch, I want to take a step back.
> > > 
> > > I think @python3kgae copied this code from DXC, but I don't think it is 
> > > ever used. I don't think we have any attributes in the language that are 
> > > valid with cbuffer or tbuffer  subjects. We certainly don't have any 
> > > attributes implemented in clang that would be valid on these targets.
> > > 
> > > That makes me think we should remove since it should be dead and 
> > > unreachable and untestable code.
> > > 
> > > Since these HLSL buffer decls are an older (although frequently used) 
> > > HLSL feature, I think our general preference is to not extend new feature 
> > > support to them, and instead to encourage users to use the newer buffer 
> > > types.
> > > 
> > > Does that sound reasonable?
> > > We certainly don't have any attributes implemented in clang that would be 
> > > valid on these targets.
> > 
> > Despite knowing nothing about HLSL, I feel like pushing back a little bit 
> > here: deprecated, nodiscard, maybe_unused, and many others seem like they'd 
> > not only be valid on the target but perhaps useful to users.
> > 
> > > Does that sound reasonable?
> > 
> > I'm totally fine with that approach; we can debate attributes later. :-)
> > Despite knowing nothing about HLSL, I feel like pushing back a little bit 
> > here: deprecated, nodiscard, maybe_unused, and many others seem like they'd 
> > not only be valid on the target but perhaps useful to users.
> 
> Okay... you got me here. I hadn't considered `deprecated` but can see a use 
> for it. I don't think the other two apply, but I'll concede there may be more 
> general clang attributes that do have uses.
> 
> If we can postpone this discussion though I think we can do some background 
> and get a better feeling for what attributes we should and shouldn't support, 
> and maybe consider the syntax a bit carefully too.
> 
> If I'm reading this correctly the DXC-supported syntax is:
> 
> ```
> cbuffer A { ... } [some_attribute]
> ```
> 
> (note: DXC doesn't really support CXX11 attributes, just the MS syntax)
> 
> If this syntax is really unreachable in DXC (which I believe it is), it might 
> be better to

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 459092.
yronglin added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Basic/Builtins.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-redecl.cpp


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << Call->getSourceRange();
 }
 
+/// Checks that a call expression's argument count is in the desired range. 
This
+/// is useful when doing custom type-checking on a variadic function. Returns
+/// true on error.
+static bool checkArgCountRange(Sema &S, CallExpr *Call, unsigned MinArgCount,
+   unsigned MaxArgCount) {
+  return checkArgCountAtLeast(S, Call, MinArgCount) ||
+ checkArgCountAtMost(S, Call, MaxArgCount);
+}
+
 /// Checks that a call expression's argument count is the desired number.
 /// This is useful when doing custom type-checking.  Returns true on error.
 static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) {
@@ -7643,17 +7652,15 @@
 /// Handle __builtin_assume_aligned. This is declared
 /// as (const void*, size_t, ...) and can take one optional constant int arg.
 bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) {
-  if (checkArgCountAtMost(*this, TheCall, 3))
+  if (checkArgCountRange(*this, TheCall, 2, 3))
 return true;
 
   unsigned NumArgs = TheCall->getNumArgs();
   Expr *FirstArg = TheCall->getArg(0);
-  if (auto *CE = dyn_cast(FirstArg))
-FirstArg = CE->getSubExprAsWritten();
 
   {
 ExprResult FirstArgResult =
-DefaultFunctionArrayLvalueConversion(FirstArg, /*Diagnose=*/false);
+DefaultFunctionArrayLvalueConversion(FirstArg);
 if (FirstArgResult.isInvalid())
   return true;
 TheCall->setArg(0, FirstArgResult.get());
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -209,6 +209,7 @@
 
 bool Builtin::Context::canBeRedeclared(unsigned ID) const {
   return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start ||
+ ID == Builtin::BI__builtin_assume_aligned ||
  (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) ||
  isInStdNamespace(ID);
 }
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -546,7 +546,7 @@
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
 BUILTIN(__builtin_stdarg_start, "vA.", "nt")
-BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
+BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nct")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << Call->getSourceRange();
 }
 
+/// Checks that a call expression's argument count is in the desired range. This
+/// is useful when doing custom type-checking on a variadic function. Returns
+/// true on error.
+static bool checkArgCountRange(Sema &S, CallExpr *Call, unsigned MinArgCount,
+   unsi

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Markus Böck via Phabricator via cfe-commits]

zero9178 added inline comments.



Comment at: clang/www/hacking.html:295-296
+  directory will cause the update of the diff to start a CI run. This dummy
+  file will also add the libc++ group to the list of reviewers. The status of
+  the build will be available in Phabricator.
+

tahonermann wrote:
> ldionne wrote:
> > Mordante wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > I am guessing the dummy file should then be removed before landing 
> > > > > the commit and we should document that?
> > > > > 
> > > > > (FWIW, adding a dummy file feels really unclean as a design -- it's 
> > > > > mysterious behavior for new contributors, which the documentation 
> > > > > helps with a bit, but also it's a risk for checking in files that are 
> > > > > never intended to be in the tree. If there's a way to improve this 
> > > > > somehow so we don't need to trick the precommit CI into running, that 
> > > > > would be really nice and totally outside of the scope of this review.)
> > > > It would be great if just the bootstrapping build could be run on all 
> > > > clang reviews. That should show most problems with changes in clang. If 
> > > > there are any problems in libc++, fixing them would result in a full CI 
> > > > run. Having libc++ run against the patch would probably also be awesome 
> > > > as a smoke test for any clang changes.
> > > +1 to what @ldionne said.
> > > 
> > > One of the advantages would be that it's possible to test Clang against 
> > > the libc++ code base using different language versions, by defining 
> > > multiple test runs using different language versions. (Slightly related 
> > > to 
> > > https://discourse.llvm.org/t/lit-run-a-run-line-multiple-times-with-different-replacements/64932)
> > >  @aaron.ballman is looking at extending the Clang pre-commit CI something 
> > > worth looking into?
> > > 
> > > 
> > I agree. Let's:
> > 
> > 1. Setup a `buildkite-pipeline.yml` file that controls the jobs done for 
> > Clang. I think right now this is hardcoded somewhere in 
> > https://github.com/google/llvm-premerge-checks
> > 2. Simply add `LLVM_ENABLE_RUNTIMES=libcxx` to the config that Clang uses 
> > so they'll automatically build libc++ with the Clang that was just built 
> > anyway. That would run on Clang's own agents that are used for Clang tests, 
> > not on the libc++ CI agents. We can adjust if running the libc++ tests 
> > causes too much strain on the infrastructure running Clang CI.
> > 
> > That way, a basic bootstrapping build of libc++ will be run on every Clang 
> > change, and TBH I think all of this added documentation for Clang folks to 
> > test libc++ changes can simply go away. There really wouldn't be a reason 
> > to ever add a `.DELETE_ME` file to libc++ for a Clang patch anymore.
> This sounds like a good approach to me. This might be a bit tangential, but 
> if `libcxx` is present in `LLVM_ENABLE_RUNTIMES`, could we also have the 
> `check-all` target depend on `check-libcxx`? That would help ensure that 
> developers run libc++ tests locally before they hit the CI infrastructure. 
> Likewise for `compiler-rt` and `libcxxabi` and the `check-runtimes` target.
This is currently being implemented by https://reviews.llvm.org/D132438 and 
landed before but had to be reverted unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D133425#3780435 , @ldionne wrote:

> In D133425#3779121 , @dblaikie 
> wrote:
>
>>> One thing I don't understand in the current state of things is why the 
>>> diagnostic fires at all inside system headers. I thought warnings in system 
>>> headers were discarded?
>>
>> It doesn't fire if the location of the diagnostic (the place that's using 
>> CTAD) is in a system header. But it does fire if that place is outside a 
>> system header, but the template that is being used is in a system header 
>> (see the SysHeaderObj example above - maybe the naming is confusing? It's a 
>> `SysHeaderClass` but the object (`sho`) is not in a system header).
>
> Oh, right, so my confusion was about point-of-definition vs point-of-use.
>
>>> FWIW, my "objection" that we should not silence the warning when users try 
>>> using CTAD with arbitrary types in `std::` remains -- I think it would be 
>>> making a disservice to users to let them use CTAD with classes that have 
>>> not been designed with that in mind. At the end of the day, I think I'm 
>>> advocating for individual classes opting-out of the warning, while the 
>>> patch as currently formulated forces all classes in system headers to be 
>>> opted-out of the warning.
>>
>> I think the problem is that not all system headers match the style required 
>> for this constraint - so in the interests of compatibility with the complex 
>> world of existing system headers, it's suitable not to enforce this 
>> stylistic constraint (requiring explicit deduction guides even when the 
>> default is what the template author intended to allow CTAD) on system 
>> headers. With the knowledge that this does introduce false negatives, but 
>> that we don't have a strong enough signal to avoid them.
>
> I think this might come down to a difference in opinion here. Personally, I'd 
> rather restrict the use of CTAD only to the small subset of classes that are 
> meant to work with it, and warn everywhere else. And I'm fine if a system 
> library has to jump through a few hoops in order to avoid warning on a class 
> that does work well with CTAD.

But that's the issue, and generally why warnings are suppressed in system 
headers - as a user, you can't fix the system headers you depend on. So it 
usually ends up as a "the user can't use this feature with -Werror/the warning 
enabled, so to write valid code they turn off or ignore the warning".

> Allowing users to use CTAD on a class is like a contract given to them, and 
> the fact that this contract is implicitly assumed by the language is often 
> cited as a questionable design decision in C++17. I think this warning is 
> useful and if the special markup had been there on the few classes that need 
> it (like `lock_guard`), we wouldn't be having that discussion at all because 
> I don't think anybody actually wants to use CTAD with random types in a 
> system library.
>
> In particular, the warning is useful in general user code, and I don't really 
> follow why the fact that a type is authored in a system library would 
> suddenly make it more okay to use with CTAD.

It's more an argument from false positives in the warning because libraries 
outside your control may not adhere to this coding convention (which it is - 
like parentheses around assignment in an if condition, it's a somewhat 
arbitrarily chosen syntax intended to communicate explicit intent rather than 
the implicit default - but it's something all the authors need to agree on, not 
something that would be known independently by the author of a library/happen 
to be just general good practice when writing code in the absence of this 
warning depending on that spelling/signal)

> Types in system libraries are not different from user types w.r.t. CTAD -- 
> several types are just not designed with CTAD in mind.
>
> Finally, I'd like to note that this warning only triggers if the user 
> explicitly passes `-Wctad-maybe-unsupported`. It's not enabled by `-Wall` or 
> `-Wextra`. I would argue that users that go out of their way to enable this 
> warning probably share my sentiment that CTAD is sometimes harmful, and they 
> probably want to be warned about questionable uses even for classes defined 
> in a system library.

Maybe? But if they're stuck with system libraries that don't use this 
convention, this extra coverage of the warning may prevent them from using the 
warning at all, even for their non-system-header code.

> Anyway, I think I've made my opinion clear and I don't think I have much more 
> to bring to the table. I'll ship D133535  
> regardless, but if this patch ships, I think we should have some way of 
> enabling the warning for types in system headers. For example, folks using 
> `-Wctad-maybe-unsupported` should really be warned about using CTAD on a type 
> like `std::reverse_iterator`.

Yeah, that migh

[PATCH] D133194: rewording note note_constexpr_invalid_cast

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman added a comment.

Hi, 
I am not getting why **Sema/cast.c** is failing here. While all test are 
running fine on my system?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133194

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

[PATCH] D133586: [clang] do not hash undefined qualifiers for FunctionNoProtoType

[Richard Howell via Phabricator via cfe-commits]

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

When calculating the ODR hash of a `FunctionNoProtoType` do not
include qualifiers derived from `FastTypeQuals`. These are only
defined in the constructor for `FunctionProtoType`.

This change ensures the ODR hash and PCM output is stable for
and modules containing `FunctionNoProtoType`s.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133586

Files:
  clang/lib/AST/ODRHash.cpp


Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -889,9 +889,12 @@
   void VisitFunctionType(const FunctionType *T) {
 AddQualType(T->getReturnType());
 T->getExtInfo().Profile(ID);
-Hash.AddBoolean(T->isConst());
-Hash.AddBoolean(T->isVolatile());
-Hash.AddBoolean(T->isRestrict());
+if (T->isFunctionProtoType()) {
+  // These values are undefined for FunctionNoProtoType
+  Hash.AddBoolean(T->isConst());
+  Hash.AddBoolean(T->isVolatile());
+  Hash.AddBoolean(T->isRestrict());
+}
 VisitType(T);
   }
 


Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -889,9 +889,12 @@
   void VisitFunctionType(const FunctionType *T) {
 AddQualType(T->getReturnType());
 T->getExtInfo().Profile(ID);
-Hash.AddBoolean(T->isConst());
-Hash.AddBoolean(T->isVolatile());
-Hash.AddBoolean(T->isRestrict());
+if (T->isFunctionProtoType()) {
+  // These values are undefined for FunctionNoProtoType
+  Hash.AddBoolean(T->isConst());
+  Hash.AddBoolean(T->isVolatile());
+  Hash.AddBoolean(T->isRestrict());
+}
 VisitType(T);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133587: Loop names used in reporting can grow very large

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: Whitney, dongjunduo, aeubanks, MaskRay.
Herald added subscribers: ormris, StephenFan, steven_wu, zzheng, hiraditya.
Herald added a project: All.
jamieschmeiser requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

In his work for https://reviews.llvm.org/D131960, @dongjunduo found that
the names for loops generated by getIRName could grow to be extremely
large because they are based on a serialization of the loop.  Replace this with
the name of the loop (which is based on the header) and patch up the
affected lit tests.

It was suggested (in separate conversations) that an analogous
fix be made in  https://reviews.llvm.org/D131960 with a FIXME
comment to resolve the 2 pieces of code so https://reviews.llvm.org/D131960
can be delivered as it is part of a GSoC project that is finishing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133587

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pass-ordering.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/loopnest-pass-ordering.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/pass-pipeline-parsing.ll
  llvm/test/Other/print-before-after.ll
  llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
  llvm/test/Transforms/LoopPredication/preserve-bpi.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/LoopUnroll/revisit.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
  
llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
@@ -4,7 +4,7 @@
 ; Running loop-distribute will result in LoopAccessAnalysis being required and
 ; cached in the LoopAnalysisManagerFunctionProxy.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner
 
 
 ; Then simple-loop-unswitch is removing/replacing some loops (resulting in
@@ -17,7 +17,7 @@
 ; SimpleLoopUnswitch not marking the Loop as removed, so we missed clearing
 ; the analysis caches.
 ;
-; CHECK: Running pass: SimpleLoopUnswitchPass on Loop at depth 1 containing: %loop_begin,%loop_b,%loop_b_inner,%loop_b_inner_exit,%loop_a,%loop_a_inner,%loop_a_inner_exit,%latch
+; CHECK: Running pass: SimpleLoopUnswitchPass on loop_begin
 ; CHECK-NEXT: Running analysis: OuterAnalysisManagerProxy
 ; CHECK-NEXT: Clearing all analysis results for: loop_a_inner
 
@@ -27,7 +27,7 @@
 ; loop_a_inner.us). This kind of verifies that it was correct to remove the
 ; loop_a_inner related analysis above.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner.us
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner.us
 
 
 define i32 @test6(i1* %ptr, i1 %cond1, i32* %a.ptr, i32* %b.ptr) {
Index: llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
===
--- llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
+++ llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
@@ -8,22 +8,22 @@
 ; CHECK: Running analysis: LoopAnalysis
 ; CHECK: Running analysis: InnerAnalysisManagerProxy<
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner2.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %outer.header
+; CHECK: Running analysis: LoopAccessAnalysis on outer.header
 ; CHECK: Running pass: LoopUnrollPass
 ; CHECK: Clearing all analysis results for: inner2.header
 ; CHECK: Clearing all analysis results for: outer.header
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header
 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header.1
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header:
 ; CHECK: Running pass: LoopAccessIn

[PATCH] D133588: [NFC] Remove a FIXME fixed by an earlier patch.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: sgatev.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Commit 28bd7945eabdbde2b1fc071ab2f9b78e6e754a1a 
 
incidentally fixed the
associated FIXME, but didn't delete it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133588

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -81,9 +81,6 @@
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 auto *Expr2 = cast(Val2);
 auto &MergedVal = MergedEnv.makeAtomicBoolValue();


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -81,9 +81,6 @@
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 auto *Expr2 = cast(Val2);
 auto &MergedVal = MergedEnv.makeAtomicBoolValue();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133587: Loop names used in reporting can grow very large

[Arthur Eubanks via Phabricator via cfe-commits]

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

this is something I've wanted to do for a while but never got around to
thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133587

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

[PATCH] D133586: [clang] do not hash undefined qualifiers for FunctionNoProtoType

[Richard Howell via Phabricator via cfe-commits]

rmaz updated this revision to Diff 459108.
rmaz added a comment.

Move code to VisitFunctionProtoType instead of branch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133586

Files:
  clang/lib/AST/ODRHash.cpp


Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -889,9 +889,6 @@
   void VisitFunctionType(const FunctionType *T) {
 AddQualType(T->getReturnType());
 T->getExtInfo().Profile(ID);
-Hash.AddBoolean(T->isConst());
-Hash.AddBoolean(T->isVolatile());
-Hash.AddBoolean(T->isRestrict());
 VisitType(T);
   }
 
@@ -904,6 +901,9 @@
 for (auto ParamType : T->getParamTypes())
   AddQualType(ParamType);
 
+Hash.AddBoolean(T->isConst());
+Hash.AddBoolean(T->isVolatile());
+Hash.AddBoolean(T->isRestrict());
 VisitFunctionType(T);
   }
 


Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -889,9 +889,6 @@
   void VisitFunctionType(const FunctionType *T) {
 AddQualType(T->getReturnType());
 T->getExtInfo().Profile(ID);
-Hash.AddBoolean(T->isConst());
-Hash.AddBoolean(T->isVolatile());
-Hash.AddBoolean(T->isRestrict());
 VisitType(T);
   }
 
@@ -904,6 +901,9 @@
 for (auto ParamType : T->getParamTypes())
   AddQualType(ParamType);
 
+Hash.AddBoolean(T->isConst());
+Hash.AddBoolean(T->isVolatile());
+Hash.AddBoolean(T->isRestrict());
 VisitFunctionType(T);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] abc16c7 - [NFC] Remove a FIXME fixed by an earlier patch.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-09-09T17:13:52Z
New Revision: abc16c7a5b0a63d14172262153608b3d24de957f

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

LOG: [NFC] Remove a FIXME fixed by an earlier patch.

Commit 28bd7945eabdbde2b1fc071ab2f9b78e6e754a1a incidentally fixed the
associated FIXME, but didn't delete it.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 9acd993eb25da..8b761bef226ac 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -81,9 +81,6 @@ static Value *mergeDistinctValues(QualType Type, Value *Val1,
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 auto *Expr2 = cast(Val2);
 auto &MergedVal = MergedEnv.makeAtomicBoolValue();



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

[PATCH] D133588: [NFC] Remove a FIXME fixed by an earlier patch.

[Yitzhak Mandelbaum 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 rGabc16c7a5b0a: [NFC] Remove a FIXME fixed by an earlier 
patch. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133588

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -81,9 +81,6 @@
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 auto *Expr2 = cast(Val2);
 auto &MergedVal = MergedEnv.makeAtomicBoolValue();


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -81,9 +81,6 @@
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 auto *Expr2 = cast(Val2);
 auto &MergedVal = MergedEnv.makeAtomicBoolValue();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan.
MyDeveloperDay added a project: clang-format.
Herald added a project: All.
MyDeveloperDay requested review of this revision.
Herald added a project: clang.

Working in a mixed environment of both vscode/vim with a team configured 
prettier configuration, this can leave clang-format and prettier fighting each 
other over the formatting of arrays, both simple arrays of elements.

This review aims to add some "control knobs" to the Json formatting in 
clang-format to help align the two tools so they can be used interchangeably.

This will allow simply arrays `[1, 2, 3]` to remain on a single line but will 
break those arrays based on context within that array.

Happy to change the name of the option (this is the third name I tried)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133589

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- clang/unittests/Format/FormatTestJson.cpp
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -159,6 +159,27 @@
"]");
 }
 
+TEST_F(FormatTestJson, JsonArrayOneLine) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.JsonMultilineArrays = false;
+  Style.SpacesInContainerLiterals = false;
+  verifyFormat("[]", Style);
+  verifyFormat("[1]", Style);
+  verifyFormat("[1, 2]", Style);
+  verifyFormat("[1, 2, 3]", Style);
+  verifyFormat("[1, 2, 3, 4]", Style);
+  verifyFormat("[1, 2, 3, 4, 5]", Style);
+
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2,\n"
+   "  {\n"
+   "A: 1\n"
+   "  }\n"
+   "]",
+   Style);
+}
+
 TEST_F(FormatTestJson, JsonNoStringSplit) {
   FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
   Style.IndentWidth = 4;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4399,18 +4399,48 @@
 // }
 if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace))
   return true;
-// Always break after a JSON array opener.
+// Always break after a JSON array opener based on JsonMultilineArrays
 // [
 // ]
 if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
 !Right.is(tok::r_square)) {
-  return true;
+  if (Right.is(tok::l_brace))
+return true;
+  // scan to the right if an we see an object or an array inside
+  // then break
+  const auto *Tok = &Right;
+  while (Tok) {
+if (Tok->isOneOf(tok::l_brace, tok::l_square)) {
+  return true;
+}
+if (Tok->isOneOf(tok::r_brace, tok::r_square)) {
+  break;
+}
+Tok = Tok->Next;
+  }
+  return Style.JsonMultilineArrays;
 }
 // Always break after successive entries.
 // 1,
 // 2
-if (Left.is(tok::comma))
-  return true;
+if (Left.is(tok::comma)) {
+  if (Right.is(tok::l_brace)) {
+return true;
+  }
+  // scan to the right if an we see an object or an array inside
+  // then break
+  const auto *Tok = &Right;
+  while (Tok) {
+if (Tok->isOneOf(tok::l_brace, tok::l_square)) {
+  return true;
+}
+if (Tok->isOneOf(tok::r_brace, tok::r_square)) {
+  break;
+}
+Tok = Tok->Next;
+  }
+  return Style.JsonMultilineArrays;
+}
   }
 
   // If the last token before a '}', ']', or ')' is a comma or a trailing
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -756,6 +756,8 @@
 IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
 
+IO.mapOptional("JsonMultilineArrays", Style.JsonMultilineArrays);
+
 IO.mapOptional("PackConstructorInitializers",
Style.PackConstructorInitializers);
 // For backward compatibility:
@@ -1249,6 +1251,7 @@
   LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
   LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
+  LLVMStyle.JsonMultilineArrays = true;
   LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Form

[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

It turns out that this change wasn't necessary. I've deleted the relevant FIXME 
in https://reviews.llvm.org/rGabc16c7a5b0a63d14172262153608b3d24de957f.

AFAICT, the reason that the new test `JoinBackedge` fails at HEAD is because we 
discard the results of analyzing the function body, so the update `Foo = false` 
is simply ignored. This hypothesis rests on observing that each block is 
visited only once during analysis of the test. It's also consistent with the 
known (broken) behavior of our comparison operator. Interestingly, this is a 
nicely stark illustration of the unsoundness of our current approach. :( So, 
this test will be useful in the future when add proper widening.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130270

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

[PATCH] D133088: [Clang] Fix wrong diagnostic for scope identifier with internal linkage

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5904-5905
 def err_block_extern_cant_init : Error<
-  "'extern' variable cannot have an initializer">;
+  "declaration of block scope identifier with %select{external|internal}0 "
+  "linkage shall have no initializer">;
 def warn_extern_init : Warning<"'extern' variable has an initializer">,

I don't think we want to try to call out external vs internal linkage here, 
more on why below. Also, we don't usually use "shall" in diagnostic wording.



Comment at: clang/lib/Sema/SemaDecl.cpp:12774-12784
+  // C99 6.7.8p5. C++ has no such restriction, but that is a defect.
   if (VDecl->isLocalVarDecl() && VDecl->hasExternalStorage()) {
-// C99 6.7.8p5. C++ has no such restriction, but that is a defect.
-Diag(VDecl->getLocation(), diag::err_block_extern_cant_init);
+unsigned LinkageKind = /*external*/ 0;
+// C2x 6.7.10p6.
+if (VDecl->getFormalLinkage() == InternalLinkage)
+  LinkageKind = /*internal*/ 1;
+





Comment at: clang/lib/Sema/SemaDecl.cpp:12776-12778
+unsigned LinkageKind = /*external*/ 0;
+// C2x 6.7.10p6.
+if (VDecl->getFormalLinkage() == InternalLinkage)





Comment at: clang/test/Sema/err-decl-block-extern-no-init.c:6
+{
+extern int x = 1; // expected-error {{declaration of block scope 
identifier with internal linkage shall have no initializer}}
+}

This example is why I think we should reword the diagnostic. As it stands, this 
diagnostic wording would be baffling to most users because they'd go "whaddya 
mean *INTERNAL* linkage, I said `extern` for a reason!" So using a diagnostic 
wording of "with linkage" sidesteps that -- what the linkage is doesn't matter 
for fixing the code, it's the initializer that's the issue.

Also, I don't know what it means for a block scope variable to have *internal* 
linkage instead of *no* linkage (and I'm not certain it's possible to observe 
the difference).

That said, I think this example should have a *second* diagnostic, which is a 
warning, letting the user know that the actual linkage does not match the 
specified linkage. (This should be done in a separate patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133088

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

[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

[Sam Estep via Phabricator via cfe-commits]

samestep accepted this revision.
samestep added a comment.

@ymandel Huh, interesting! I'll go ahead and abandon this patch, then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130270

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

[Aaron H Liu via Phabricator via cfe-commits]

AaronLiu added a comment.

This cause a large amount failures in our testing with errors such as:

  error: ISO C++17 does not allow 'register' storage class specifier 
[-Wregister]

  error: ISO C++17 does not allow dynamic exception specifications 
[-Wdynamic-exception-spec]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ASTDumper.cpp:205
+  if (const Decl *D = dyn_cast(this))
+D->dump();
+}

aaron.ballman wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > erichkeane wrote:
> > > > > > One thing to note is that the 'else' case here is a little 
> > > > > > uninformative.  See 
> > > > > > https://clang.llvm.org/doxygen/DeclBase_8cpp_source.html#l00915 for 
> > > > > > some similar logic here (though not sure we should be emulating 
> > > > > > that).
> > > > > > 
> > > > > > More, I wonder if there is SOME message here that should be dumped 
> > > > > > for 'else'.
> > > > > Looking at what inherits from `DeclContext`, is there use of it which 
> > > > > is *not* a `Decl`? I couldn't find a use where it's not also a `Decl`.
> > > > I've DEFINITELY run into it in the debugger before, but I have no idea 
> > > > WHAT case that is. It is sometimes just "DeclContext is an invalid 
> > > > pointer" kinda thing, so it might be worth-while to have SOME output 
> > > > besides "print nothing", particularly when debugging.
> > > IIRC, the case this comes up in is when the object is only partially 
> > > constructed, and so I agree that having an `else` clause here would be 
> > > useful -- because this interface is predominately used from a debugger, 
> > > it has to deal with special "impossible" situations a bit more carefully.
> > So looking at the other dump member functions, all of them seem to assume 
> > we have a valid `DeclContext` and so they do not have any else either.
> > 
> > So maybe we want to think about making the rest a bit smarter as well?
> > So maybe we want to think about making the rest a bit smarter as well?
> 
> What do you have in mind?
> 
> I think it's plausible to be in a partially constructed state in terms of 
> when ctors are called, but I'm far less worried about people calling a dump 
> method in the middle of the member inits happening.
> > So maybe we want to think about making the rest a bit smarter as well?
> 
> What do you have in mind?
> 
> I think it's plausible to be in a partially constructed state in terms of 
> when ctors are called, but I'm far less worried about people calling a dump 
> method in the middle of the member inits happening.

Apologies, what I meant was if we are going to add an `else` here then we 
should be consistent and add it in the other dump functions as well. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133499

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131465#3780717 , @AaronLiu wrote:

> This cause a large amount failures in our testing with errors such as:
>
>   error: ISO C++17 does not allow 'register' storage class specifier 
> [-Wregister]
>
>   error: ISO C++17 does not allow dynamic exception specifications 
> [-Wdynamic-exception-spec]

Sorry to hear that. Who is "our" in this case and do you have a suggestion for 
how you'd like to proceed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTDumper.cpp:205
+  if (const Decl *D = dyn_cast(this))
+D->dump();
+}

shafik wrote:
> aaron.ballman wrote:
> > shafik wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > aaron.ballman wrote:
> > > > > > erichkeane wrote:
> > > > > > > One thing to note is that the 'else' case here is a little 
> > > > > > > uninformative.  See 
> > > > > > > https://clang.llvm.org/doxygen/DeclBase_8cpp_source.html#l00915 
> > > > > > > for some similar logic here (though not sure we should be 
> > > > > > > emulating that).
> > > > > > > 
> > > > > > > More, I wonder if there is SOME message here that should be 
> > > > > > > dumped for 'else'.
> > > > > > Looking at what inherits from `DeclContext`, is there use of it 
> > > > > > which is *not* a `Decl`? I couldn't find a use where it's not also 
> > > > > > a `Decl`.
> > > > > I've DEFINITELY run into it in the debugger before, but I have no 
> > > > > idea WHAT case that is. It is sometimes just "DeclContext is an 
> > > > > invalid pointer" kinda thing, so it might be worth-while to have SOME 
> > > > > output besides "print nothing", particularly when debugging.
> > > > IIRC, the case this comes up in is when the object is only partially 
> > > > constructed, and so I agree that having an `else` clause here would be 
> > > > useful -- because this interface is predominately used from a debugger, 
> > > > it has to deal with special "impossible" situations a bit more 
> > > > carefully.
> > > So looking at the other dump member functions, all of them seem to assume 
> > > we have a valid `DeclContext` and so they do not have any else either.
> > > 
> > > So maybe we want to think about making the rest a bit smarter as well?
> > > So maybe we want to think about making the rest a bit smarter as well?
> > 
> > What do you have in mind?
> > 
> > I think it's plausible to be in a partially constructed state in terms of 
> > when ctors are called, but I'm far less worried about people calling a dump 
> > method in the middle of the member inits happening.
> > > So maybe we want to think about making the rest a bit smarter as well?
> > 
> > What do you have in mind?
> > 
> > I think it's plausible to be in a partially constructed state in terms of 
> > when ctors are called, but I'm far less worried about people calling a dump 
> > method in the middle of the member inits happening.
> 
> Apologies, what I meant was if we are going to add an `else` here then we 
> should be consistent and add it in the other dump functions as well. 
None of the other DeclContext dump functions cast to `Decl`, so I'm not certain 
there's an `else` to add to any of them, or am I misunderstanding?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133499

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

[PATCH] D133194: rewording note note_constexpr_invalid_cast

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 459121.
Codesbyusman added a comment.

updating one time again, to again run test as no failed test on my system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133194

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/cast.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx20_2b,cxx2b-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx11_20,cxx20_2b -triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_20,cxx11-triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-designator -fcxx-exceptions -pedantic %s -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion -Wvla 
+
 
 namespace StaticAssertFoldTest {
 
@@ -11,6 +12,10 @@
 
 }
 
+int array[(long)(char *)0]; // expected-warning {{variable length arrays are a C99 feature}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
+
 typedef decltype(sizeof(char)) size_t;
 
 template constexpr T id(const T &t) { return t; }
@@ -2449,6 +2454,6 @@
 
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
+  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}} 
 }
 }
Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -1,4 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -DConstantCast -Wvla -verify %s 
+
+#ifdef ConstantCast
+int array[(long)(char *)0]; // expected-warning {{variable length array used}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{this conversion is not allowed in a constant expression}}
+#endif
 
 typedef struct { unsigned long bits[(((1) + (64) - 1) / (64))]; } cpumask_t;
 cpumask_t x;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7473,12 +7473,14 @@
   }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 0 << Info.Ctx.getLangOpts().CPlusPlus;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
 if (!Info.Ctx.getLangOpts().CPlusPlus20)
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+  CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 1 << Info.Ctx.getLangOpts().CPlusPlus;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitBuiltinBitCastExpr(const BuiltinBitCastExpr *E) {
@@ -8179,7 +8181,8 @@
   return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
 case CK_LValueBitCast:
-  this->CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  this->CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return false;
   Result.Designator.setInvalid();
@@ -8890,9 +8893,10 @@
 Result.Designator.setInvalid();
 if (SubExpr->getType()->isVoidPointerType

[PATCH] D133578: [OpenMP] Add generation of SIMD align assumptions to OMPIRBuilder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thanks for this patch! I have two drive by comments that should probably be 
addressed first.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:629
+ llvm::ArrayRef AlignedVars,
+ llvm::Value *Alignment, Value *IfCond, ConstantInt *Simdlen,
  ConstantInt *Safelen);

Aren't there different alignments possible, so X is aligned 32 and Y is aligned 
64? If so, should we tie the Value and Alignment together in the API?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976
 
+  const int DefaultAlignment = 16;
+

This doesn't work. Use the data layout for any default values please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133578

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

[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D130270#3780705 , @samestep wrote:

> @ymandel Huh, interesting! I'll go ahead and abandon this patch, then?

Please. Sorry I didn't catch this earlier!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130270

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

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1225
 assert(CAT && "can't emit array init for non-constant-bound array");
-unsigned NumInitElements = ILE->getNumInits();
-unsigned NumElements = CAT->getSize().getZExtValue();
+uint64_t NumInitElements = ILE->getNumInits();
+uint64_t NumElements = CAT->getSize().getZExtValue();

Note that getNumInits() itself is returning "unsigned".  (Looks easy to fix, 
though.)



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1265
+
+Inits.push_back(llvm::ConstantAggregateZero::get(FillerType));
+

Need to be careful here: in general, you can't just fill everything with zero.  
Need to check how getArrayFiller() is lowered.  (For example, consider `class 
X; int X::*a[10] = { 0 };`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D133194: rewording note note_constexpr_invalid_cast

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/cast.c:2
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -DConstantCast -Wvla -verify %s 
+

Instead of adding this RUN line, I think you can modify the previous RUN line 
to add `-Wvla`, and skip the `#ifdef` entirely.

The reason this test is passing for you locally but failing on the server is 
because this RUN line is missing a triple and there are parts of the test which 
are sensitive to the target architecture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133194

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D131465#3780717 , @AaronLiu wrote:

> This cause a large amount failures in our testing with errors such as:
>
>   error: ISO C++17 does not allow 'register' storage class specifier 
> [-Wregister]
>
>   error: ISO C++17 does not allow dynamic exception specifications 
> [-Wdynamic-exception-spec]

You may want to `-std=gnu++14` or `-std=c++14` to pin your projects to C++14, 
instead of relying on the Clang default.
Is your case is like PlayStation 4 and 5 where a platform wants to have a 
different default (I'd hope that we can try avoiding such differences), a 
target maintainer can weigh in and perhaps change it.
(The configuration file discussions are interesting, too 
https://discourse.llvm.org/t/configuration-files/42529 
https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[clang] 5e3ac79 - Loop names used in reporting can grow very large

[Jamie Schmeiser via cfe-commits]

Author: Jamie Schmeiser
Date: 2022-09-09T13:45:14-04:00
New Revision: 5e3ac7969039678fc017a5599b051ea2b78075a4

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

LOG: Loop names used in reporting can grow very large

Summary:
The code for generating a name for loops for various reporting scenarios
created a name by serializing the loop into a string.  This may result in
a very large name for a loop containing many blocks.  Use the getName()
function on the loop instead.

Author: Jamie Schmeiser 
Reviewed By: Whitney (Whitney Tsang), aeubanks (Arthur Eubanks)
Differential Revision: https://reviews.llvm.org/D133587

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/StandardInstrumentations.cpp
llvm/test/Other/loop-pass-ordering.ll
llvm/test/Other/loop-pass-printer.ll
llvm/test/Other/loopnest-pass-ordering.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/pass-pipeline-parsing.ll
llvm/test/Other/print-before-after.ll
llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
llvm/test/Transforms/LoopPredication/preserve-bpi.ll
llvm/test/Transforms/LoopRotate/pr35210.ll
llvm/test/Transforms/LoopUnroll/revisit.ll
llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll

llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 463fc522c6a28..3dbc55e23e28d 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -83,7 +83,7 @@
 ; CHECK-O: Running pass: LowerConstantIntrinsicsPass on main
 ; CHECK-O: Running pass: LoopSimplifyPass on main
 ; CHECK-O: Running pass: LCSSAPass on main
-; CHECK-O: Running pass: LoopRotatePass on Loop at depth 1 containing: %b
+; CHECK-O: Running pass: LoopRotatePass on b
 ; CHECK-O: Running pass: LoopDistributePass on main
 ; CHECK-O: Running pass: InjectTLIMappings on main
 ; CHECK-O: Running pass: LoopVectorizePass on main
@@ -99,7 +99,7 @@
 ; CHECK-O: Running pass: 
RequireAnalysisPass<{{.*}}OptimizationRemarkEmitterAnalysis
 ; CHECK-O: Running pass: LoopSimplifyPass on main
 ; CHECK-O: Running pass: LCSSAPass on main
-; CHECK-O: Running pass: LICMPass on Loop at depth 1 containing: %b
+; CHECK-O: Running pass: LICMPass on b
 ; CHECK-O: Running pass: AlignmentFromAssumptionsPass on main
 ; CHECK-O: Running pass: LoopSinkPass on main
 ; CHECK-O: Running pass: InstSimplifyPass on main

diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp 
b/llvm/lib/Passes/StandardInstrumentations.cpp
index 861e76b38f66b..1ac557d975cec 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -188,10 +188,7 @@ std::string getIRName(Any IR) {
 
   if (any_isa(IR)) {
 const Loop *L = any_cast(IR);
-std::string S;
-raw_string_ostream OS(S);
-L->print(OS, /*Verbose*/ false, /*PrintNested*/ false);
-return OS.str();
+return L->getName().str();
   }
 
   llvm_unreachable("Unknown wrapped IR type");

diff  --git a/llvm/test/Other/loop-pass-ordering.ll 
b/llvm/test/Other/loop-pass-ordering.ll
index ab3839f5cc997..fde49a7b73a5f 100644
--- a/llvm/test/Other/loop-pass-ordering.ll
+++ b/llvm/test/Other/loop-pass-ordering.ll
@@ -8,11 +8,11 @@
 ;  /  \\
 ; loop.0.0  loop.0.1  loop.1.0
 ;
-; CHECK: Running pass: NoOpLoopPass on Loop at depth 2 containing: %loop.0.0
-; CHECK: Running pass: NoOpLoopPass on Loop at depth 2 containing: %loop.0.1
-; CHECK: Running pass: NoOpLoopPass on Loop at depth 1 containing: %loop.0
-; CHECK: Running pass: NoOpLoopPass on Loop at depth 2 containing: %loop.1.0
-; CHECK: Running pass: NoOpLoopPass on Loop at depth 1 containing: %loop.1
+; CHECK: Running pass: NoOpLoopPass on loop.0.0
+; CHECK: Running pass: NoOpLoopPass on loop.0.1
+; CHECK: Running pass: NoOpLoopPass on loop.0
+; CHECK: Running pass: NoOpLoopPass on loop.1.0
+; CHECK: Running pass: NoOpLoopPass on loop.1
 
 define void @f() {
 entry:

diff  --git a/llvm/test/Other/loop-pass-printer.ll 
b/llvm/test/Other/loop-pass-printer.ll
index 98ad72b142bca..60bfdbbaea9ef 100644
--- a/llvm/test/Other/loop-pass-printer.ll
+++ b/llvm/test/Other/loop-pass-printer.ll
@@ -35,7 +35,7 @@
 ; BAR: ; Exit blocks
 ; BAR:  end:
 
-; FOO-MODULE: IR Dump After {{Unroll|LoopFullUnrollPass}} {{.*}}%loop
+; FOO-MODULE: IR Dump After {{Unroll|LoopFullUnrollPass}} {{.*}}loop
 ; FOO-MODULE-NEXT: ModuleID =
 ; FOO-MODULE: define void @foo
 ; FOO-MODULE: define void @bar

diff  --git a/llvm/test/Other/loopnest-pass-ordering.ll 
b/llvm/test/Other/loopnest-pass-

[PATCH] D133587: Loop names used in reporting can grow very large

[Jamie Schmeiser 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 rG5e3ac7969039: Loop names used in reporting can grow very 
large (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133587

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pass-ordering.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/loopnest-pass-ordering.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/pass-pipeline-parsing.ll
  llvm/test/Other/print-before-after.ll
  llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
  llvm/test/Transforms/LoopPredication/preserve-bpi.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/LoopUnroll/revisit.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
  
llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
@@ -4,7 +4,7 @@
 ; Running loop-distribute will result in LoopAccessAnalysis being required and
 ; cached in the LoopAnalysisManagerFunctionProxy.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner
 
 
 ; Then simple-loop-unswitch is removing/replacing some loops (resulting in
@@ -17,7 +17,7 @@
 ; SimpleLoopUnswitch not marking the Loop as removed, so we missed clearing
 ; the analysis caches.
 ;
-; CHECK: Running pass: SimpleLoopUnswitchPass on Loop at depth 1 containing: %loop_begin,%loop_b,%loop_b_inner,%loop_b_inner_exit,%loop_a,%loop_a_inner,%loop_a_inner_exit,%latch
+; CHECK: Running pass: SimpleLoopUnswitchPass on loop_begin
 ; CHECK-NEXT: Running analysis: OuterAnalysisManagerProxy
 ; CHECK-NEXT: Clearing all analysis results for: loop_a_inner
 
@@ -27,7 +27,7 @@
 ; loop_a_inner.us). This kind of verifies that it was correct to remove the
 ; loop_a_inner related analysis above.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner.us
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner.us
 
 
 define i32 @test6(i1* %ptr, i1 %cond1, i32* %a.ptr, i32* %b.ptr) {
Index: llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
===
--- llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
+++ llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
@@ -8,22 +8,22 @@
 ; CHECK: Running analysis: LoopAnalysis
 ; CHECK: Running analysis: InnerAnalysisManagerProxy<
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner2.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %outer.header
+; CHECK: Running analysis: LoopAccessAnalysis on outer.header
 ; CHECK: Running pass: LoopUnrollPass
 ; CHECK: Clearing all analysis results for: inner2.header
 ; CHECK: Clearing all analysis results for: outer.header
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header
 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header.1
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header:
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header.1
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header.1
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header.1:
 
Index: llvm/test/Transforms/LoopUnroll/revisit.ll
===
--- llvm/test/Transforms/LoopUnroll/revisit.ll
+++ llvm/test/Transforms/LoopUnroll/revisit.ll
@@ -39,7 +39,7 @@
 l0.0.0:
   %cond.0.0.0 = load volatile i1, i1* %ptr
   br i1 %cond.0.0.0, label %l0.0.0, label %l0.0.1.ph
-; CHECK: LoopFullUnrollP

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

[Ofek Shochat via Phabricator via cfe-commits]

OfekShochat added a subscriber: eli.friedman.
OfekShochat added a comment.

yep @eli.friedman. probably gonna scratch this idea, makes more sense to 
actually tackle the problem itself, which is that getArrayFiller doesnt return 
anything. trying to actually get to the problem, but its quite hard, Visit's 
are thrown everywhere, and I cant really look at the problem. 
tryEmitPrivateForVarInit is called, and then it calls a Visit, which somehow 
transfers it to VisitInitListExpr. that thing worked just because it worked 
around that problem, even if not intentionally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133570

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

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129883

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D131465#3776841 , @nikic wrote:

> In D131465#3776599 , @nikic wrote:
>
>> It looks like the switch to `-std=c++17` has some major compile-time impact: 
>> http://llvm-compile-time-tracker.com/compare.php?from=047c7aa96dadf8a2c71a29e2df610d628d9e7e3e&to=3e99b8d947ac024831e59be2b604ac67a24fed94&stat=instructions
>>  The `O0` build of 7zip becomes 80% slower (without codegen changes).
>
> I suspect this is due to increased STL size in in C++17. Taking one sample 
> file, preprocessed source is 289149 bytes with `-std=gnu++14` and 654888 with 
> `-std=gnu++17`. The source size increased by more than a factor of two, so 
> it's not very surprising that the parsing time also increased by the same 
> factor.

Thanks for the analysis. C++'s fault then...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[PATCH] D132913: [HLSL] Preserve vec3 for HLSL.

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz 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/D132913/new/

https://reviews.llvm.org/D132913

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