date:20220404

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a comment.

I am not sure that the choice of `isOSBinFormatELF` to (afaik) primarily scope 
this change from affecting AIX (where we know the library calls are not 
implemented to be lock-free yet) is better than alternative where the condition 
is for little-endian mode or specifically for not AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Amir Ayupov via Phabricator via cfe-commits

Amir added inline comments.



Comment at: bolt/lib/Core/DebugData.cpp:823
 Hasher.update(AbbrevData);
-StringRef Key = Hasher.final();
+auto Hash = Hasher.final();
+StringRef Key((const char *)Hash.data(), Hash.size());

I think it would be more in line with LLVM coding standards to expand `auto` in 
this case (and others) – see 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable.
 
My understanding is that `auto` is fine where the type is obvious from the 
context (is explicitly available on the same line e.g. with casts), is abstract 
(T::iterator types), or doesn't matter (e.g. lambdas).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

2022-04-04 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added inline comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// TODO: when clang allows c++17, use std::clamp instead
+uint32_t clamp(uint64_t value, uint32_t hi, uint32_t low) {
+  if (value > hi)

Nit: seems more intuitive to pass low before high. But not sure the low is 
needed in this case, see comment at callsite.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:201
+  auto Tolerance = getMisExpectTolerance(I.getContext());
+  Tolerance = clamp(Tolerance, 99, 0);
+

Looks like Tolerance is unsigned, as are the arguments to clamp. So it can 
never go below 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi updated this revision to Diff 420378.
akyrtzi added a comment.

Also revert the `README` changes to the previous version of `BLAKE3` class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

Files:
  bolt/lib/Core/DebugData.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Serialization/ASTWriter.cpp
  lld/MachO/SyntheticSections.cpp
  llvm/include/llvm/Support/BLAKE3.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/include/llvm/Support/MD5.h
  llvm/include/llvm/Support/SHA1.h
  llvm/include/llvm/Support/SHA256.h
  llvm/include/llvm/Support/raw_sha1_ostream.h
  llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/DebugInfo/CodeView/TypeHashing.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
  llvm/lib/MC/MCDwarf.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/ObjCopy/MachO/MachOWriter.cpp
  llvm/lib/Support/BLAKE3/README.md
  llvm/lib/Support/MD5.cpp
  llvm/lib/Support/SHA1.cpp
  llvm/lib/Support/SHA256.cpp
  llvm/unittests/Support/BLAKE3Test.cpp
  llvm/unittests/Support/HashBuilderTest.cpp
  llvm/unittests/Support/MD5Test.cpp
  llvm/unittests/Support/SHA256.cpp
  llvm/unittests/Support/raw_sha1_ostream_test.cpp
  mlir/lib/Pass/IRPrinting.cpp

Index: mlir/lib/Pass/IRPrinting.cpp
===
--- mlir/lib/Pass/IRPrinting.cpp
+++ mlir/lib/Pass/IRPrinting.cpp
@@ -66,7 +66,7 @@
 ArrayRef(reinterpret_cast(), sizeof(T)));
   }
 
-  SmallString<20> hash;
+  std::array hash;
 };
 
 //===--===//
Index: llvm/unittests/Support/raw_sha1_ostream_test.cpp
===
--- llvm/unittests/Support/raw_sha1_ostream_test.cpp
+++ llvm/unittests/Support/raw_sha1_ostream_test.cpp
@@ -14,7 +14,7 @@
 
 using namespace llvm;
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789ABCDEF";
   size_t Length = Input.size();
 
@@ -39,7 +39,7 @@
 TEST(sha1_hash_test, Basic) {
   ArrayRef Input((const uint8_t *)"Hello World!", 12);
   std::array Vec = SHA1::hash(Input);
-  std::string Hash = toHex({(const char *)Vec.data(), 20});
+  std::string Hash = toHex(Vec);
   ASSERT_EQ("2EF7BDE608CE5404E97D5F042F95F89F1C232871", Hash);
 }
 
Index: llvm/unittests/Support/SHA256.cpp
===
--- llvm/unittests/Support/SHA256.cpp
+++ llvm/unittests/Support/SHA256.cpp
@@ -20,7 +20,7 @@
 
 namespace {
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789abcdef";
   size_t Length = Input.size();
 
Index: llvm/unittests/Support/MD5Test.cpp
===
--- llvm/unittests/Support/MD5Test.cpp
+++ llvm/unittests/Support/MD5Test.cpp
@@ -62,7 +62,7 @@
   std::array Vec = MD5::hash(Input);
   MD5::MD5Result MD5Res;
   SmallString<32> Res;
-  memcpy(MD5Res.Bytes.data(), Vec.data(), Vec.size());
+  memcpy(MD5Res.data(), Vec.data(), Vec.size());
   MD5::stringifyResult(MD5Res, Res);
   EXPECT_EQ(Res, "c3fcd3d76192e4007dfb496cca67e13b");
   EXPECT_EQ(0x3be167ca6c49fb7dULL, MD5Res.high());
@@ -79,10 +79,7 @@
 ReferenceHash.update("abcd");
 MD5::MD5Result ReferenceResult;
 ReferenceHash.final(ReferenceResult);
-StringRef ExpectedResult =
-StringRef(reinterpret_cast(ReferenceResult.Bytes.data()),
-  ReferenceResult.Bytes.size());
-EXPECT_EQ(Hash.result(), ExpectedResult);
+EXPECT_EQ(Hash.result(), ReferenceResult);
   }
 
   Hash.update("xyz");
@@ -93,10 +90,7 @@
 ReferenceHash.update("xyz");
 MD5::MD5Result ReferenceResult;
 ReferenceHash.final(ReferenceResult);
-StringRef ExpectedResult =
-StringRef(reinterpret_cast(ReferenceResult.Bytes.data()),
-  ReferenceResult.Bytes.size());
-EXPECT_EQ(Hash.final(), ExpectedResult);
+EXPECT_EQ(Hash.final(), ReferenceResult);
   }
 }
 } // namespace
Index: llvm/unittests/Support/HashBuilderTest.cpp
===
--- llvm/unittests/Support/HashBuilderTest.cpp
+++ llvm/unittests/Support/HashBuilderTest.cpp
@@ -44,13 +44,15 @@
   HasherTAndEndianness::Endianness>;
 
 template 
-static std::string hashWithBuilder(const Ts &...Args) {
-  return HashBuilder().add(Args...).final().str();
+static typename HashBuilder::template HashResultTy<>
+hashWithBuilder(const Ts &...Args) {
+  return HashBuilder().add(Args...).final();
 }
 
 template 
-static std::string hashRangeWithBuilder(const Ts &...Args) {
-  return HashBuilder().addRange(Args...).final().str();

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi added inline comments.



Comment at: llvm/include/llvm/Support/BLAKE3.h:38
+/// A class that wraps the BLAKE3 algorithm.
+template  class BLAKE3 {
 public:

dexonsmith wrote:
> The visual noise of `BLAKE3<>` everywhere is a bit unfortunate.
> 
> Another option:
> ```
> lang=c++
> // Hardcoded to 32B. OR same implementation as before, with optional template
> // parameters to choose a size when accessing the hash.
> class BLAKE3 { /* ... */ };
> 
> // Wrap final(), result(), and hash() to truncate the hash.
> template  class TruncatedBLAKE3 : public BLAKE3 { 
> ... };
> ```
> 
> WDYT?
Good idea! I moved `BLAKE3` back to templated sizes for `final()` and 
`result()` and added `TruncatedBLAKE3` that has the size parameter on the class.
`TruncatedBLAKE3` is useful for using BLAKE3 as the hasher type for 
`HashBuilder` with non-default hash sizes, otherwise one can use `BLAKE3` for 
all other uses.



Comment at: llvm/include/llvm/Support/MD5.h:82
+  /// Finishes off the hash, and returns the 16-byte hash data.
+  std::array final();
 

dexonsmith wrote:
> Should this (and `result()` below) be `MD5Result`? It has an implicit cast to 
> `std::array`. Maybe it's better as you have it... not sure...
I avoided `MD5Result` due to not being as good as `std::array` for a "drop-in 
replacement" for `StringRef` due to lack of `data()` and `size()`.
But it occurred to me that `MD5Result` could just inherit from 
`std::array` which would make it better fit to replace `StringRef` 
//and// improves & simplifies its API, see updated patch.



Comment at: llvm/lib/Support/SHA1.cpp:289
+std::array HashResult;
+std::array ReturnResult;
+  };

dexonsmith wrote:
> Should this be `HASH_LENGTH` instead of 20?
Updated 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

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

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi updated this revision to Diff 420377.
akyrtzi added a comment.

- Move `BLAKE3` back to templated sizes for `final()` and `result()` and add 
`TruncatedBLAKE3` that has the size parameter on the class.
- Make `MD5Result` inherit from `std::array` which both simplifies 
its API and makes it a better fit for replacing `StringRef`.
- Use `HASH_LENGTH` in implementations of `SHA1::final()` and `SHA256::final()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

Files:
  bolt/lib/Core/DebugData.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Serialization/ASTWriter.cpp
  lld/MachO/SyntheticSections.cpp
  llvm/include/llvm/Support/BLAKE3.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/include/llvm/Support/MD5.h
  llvm/include/llvm/Support/SHA1.h
  llvm/include/llvm/Support/SHA256.h
  llvm/include/llvm/Support/raw_sha1_ostream.h
  llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/DebugInfo/CodeView/TypeHashing.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
  llvm/lib/MC/MCDwarf.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/ObjCopy/MachO/MachOWriter.cpp
  llvm/lib/Support/BLAKE3/README.md
  llvm/lib/Support/MD5.cpp
  llvm/lib/Support/SHA1.cpp
  llvm/lib/Support/SHA256.cpp
  llvm/unittests/Support/BLAKE3Test.cpp
  llvm/unittests/Support/HashBuilderTest.cpp
  llvm/unittests/Support/MD5Test.cpp
  llvm/unittests/Support/SHA256.cpp
  llvm/unittests/Support/raw_sha1_ostream_test.cpp
  mlir/lib/Pass/IRPrinting.cpp

Index: mlir/lib/Pass/IRPrinting.cpp
===
--- mlir/lib/Pass/IRPrinting.cpp
+++ mlir/lib/Pass/IRPrinting.cpp
@@ -66,7 +66,7 @@
 ArrayRef(reinterpret_cast(), sizeof(T)));
   }
 
-  SmallString<20> hash;
+  std::array hash;
 };
 
 //===--===//
Index: llvm/unittests/Support/raw_sha1_ostream_test.cpp
===
--- llvm/unittests/Support/raw_sha1_ostream_test.cpp
+++ llvm/unittests/Support/raw_sha1_ostream_test.cpp
@@ -14,7 +14,7 @@
 
 using namespace llvm;
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789ABCDEF";
   size_t Length = Input.size();
 
@@ -39,7 +39,7 @@
 TEST(sha1_hash_test, Basic) {
   ArrayRef Input((const uint8_t *)"Hello World!", 12);
   std::array Vec = SHA1::hash(Input);
-  std::string Hash = toHex({(const char *)Vec.data(), 20});
+  std::string Hash = toHex(Vec);
   ASSERT_EQ("2EF7BDE608CE5404E97D5F042F95F89F1C232871", Hash);
 }
 
Index: llvm/unittests/Support/SHA256.cpp
===
--- llvm/unittests/Support/SHA256.cpp
+++ llvm/unittests/Support/SHA256.cpp
@@ -20,7 +20,7 @@
 
 namespace {
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789abcdef";
   size_t Length = Input.size();
 
Index: llvm/unittests/Support/MD5Test.cpp
===
--- llvm/unittests/Support/MD5Test.cpp
+++ llvm/unittests/Support/MD5Test.cpp
@@ -62,7 +62,7 @@
   std::array Vec = MD5::hash(Input);
   MD5::MD5Result MD5Res;
   SmallString<32> Res;
-  memcpy(MD5Res.Bytes.data(), Vec.data(), Vec.size());
+  memcpy(MD5Res.data(), Vec.data(), Vec.size());
   MD5::stringifyResult(MD5Res, Res);
   EXPECT_EQ(Res, "c3fcd3d76192e4007dfb496cca67e13b");
   EXPECT_EQ(0x3be167ca6c49fb7dULL, MD5Res.high());
@@ -79,10 +79,7 @@
 ReferenceHash.update("abcd");
 MD5::MD5Result ReferenceResult;
 ReferenceHash.final(ReferenceResult);
-StringRef ExpectedResult =
-StringRef(reinterpret_cast(ReferenceResult.Bytes.data()),
-  ReferenceResult.Bytes.size());
-EXPECT_EQ(Hash.result(), ExpectedResult);
+EXPECT_EQ(Hash.result(), ReferenceResult);
   }
 
   Hash.update("xyz");
@@ -93,10 +90,7 @@
 ReferenceHash.update("xyz");
 MD5::MD5Result ReferenceResult;
 ReferenceHash.final(ReferenceResult);
-StringRef ExpectedResult =
-StringRef(reinterpret_cast(ReferenceResult.Bytes.data()),
-  ReferenceResult.Bytes.size());
-EXPECT_EQ(Hash.final(), ExpectedResult);
+EXPECT_EQ(Hash.final(), ReferenceResult);
   }
 }
 } // namespace
Index: llvm/unittests/Support/HashBuilderTest.cpp
===
--- llvm/unittests/Support/HashBuilderTest.cpp
+++ llvm/unittests/Support/HashBuilderTest.cpp
@@ -44,13 +44,15 @@
   HasherTAndEndianness::Endianness>;
 
 template 
-static std::string hashWithBuilder(const Ts &...Args) {
-  return HashBuilder().add(Args...).final().str();
+static

[PATCH] D122699: [HLSL] Add Semantic syntax, and SV_GroupIndex

2022-04-04 Thread Chris Bieneman via Phabricator via cfe-commits

beanz updated this revision to Diff 420376.
beanz added a comment.

Updating based on feedback from @aaron.ballman

I think this covers all the cases for parsing as a function parameter. I 
haven't added the code to parse these in structure definitions or globals yet. 
If it is okay I'd like to do that in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122699

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/AttributeCommonInfo.h
  clang/include/clang/Basic/Attributes.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/semantic_parsing.hlsl
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1493,6 +1493,9 @@
 Spelling += Namespace;
 Spelling += " ";
   }
+} else if (Variety == "HLSLSemantic") {
+  Prefix = ":";
+  Suffix = "";
 } else {
   llvm_unreachable("Unknown attribute syntax variety!");
 }
@@ -3300,7 +3303,7 @@
   // Separate all of the attributes out into four group: generic, C++11, GNU,
   // and declspecs. Then generate a big switch statement for each of them.
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
-  std::vector Declspec, Microsoft, GNU, Pragma;
+  std::vector Declspec, Microsoft, GNU, Pragma, HLSLSemantic;
   std::map> CXX, C2x;
 
   // Walk over the list of all attributes, and split them out based on the
@@ -3321,6 +3324,8 @@
 C2x[SI.nameSpace()].push_back(R);
   else if (Variety == "Pragma")
 Pragma.push_back(R);
+  else if (Variety == "HLSLSemantic")
+HLSLSemantic.push_back(R);
 }
   }
 
@@ -3338,6 +3343,9 @@
   OS << "case AttrSyntax::Pragma:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
+  OS << "case AttrSyntax::HLSLSemantic:\n";
+  OS << "  return llvm::StringSwitch(Name)\n";
+  GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
   auto fn = [](const char *Spelling, const char *Variety,
   const std::map> ) {
 OS << "case AttrSyntax::" << Variety << ": {\n";
@@ -4286,7 +4294,7 @@
 
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
   std::vector GNU, Declspec, Microsoft, CXX11,
-  Keywords, Pragma, C2x;
+  Keywords, Pragma, C2x, HLSLSemantic;
   std::set Seen;
   for (const auto *A : Attrs) {
 const Record  = *A;
@@ -4338,6 +4346,8 @@
   Matches = 
 else if (Variety == "Pragma")
   Matches = 
+else if (Variety == "HLSLSemantic")
+  Matches = 
 
 assert(Matches && "Unsupported spelling variety found");
 
@@ -4373,6 +4383,8 @@
   StringMatcher("Name", Keywords, OS).Emit();
   OS << "  } else if (AttributeCommonInfo::AS_Pragma == Syntax) {\n";
   StringMatcher("Name", Pragma, OS).Emit();
+  OS << "  } else if (AttributeCommonInfo::AS_HLSLSemantic == Syntax) {\n";
+  StringMatcher("Name", HLSLSemantic, OS).Emit();
   OS << "  }\n";
   OS << "  return AttributeCommonInfo::UnknownAttribute;\n"
  << "}\n";
@@ -4482,7 +4494,7 @@
   }
 }
 
-enum class SpellingKind {
+enum class SpellingKind : size_t {
   GNU,
   CXX11,
   C2x,
@@ -4490,8 +4502,10 @@
   Microsoft,
   Keyword,
   Pragma,
+  HLSLSemantic,
+  NumSpellingKinds
 };
-static const size_t NumSpellingKinds = (size_t)SpellingKind::Pragma + 1;
+static const size_t NumSpellingKinds = (size_t)SpellingKind::NumSpellingKinds;
 
 class SpellingList {
   std::vector Spellings[NumSpellingKinds];
@@ -4509,7 +4523,8 @@
 .Case("Declspec", SpellingKind::Declspec)
 .Case("Microsoft", SpellingKind::Microsoft)
 .Case("Keyword", SpellingKind::Keyword)
-.Case("Pragma", SpellingKind::Pragma);
+.Case("Pragma", SpellingKind::Pragma)
+.Case("HLSLSemantic", SpellingKind::HLSLSemantic);
 std::string Name;
 if (!Spelling.nameSpace().empty()) {
   switch (Kind) {
@@ -4610,8 +4625,8 @@
   // List what spelling syntaxes the attribute supports.
   OS << ".. csv-table:: Supported Syntaxes\n";
   OS << "   :header: \"GNU\", \"C++11\", \"C2x\", \"``__declspec``\",";
-  OS << " \"Keyword\", \"``#pragma``\", \"``#pragma clang attribute``\"\n\n";
-  OS << "   \"";
+  OS << " \"Keyword\", \"``#pragma``\", \"``#pragma clang attribute``\",";
+  OS << " \"HLSL Semantic\"\n\n   \"";
   for (size_t

[PATCH] D121560: [clang][Opt] Add NoArgUnusedWith to not warn for unused redundant options

2022-04-04 Thread Alex Brachet via Phabricator via cfe-commits

abrachet updated this revision to Diff 420375.
abrachet added a comment.

Actually clang-format...


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

https://reviews.llvm.org/D121560

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/claim-unused.c
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/ObjdumpOptID.h
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -258,6 +258,16 @@
   OS << "#endif // PREFIX\n\n";
 
   OS << "/\n";
+
+  OS << R"(
+#ifndef OPT_PREFIX
+#define OPT_PREFIX
+#endif
+
+#define CAT_(A, B) A ## B
+#define CAT(A, B) CAT_(A, B)
+#define GET_OPT(OPT) CAT(OPT_PREFIX, OPT)
+  )";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
   for (const Record  : llvm::make_pointee_range(Groups)) {
@@ -298,6 +308,9 @@
 OS << ", nullptr";
 
 // The option Values (unused for groups).
+OS << ", nullptr";
+
+// NoArgUnusedWith (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -388,6 +401,20 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+
+// List of flags who's presence should cause this flag to not warn if used.
+OS << ", ";
+std::vector List = R.getValueAsListOfDefs("NoArgUnusedWith");
+if (!List.size()) {
+  OS << "nullptr";
+  return;
+}
+OS << "((const unsigned *)&(unsigned []){";
+// First element is the length of the array.
+OS << List.size();
+for (Record *R : List)
+  OS << ", GET_OPT(" << R->getName() << ")";
+OS << "})";
   };
 
   auto IsMarshallingOption = [](const Record ) {
@@ -405,6 +432,11 @@
   OptsWithMarshalling.push_back();
   }
   OS << "#endif // OPTION\n";
+  OS << R"(#undef GET_OPT
+#undef CAT
+#undef CAT_
+#undef OPT_PREFIX
+)";
 
   auto CmpMarshallingOpts = [](const Record *const *A, const Record *const *B) {
 unsigned AID = (*A)->getID();
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,7 +18,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
   OPT_##ID,
 #include "Opts.inc"
   LastOption
@@ -37,9 +37,10 @@
 
 static const OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
-   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
+  {PREFIX,NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class, \
+   PARAM, FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES,  \
+   UNUSEDWITH},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -18,9 +18,9 @@
 static

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:443
 
-// PPC64 supports atomics up to 8 bytes.
-MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+// PPC64 supports atomics up to 16 bytes.
+MaxAtomicPromoteWidth = 128;

Clarify that the statement is not for PPC64 in general.



Comment at: clang/lib/Basic/Targets/PPC.h:445
+MaxAtomicPromoteWidth = 128;
+// PPC64 supports inlining atomics up to 8 bytes.
+MaxAtomicInlineWidth = 64;

Clarify that the support up to 8 bytes is for "baseline" PPC64 (i.e., 
non-baseline implementations may support lock-free inline 16-byte atomic 
operations).



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:18033
+ (EnableQuadwordAtomics ||
+  Subtarget.getTargetTriple().isOSBinFormatELF()) &&
+ Subtarget.hasQuadwordAtomics();

Can we have a comment here to explain the `isOSBinFormatELF` check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[clang] 4875ff1 - [RISCV] Remove redundant enabling of IAS for Clang, NFC

2022-04-04 Thread Brad Smith via cfe-commits


Author: Brad Smith
Date: 2022-04-04T23:44:49-04:00
New Revision: 4875ff1dc90bba089a5a14023d5eec69490b0422

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

LOG: [RISCV] Remove redundant enabling of IAS for Clang, NFC

Generic_GCC::IsIntegratedAssemblerDefault() already takes care of RISCV.

Reviewed By: kito-cheng, MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.h 
b/clang/lib/Driver/ToolChains/RISCVToolchain.h
index 62099bee04047..46b94bdb54e09 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -22,7 +22,6 @@ class LLVM_LIBRARY_VISIBILITY RISCVToolChain : public 
Generic_ELF {
  const llvm::opt::ArgList );
 
   static bool hasGCCToolchain(const Driver , const llvm::opt::ArgList );
-  bool IsIntegratedAssemblerDefault() const override { return true; }
   void addClangTargetOptions(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
  Action::OffloadKind) const override;



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

[PATCH] D123097: [RISCV] Remove redundant enabling of IAS for Clang, NFC

2022-04-04 Thread Brad Smith via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4875ff1dc90b: [RISCV] Remove redundant enabling of IAS for 
Clang, NFC (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123097

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.h


Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -22,7 +22,6 @@
  const llvm::opt::ArgList );
 
   static bool hasGCCToolchain(const Driver , const llvm::opt::ArgList );
-  bool IsIntegratedAssemblerDefault() const override { return true; }
   void addClangTargetOptions(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
  Action::OffloadKind) const override;


Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -22,7 +22,6 @@
  const llvm::opt::ArgList );
 
   static bool hasGCCToolchain(const Driver , const llvm::opt::ArgList );
-  bool IsIntegratedAssemblerDefault() const override { return true; }
   void addClangTargetOptions(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
  Action::OffloadKind) const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:448
+  void setMaxAtomicWidth() override {
+// For layout on ELF targets, we support up to 16 bytes.
+if (getTriple().isOSBinFormatELF())

hubert.reinterpretcast wrote:
> lkail wrote:
> > hubert.reinterpretcast wrote:
> > > I believe this should be presented more as this override being 
> > > implemented currently only for ELF targets. The current presentation 
> > > seems to imply more design intent for non-ELF targets than there is 
> > > consensus for.
> > > 
> > > For example:
> > > ```
> > > if (!getTriple().isOSBinFormatELF())
> > >   return PPCTargetInfo::setMaxAtomicWidth();
> > > ```
> > It looks a chance to adjust according to arch level to me(Considering we 
> > finally will make xcoff and elf targets consistent here). If you have 
> > strong opinion on this, I'll make a change here.
> This function currently modifies two different things. One that should be 
> arch-level-sensitive (and, iiuc, is already so) and one that should not be 
> arch-level-sensitive.
> 
> So, the `isOSBinFormatELF` checks are here mainly to scope the patch 
> (temporarily).
> 
> I only suggested to use the call to the base class as a shorthand for 
> "nothing special that this function is doing". The associated comment should 
> read something like "Restrict 16-byte atomic changes to ELF targets for now. 
> TODO: Consider other targets as well."
> 
> At the very least (for this patch), I do not believe the `isOSBinFormatELF` 
> should be checked twice in terms of control flow.
The new version of the change already addresses this; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:448
+  void setMaxAtomicWidth() override {
+// For layout on ELF targets, we support up to 16 bytes.
+if (getTriple().isOSBinFormatELF())

lkail wrote:
> hubert.reinterpretcast wrote:
> > I believe this should be presented more as this override being implemented 
> > currently only for ELF targets. The current presentation seems to imply 
> > more design intent for non-ELF targets than there is consensus for.
> > 
> > For example:
> > ```
> > if (!getTriple().isOSBinFormatELF())
> >   return PPCTargetInfo::setMaxAtomicWidth();
> > ```
> It looks a chance to adjust according to arch level to me(Considering we 
> finally will make xcoff and elf targets consistent here). If you have strong 
> opinion on this, I'll make a change here.
This function currently modifies two different things. One that should be 
arch-level-sensitive (and, iiuc, is already so) and one that should not be 
arch-level-sensitive.

So, the `isOSBinFormatELF` checks are here mainly to scope the patch 
(temporarily).

I only suggested to use the call to the base class as a shorthand for "nothing 
special that this function is doing". The associated comment should read 
something like "Restrict 16-byte atomic changes to ELF targets for now. TODO: 
Consider other targets as well."

At the very least (for this patch), I do not believe the `isOSBinFormatELF` 
should be checked twice in terms of control flow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122377

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

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

2022-04-04 Thread Michael Kruse via Phabricator via cfe-commits

Meinersbur updated this revision to Diff 420371.
Meinersbur marked an inline comment as done.
Meinersbur added a comment.

- Rebase
- Fix convertOmpAtomicUpdate, convertOmpAtomicCapture, and convertOmpOrdered


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

Files:
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/test/Target/LLVMIR/openmp-llvm.mlir

Index: mlir/test/Target/LLVMIR/openmp-llvm.mlir
===
--- mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -1854,6 +1854,9 @@
 
 // CHECK-LABEL: @omp_sections_trivial
 llvm.func @omp_sections_trivial() -> () {
+  // CHECK:   br label %[[ENTRY:[a-zA-Z_.]+]]
+
+  // CHECK: [[ENTRY]]:
   // CHECK:   br label %[[PREHEADER:.*]]
 
   // CHECK: [[PREHEADER]]:
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
@@ -72,6 +72,21 @@
 return allocaInsertPoint;
 
   // Otherwise, insert to the entry block of the surrounding function.
+  // If the current IRBuilder InsertPoint is the function's entry, it cannot
+  // also be used for alloca insertion which would result in insertion order
+  // confusion. Create a new BasicBlock for the Builder and use the entry block
+  // for the allocs.
+  if (builder.GetInsertBlock() ==
+  ()->getParent()->getEntryBlock()) {
+assert(builder.GetInsertPoint() == builder.GetInsertBlock()->end() &&
+   "Assuming end of basic block");
+llvm::BasicBlock *entryBB = llvm::BasicBlock::Create(
+builder.getContext(), "entry", builder.GetInsertBlock()->getParent(),
+builder.GetInsertBlock()->getNextNode());
+builder.CreateBr(entryBB);
+builder.SetInsertPoint(entryBB);
+  }
+
   llvm::BasicBlock  =
   builder.GetInsertBlock()->getParent()->getEntryBlock();
   return llvm::OpenMPIRBuilder::InsertPointTy(
@@ -255,23 +270,12 @@
   // TODO: Is the Parallel construct cancellable?
   bool isCancellable = false;
 
-  // Ensure that the BasicBlock for the the parallel region is sparate from the
-  // function entry which we may need to insert allocas.
-  if (builder.GetInsertBlock() ==
-  ()->getParent()->getEntryBlock()) {
-assert(builder.GetInsertPoint() == builder.GetInsertBlock()->end() &&
-   "Assuming end of basic block");
-llvm::BasicBlock *entryBB =
-llvm::BasicBlock::Create(builder.getContext(), "parallel.entry",
- builder.GetInsertBlock()->getParent(),
- builder.GetInsertBlock()->getNextNode());
-builder.CreateBr(entryBB);
-builder.SetInsertPoint(entryBB);
-  }
+  llvm::OpenMPIRBuilder::InsertPointTy allocaIP =
+  findAllocaInsertPoint(builder, moduleTranslation);
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createParallel(
-  ompLoc, findAllocaInsertPoint(builder, moduleTranslation), bodyGenCB,
-  privCB, finiCB, ifCond, numThreads, pbKind, isCancellable));
+  ompLoc, allocaIP, bodyGenCB, privCB, finiCB, ifCond, numThreads, pbKind,
+  isCancellable));
 
   return bodyGenStatus;
 }
@@ -522,7 +526,6 @@
   SmallVector vecValues =
   moduleTranslation.lookupValues(orderedOp.depend_vec_vars());
 
-  llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   size_t indexVecValues = 0;
   while (indexVecValues < vecValues.size()) {
 SmallVector storeValues;
@@ -531,9 +534,11 @@
   storeValues.push_back(vecValues[indexVecValues]);
   indexVecValues++;
 }
+llvm::OpenMPIRBuilder::InsertPointTy allocaIP =
+findAllocaInsertPoint(builder, moduleTranslation);
+llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
 builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createOrderedDepend(
-ompLoc, findAllocaInsertPoint(builder, moduleTranslation), numLoops,
-storeValues, ".cnt.addr", isDependSource));
+ompLoc, allocaIP, numLoops, storeValues, ".cnt.addr", isDependSource));
   }
   return success();
 }
@@ -634,10 +639,12 @@
   // called for variables which have destructors/finalizers.
   auto finiCB = [&](InsertPointTy codeGenIP) {};
 
+  llvm::OpenMPIRBuilder::InsertPointTy allocaIP =
+  findAllocaInsertPoint(builder, moduleTranslation);
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createSections(
-  ompLoc, findAllocaInsertPoint(builder, moduleTranslation), sectionCBs,
-  privCB, finiCB, false,

[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

2022-04-04 Thread Michael Kruse via Phabricator via cfe-commits

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



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:74-75
+  // for the allocs.
+  // TODO: Create a dedicated alloca BasicBlock at function creation such that
+  // we do not need to move the current InertPoint here.
+  if (builder.GetInsertBlock() ==

shraiysh wrote:
> [Suggestion] We probably don't need to do this, because if a conversion does 
> not require an alloca (for example, barrier), we will be creating a 
> basicblock unnecessarily. So, creating this on-demand seems okay to me.
Making decision based on the current position of an IRBuilder creates heaps of 
problems and unpredictability. Special cases like this one here need to be 
tested and maintained, and still easily introduce bugs. An unconditional 
dedicated alloca block avoids all these problems.

An extra basic block on the other side is insignificant. It will be gone as 
soon as simplify-cfg runs.

However, this seems to be controversial, so I removed the TODO for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835

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

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added inline comments.



Comment at: llvm/include/llvm/Support/BLAKE3.h:38
+/// A class that wraps the BLAKE3 algorithm.
+template  class BLAKE3 {
 public:

The visual noise of `BLAKE3<>` everywhere is a bit unfortunate.

Another option:
```
lang=c++
// Hardcoded to 32B. OR same implementation as before, with optional template
// parameters to choose a size when accessing the hash.
class BLAKE3 { /* ... */ };

// Wrap final(), result(), and hash() to truncate the hash.
template  class TruncatedBLAKE3 : public BLAKE3 { ... 
};
```

WDYT?



Comment at: llvm/include/llvm/Support/MD5.h:82
+  /// Finishes off the hash, and returns the 16-byte hash data.
+  std::array final();
 

Should this (and `result()` below) be `MD5Result`? It has an implicit cast to 
`std::array`. Maybe it's better as you have it... not sure...



Comment at: llvm/lib/Support/SHA1.cpp:289
+std::array HashResult;
+std::array ReturnResult;
+  };

Should this be `HASH_LENGTH` instead of 20?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

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

[PATCH] D123104: [Modules] Use looked-up filename when looking for module maps

2022-04-04 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

(Forgot to click "accept".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123104

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

[PATCH] D123104: [Modules] Use looked-up filename when looking for module maps

2022-04-04 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added reviewers: benlangmuir, bruno.
dexonsmith added a comment.

This LGTM, with a couple of comments (on the comments!) inline.

> Upstreamed from apple#llvm-project 72cf785051fb1b3ef22eee4dd33366e41a275981.

Note, for some extra background:

- This was out-of-tree because it seemed "hacky". The thinking at the time 
(2017!) was that we should try to get a "clean" fix for upstream, which is what 
`FileEntryRef` is on the path toward...
- ... but `FileEntryRef` is blocked because of hard-to-reason-about failures 
related to modules (see, for example, my reverted patch at 
https://reviews.llvm.org/D92975).
- In the meantime, the in-tree behaviour is wrong, and it doesn't make sense to 
just leave it broken upstream indefinitely... and keeping it downstream is 
making it harder to wrap our heads around the full problem.
- In discussing the intersection of this with the fix @bnbarham is working on 
(reverted in https://reviews.llvm.org/D123103 after this test failed in 
downstream integration), we've come up with a more incremental way of pushing 
`FileEntryRef` forward... hopefully incremental enough that we'll be unblocked 
and can finish it off. That's what I want documented in the FileManager FIXME 
as part of this patch.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1570
 // If there is a module that corresponds to this header, suggest it.
-hasModuleMap(File->getName(), Root, IsSystemHeaderDir);
+
+// FIXME: File->getName() *should* be the same as FileName, but because

Nit: I'd very slightly prefer the two paragraphs be part of the same comment 
(put a `//` here for the blank line), but if you'd prefer it like this it's 
fine.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1571-1572
+
+// FIXME: File->getName() *should* be the same as FileName, but because
+// of the VFS and various hacks in FileManager, that's not necessarily the
+// case. We should update this to use FileEntryRef instead and either

Can you also expand the "redirection" FIXME in FileManager.cpp (or add an 
appropriate one)?

I think the steps are something like:
- Add API to determine if the name has been rewritten by the VFS (that's the 
patch you're working on).
- Expose the requested filename for adoption. I think the way to implement this 
is to allow "redirection FileEntryRef"s to be returned, rather than returning 
the pointed-at-FileEntryRef, and customizing `getName()` to see through the 
indirection.
- Update callers such as `HeaderSearch::findUsableModuleForHeader()` (comment 
should mention this function specifically as an example I think!) to explicitly 
get the requested filename rather than using `getName()`.
- Add a FileManager::getExternalPath API for explicitly getting the remapped 
external filename when there is one available, and adopt it in callers like 
diagnostics/deps reporting instead of calling `getName()` directly.
- Switch the meaning of `FileEntryRef::getName()` to get the requested name, 
not the external name. Once that sticks, revert callers that want the requested 
name back to calling `getName()`.
- Add an API to VFS to get the external filename lazily, stop doing it up 
front, and update FileManager::getExternalPath to do the right thing. (This 
will stop creating redirection entries for those cases.)

The need to expose the requested-name explicitly as a first step is a new 
insight we had when discussing this offline. Overall I think the plan for 
unwinding these hacks is under-documented; since we've spent a good deal of 
time on this, I'd like to record what we know so anyone new looking at 
`FileManager::getFileRef()` can guess how to help out (and better understand 
any odd behaviour they're seeing). Feel free to trim down and/or expand and/or 
completely rewrite, as long as the comment in FileManager encapsulates your 
understanding of how we can incrementally unwind all these hacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123104

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

[PATCH] D118525: [modules] Merge ObjC interface ivars with anonymous types.

2022-04-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG29444f0444c6: [modules] Merge ObjC interface ivars with 
anonymous types. (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118525

Files:
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/AST/ast-dump-decl.mm
  clang/test/Modules/merge-anon-record-definition-in-objc.m
  clang/test/SemaObjC/check-dup-decls-inside-objc.m

Index: clang/test/SemaObjC/check-dup-decls-inside-objc.m
===
--- /dev/null
+++ clang/test/SemaObjC/check-dup-decls-inside-objc.m
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class -x objective-c++ %s
+
+// Test decls inside Objective-C entities are considered to be duplicates of same-name decls outside of these entities.
+
+@protocol SomeProtocol
+struct InProtocol {}; // expected-note {{previous definition is here}}
+- (union MethodReturnType { int x; float y; })returningMethod; // expected-note {{previous definition is here}}
+#ifdef __cplusplus
+// expected-error@-2 {{'MethodReturnType' cannot be defined in a parameter type}}
+#endif
+@end
+
+@interface Container {
+  struct InInterfaceCurliesWithField {} field; // expected-note {{previous definition is here}}
+  union InInterfaceCurlies { int x; float y; }; // expected-note {{previous definition is here}}
+}
+enum InInterface { kX = 0, }; // expected-note {{previous definition is here}}
+#ifdef __cplusplus
+enum class InInterfaceScoped { kXScoped = 0, }; // expected-note {{previous definition is here}}
+#endif
+@end
+
+@interface Container(Category)
+union InCategory { int x; float y; }; // expected-note {{previous definition is here}}
+@end
+
+@interface Container() {
+  enum InExtensionCurliesWithField: int { kY = 1, } extensionField; // expected-note {{previous definition is here}}
+  struct InExtensionCurlies {}; // expected-note {{previous definition is here}}
+}
+union InExtension { int x; float y; }; // expected-note {{previous definition is here}}
+@end
+
+@implementation Container {
+  union InImplementationCurliesWithField { int x; float y; } implField; // expected-note {{previous definition is here}}
+  enum InImplementationCurlies { kZ = 2, }; // expected-note {{previous definition is here}}
+}
+struct InImplementation {}; // expected-note {{previous definition is here}}
+@end
+
+@implementation Container(Category)
+enum InCategoryImplementation { kW = 3, }; // expected-note {{previous definition is here}}
+@end
+
+
+struct InProtocol { int a; }; // expected-error {{redefinition of 'InProtocol'}}
+union MethodReturnType { int a; long b; }; // expected-error {{redefinition of 'MethodReturnType'}}
+
+struct InInterfaceCurliesWithField { int a; }; // expected-error {{redefinition of 'InInterfaceCurliesWithField'}}
+union InInterfaceCurlies { int a; long b; }; // expected-error {{redefinition of 'InInterfaceCurlies'}}
+enum InInterface { kA = 10, }; // expected-error {{redefinition of 'InInterface'}}
+#ifdef __cplusplus
+enum class InInterfaceScoped { kAScoped = 10, }; // expected-error {{redefinition of 'InInterfaceScoped'}}
+#endif
+
+union InCategory { int a; long b; }; // expected-error {{redefinition of 'InCategory'}}
+
+enum InExtensionCurliesWithField: int { kB = 11, }; // expected-error {{redefinition of 'InExtensionCurliesWithField'}}
+struct InExtensionCurlies { int a; }; // expected-error {{redefinition of 'InExtensionCurlies'}}
+union InExtension { int a; long b; }; // expected-error {{redefinition of 'InExtension'}}
+
+union InImplementationCurliesWithField { int a; long b; }; // expected-error {{redefinition of 'InImplementationCurliesWithField'}}
+enum InImplementationCurlies { kC = 12, }; // expected-error {{redefinition of 'InImplementationCurlies'}}
+struct InImplementation { int a; }; // expected-error {{redefinition of 'InImplementation'}}
+
+enum InCategoryImplementation { kD = 13, }; // expected-error {{redefinition of 'InCategoryImplementation'}}
Index: clang/test/Modules/merge-anon-record-definition-in-objc.m
===
--- /dev/null
+++ clang/test/Modules/merge-anon-record-definition-in-objc.m
@@ -0,0 +1,84 @@
+// UNSUPPORTED: -zos, -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m -Wno-objc-property-implementation -Wno-incomplete-implementation \
+// RUN:-fmodules -fmodule-name=Target -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only

[clang] 29444f0 - [modules] Merge ObjC interface ivars with anonymous types.

2022-04-04 Thread Volodymyr Sapsai via cfe-commits


Author: Volodymyr Sapsai
Date: 2022-04-04T18:48:30-07:00
New Revision: 29444f0444c6d3586969fd6fbe49c8188c02c244

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

LOG: [modules] Merge ObjC interface ivars with anonymous types.

Without the fix ivars with anonymous types can trigger errors like

> error: 'TestClass::structIvar' from module 'Target' is not present in 
> definition of 'TestClass' provided earlier
> [...]
> note: declaration of 'structIvar' does not match

It happens because types of ivars from different modules are considered
to be different. And it is caused by not merging anonymous `TagDecl`
from different modules.

To fix that I've changed `serialization::needsAnonymousDeclarationNumber`
to handle anonymous `TagDecl` inside `ObjCInterfaceDecl`. But that's not
sufficient as C code inside `ObjCInterfaceDecl` doesn't use interface
decl as a decl context but switches to its parent (TranslationUnit in
most cases).  I'm changing that to make `ObjCContainerDecl` the lexical
decl context but keeping the semantic decl context intact.

Test "check-dup-decls-inside-objc.m" doesn't reflect a change in
functionality but captures the existing behavior to prevent regressions.

rdar://85563013

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

Added: 
clang/test/Modules/merge-anon-record-definition-in-objc.m
clang/test/SemaObjC/check-dup-decls-inside-objc.m

Modified: 
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Serialization/ASTCommon.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/AST/ast-dump-decl.mm

Removed: 




diff  --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 719513e7bda09..c56fcb8cc3cfe 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -1871,9 +1871,9 @@ void Parser::HelperActionsForIvarDeclarations(Decl 
*interfaceDecl, SourceLocatio
   if (!RBraceMissing)
 T.consumeClose();
 
-  Actions.ActOnObjCContainerStartDefinition(interfaceDecl);
+  assert(getObjCDeclContext() == interfaceDecl &&
+ "Ivars should have interfaceDecl as their decl context");
   Actions.ActOnLastBitfield(T.getCloseLocation(), AllIvarDecls);
-  Actions.ActOnObjCContainerFinishDefinition();
   // Call ActOnFields() even if we don't have any decls. This is useful
   // for code rewriting tools that need to be aware of the empty list.
   Actions.ActOnFields(getCurScope(), atLoc, interfaceDecl, AllIvarDecls,
@@ -1908,8 +1908,7 @@ void Parser::ParseObjCClassInstanceVariables(Decl 
*interfaceDecl,
   assert(Tok.is(tok::l_brace) && "expected {");
   SmallVector AllIvarDecls;
 
-  ParseScope ClassScope(this, Scope::DeclScope|Scope::ClassScope);
-  ObjCDeclContextSwitch ObjCDC(*this);
+  ParseScope ClassScope(this, Scope::DeclScope | Scope::ClassScope);
 
   BalancedDelimiterTracker T(*this, tok::l_brace);
   T.consumeOpen();
@@ -1973,13 +1972,13 @@ void Parser::ParseObjCClassInstanceVariables(Decl 
*interfaceDecl,
 }
 
 auto ObjCIvarCallback = [&](ParsingFieldDeclarator ) {
-  Actions.ActOnObjCContainerStartDefinition(interfaceDecl);
+  assert(getObjCDeclContext() == interfaceDecl &&
+ "Ivar should have interfaceDecl as its decl context");
   // Install the declarator into the interface decl.
   FD.D.setObjCIvar(true);
   Decl *Field = Actions.ActOnIvar(
   getCurScope(), FD.D.getDeclSpec().getSourceRange().getBegin(), FD.D,
   FD.BitfieldSize, visibility);
-  Actions.ActOnObjCContainerFinishDefinition();
   if (Field)
 AllIvarDecls.push_back(Field);
   FD.complete(Field);

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1e25346fde6f6..0b5b530bc756c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16059,9 +16059,20 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, 
TagUseKind TUK,
   // with C structs, unions, and enums when looking for a matching
   // tag declaration or definition. See the similar lookup tweak
   // in Sema::LookupName; is there a better way to deal with this?
-  while (isa(SearchDC) || isa(SearchDC))
+  while (isa(SearchDC))
+SearchDC = SearchDC->getParent();
+} else if (getLangOpts().CPlusPlus) {
+  // Inside ObjCContainer want to keep it as a lexical decl context but go
+  // past it (most often to TranslationUnit) to find the semantic decl
+  // context.
+  while (isa(SearchDC))
 SearchDC = SearchDC->getParent();
 }
+  } else if (getLangOpts().CPlusPlus) {
+// Don't use ObjCContainerDecl as the semantic decl context for anonymous
+// TagDecl the same way as we skip it for named TagDecl.
+while (isa(SearchDC))
+  SearchDC =

[PATCH] D123103: Revert "[VFS] RedirectingFileSystem only replace path if not already mapped"

2022-04-04 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

In D123103#3428298 , @bnbarham wrote:

> This broke crash reproducers in very specific circumstances, see 
> https://reviews.llvm.org/D123104. I'll re-commit with adding 
> `ExposesExternalVFSPath` instead of replacing `IsVFSMapped`.

Please expand the commit message to include some details (such as these) so 
git-log is more useful. With that, LGTM.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123103

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

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi updated this revision to Diff 420359.
akyrtzi added a comment.

Adjust `ASTFileSignature` to accept only the array hash bytes and directly from 
the `final()` call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123100

Files:
  bolt/lib/Core/DebugData.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Serialization/ASTWriter.cpp
  lld/ELF/Writer.cpp
  lld/MachO/SyntheticSections.cpp
  llvm/include/llvm/Support/BLAKE3.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/include/llvm/Support/MD5.h
  llvm/include/llvm/Support/SHA1.h
  llvm/include/llvm/Support/SHA256.h
  llvm/include/llvm/Support/raw_sha1_ostream.h
  llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/DebugInfo/CodeView/TypeHashing.cpp
  llvm/lib/ObjCopy/MachO/MachOWriter.cpp
  llvm/lib/Support/BLAKE3/README.md
  llvm/lib/Support/MD5.cpp
  llvm/lib/Support/SHA1.cpp
  llvm/lib/Support/SHA256.cpp
  llvm/unittests/Support/BLAKE3Test.cpp
  llvm/unittests/Support/HashBuilderTest.cpp
  llvm/unittests/Support/MD5Test.cpp
  llvm/unittests/Support/SHA256.cpp
  llvm/unittests/Support/raw_sha1_ostream_test.cpp
  mlir/lib/Pass/IRPrinting.cpp

STAMPS
actor(@akyrtzi) application(Differential) author(@akyrtzi) herald(H105) 
herald(H123) herald(H208) herald(H224) herald(H225) herald(H311) herald(H423) 
herald(H452) herald(H530) herald(H532) herald(H533) herald(H538) herald(H540) 
herald(H541) herald(H544) herald(H546) herald(H547) herald(H550) herald(H551) 
herald(H554) herald(H557) herald(H560) herald(H561) herald(H565) herald(H576) 
herald(H592) herald(H597) herald(H598) herald(H607) herald(H610) herald(H615) 
herald(H625) herald(H629) herald(H645) herald(H672) herald(H685) herald(H688) 
herald(H698) herald(H723) herald(H727) herald(H729) herald(H739) herald(H802) 
herald(H805) herald(H809) herald(H827) herald(H842) herald(H844) herald(H845) 
herald(H847) herald(H854) herald(H855) herald(H861) herald(H864) 
monogram(D123100) object-type(DREV) phid(PHID-DREV-ngvkfimyomxukhcmhald) 
reviewer(#lld-macho) reviewer(@Amir) reviewer(@dexonsmith) 
reviewer(@jhenderson) reviewer(@maksfb) reviewer(@MaskRay) reviewer(@rafauler) 
reviewer(@rriddle) revision-repository(rG) revision-status(accepted) 
subscriber(@aartbik) subscriber(@antiagainst) subscriber(@arichardson) 
subscriber(@arpith-jacob) subscriber(@ayermolo) subscriber(@cfe-commits) 
subscriber(@Chia-hungDuan) subscriber(@cota) subscriber(@dcaballe) 
subscriber(@dexonsmith) subscriber(@emaste) subscriber(@grosul1) 
subscriber(@hiraditya) subscriber(@Joonsoo) subscriber(@jurahul) 
subscriber(@Kayjukh) subscriber(@liufengdb) subscriber(@llvm-commits) 
subscriber(@mehdi_amini) subscriber(@mgester) subscriber(@msifontes) 
subscriber(@nicolasvasilache) subscriber(@rdzhabarov) subscriber(@rriddle) 
subscriber(@sdasgup3) subscriber(@shauheen) subscriber(@StephenFan) 
subscriber(@stephenneuendorffer) subscriber(@tatianashp) subscriber(@teijeong) 
subscriber(@wenzhicui) subscriber(@wrengr) subscriber(@yota9) tag(#all) 
tag(#clang) tag(#lld-macho) tag(#llvm) tag(#mlir) via(conduit)

Index: mlir/lib/Pass/IRPrinting.cpp
===
--- mlir/lib/Pass/IRPrinting.cpp
+++ mlir/lib/Pass/IRPrinting.cpp
@@ -66,7 +66,7 @@
 ArrayRef(reinterpret_cast(), sizeof(T)));
   }
 
-  SmallString<20> hash;
+  std::array hash;
 };
 
 //===--===//
Index: llvm/unittests/Support/raw_sha1_ostream_test.cpp
===
--- llvm/unittests/Support/raw_sha1_ostream_test.cpp
+++ llvm/unittests/Support/raw_sha1_ostream_test.cpp
@@ -14,7 +14,7 @@
 
 using namespace llvm;
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789ABCDEF";
   size_t Length = Input.size();
 
@@ -39,7 +39,7 @@
 TEST(sha1_hash_test, Basic) {
   ArrayRef Input((const uint8_t *)"Hello World!", 12);
   std::array Vec = SHA1::hash(Input);
-  std::string Hash = toHex({(const char *)Vec.data(), 20});
+  std::string Hash = toHex(Vec);
   ASSERT_EQ("2EF7BDE608CE5404E97D5F042F95F89F1C232871", Hash);
 }
 
Index: llvm/unittests/Support/SHA256.cpp
===
--- llvm/unittests/Support/SHA256.cpp
+++ llvm/unittests/Support/SHA256.cpp
@@ -20,7 +20,7 @@
 
 namespace {
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {
   static const char *const LUT = "0123456789abcdef";
   size_t Length = Input.size();
 
Index: llvm/unittests/Support/MD5Test.cpp
===
--- llvm/unittests/Support/MD5Test.cpp
+++ llvm/unittests/Support/MD5Test.cpp
@@ -79,10 +79,7 @@
 ReferenceHash.update("abcd");
 MD5::MD5Result ReferenceResult;

[PATCH] D118525: [modules] Merge ObjC interface ivars with anonymous types.

2022-04-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added a comment.

Thanks everyone for the feedback! Especially valuable is opinion on making 
`ObjCContainerDecl` a lexical decl context. As discussed earlier, not many 
people can provide feedback on Objective-C-related code and I'm still 
responsible for any problems caused by this change. So I'll commit the change 
and please let me know if it breaks anything, I'll be glad to address it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118525

STAMPS
actor(@vsapsai) application(Differential) author(@vsapsai) herald(H423) 
herald(H448) herald(H495) herald(H576) herald(H864) monogram(D118525) 
object-type(DREV) phid(PHID-DREV-sz3sdhlqgl5ypt3pi3wa) reviewer(@ahatanak) 
reviewer(@benlangmuir) reviewer(@Bigcheese) reviewer(@jansvoboda11) 
reviewer(@rsmith) revision-repository(rG) revision-status(needs-review) 
subscriber(@akyrtzi) subscriber(@benlangmuir) subscriber(@cfe-commits) 
subscriber(@ributzka) subscriber(@rjmccall) tag(#all) tag(#clang) 
tag(#clang-modules) via(web)

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

[PATCH] D123103: Revert "[VFS] RedirectingFileSystem only replace path if not already mapped"

2022-04-04 Thread Ben Barham via Phabricator via cfe-commits

bnbarham added a comment.

This broke crash reproducers in very specific circumstances, see 
https://reviews.llvm.org/D123104. I'll re-commit with adding 
`ExposesExternalVFSPath` instead of replacing `IsVFSMapped`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123103

STAMPS
actor(@bnbarham) application(Differential) author(@bnbarham) herald(H224) 
herald(H225) herald(H423) herald(H576) herald(H615) herald(H688) herald(H864) 
monogram(D123103) object-type(DREV) phid(PHID-DREV-tt3oqfaxmuyojrbgy6z7) 
reviewer(@dexonsmith) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) subscriber(@hiraditya) subscriber(@llvm-commits) 
tag(#all) tag(#clang) tag(#llvm) via(web)

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

[PATCH] D123104: [Modules] Use looked-up filename when looking for module maps

2022-04-04 Thread Ben Barham via Phabricator via cfe-commits

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

Prevent possible modulemap collisions by making sure to always use the
looked-up filename, regardless of any possible overlays.

Upstreamed from apple#llvm-project
72cf785051fb1b3ef22eee4dd33366e41a275981. As well as preventing the
collisions as above, it also highlighted an issue with the recent change
to narrow the FileManager hack of setting the `DirectoryEntry` in
`FileEntry` to the most recently looked up directory
(3fda0edc51fd68192a30e302d45db081bb02d7f9 
). 
Specifically, it would cause
the incorrect path to be used in `DoFrameworkLookup` in a crash
reproducer:

- Crash reproducers have `use-external-names` set to false
- `File->getFileEntry().getDir()->getName()` would then be the *cached* path, 
not the just looked up one
- `crash-vfs-umbrella-frameworks.m` fails with this change since the correct 
path is now looked up and causes B to be loaded twice


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123104

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/all-product-headers.yaml
  clang/test/Modules/modulemap-collision.m

STAMPS
actor(@bnbarham) application(Differential) author(@bnbarham) herald(H423) 
herald(H576) herald(H864) monogram(D123104) object-type(DREV) 
phid(PHID-DREV-h3pufxsxbgppzpajqza7) reviewer(@dexonsmith) 
revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) 
tag(#all) tag(#clang) via(conduit)

Index: clang/test/Modules/modulemap-collision.m
===
--- /dev/null
+++ clang/test/Modules/modulemap-collision.m
@@ -0,0 +1,15 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/sources %t/build
+// RUN: echo "// A.h" > %t/sources/A.h
+// RUN: echo "framework module A {}" > %t/sources/module.modulemap
+// RUN: echo "framework module A.Private { umbrella header \"A.h\" }" > %t/sources/module.private.modulemap
+// RUN: cp %t/sources/module.modulemap %t/build/module.modulemap
+// RUN: cp %t/sources/module.private.modulemap %t/build/module.private.modulemap
+
+// RUN: sed -e "s:DUMMY_DIR:%t:g" %S/Inputs/all-product-headers.yaml > %t/build/all-product-headers.yaml
+// RUN: %clang_cc1 -fsyntax-only -ivfsoverlay %t/build/all-product-headers.yaml -F%t/build -fmodules -fimplicit-module-maps -fmodules-cache-path=tmp -x objective-c %s -verify
+
+// expected-no-diagnostics
+#import 
Index: clang/test/Modules/Inputs/all-product-headers.yaml
===
--- /dev/null
+++ clang/test/Modules/Inputs/all-product-headers.yaml
@@ -0,0 +1,33 @@
+{
+  'version': 0,
+  'case-sensitive': 'false',
+  'roots': [
+{
+  'type': 'directory',
+  'name': "DUMMY_DIR/build/A.framework/PrivateHeaders"
+  'contents': [
+{
+  'type': 'file',
+  'name': "A.h",
+  'external-contents': "DUMMY_DIR/sources/A.h"
+}
+  ]
+},
+{
+  'type': 'directory',
+  'name': "DUMMY_DIR/build/A.framework/Modules"
+  'contents': [
+{
+  'type': 'file',
+  'name': "module.modulemap",
+  'external-contents': "DUMMY_DIR/build/module.modulemap"
+},
+{
+  'type': 'file',
+  'name': "module.private.modulemap",
+  'external-contents': "DUMMY_DIR/build/module.private.modulemap"
+}
+  ]
+}
+  ]
+}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,7 @@
   // If there is a module that corresponds to this header, suggest it.
   if (!findUsableModuleForHeader(
   >getFileEntry(), Dir ? Dir : File->getFileEntry().getDir(),
-  RequestingModule, SuggestedModule, IsSystemHeaderDir))
+  RequestingModule, SuggestedModule, IsSystemHeaderDir, FileName))
 return None;
 
   return *File;
@@ -1563,10 +1563,18 @@
 
 bool HeaderSearch::findUsableModuleForHeader(
 const FileEntry *File, const DirectoryEntry *Root, Module *RequestingModule,
-ModuleMap::KnownHeader *SuggestedModule, bool IsSystemHeaderDir) {
+ModuleMap::KnownHeader *SuggestedModule, bool IsSystemHeaderDir,
+StringRef FileName) {
   if (File && needModuleLookup(RequestingModule, SuggestedModule)) {
 // If there is a module that corresponds to this header, suggest it.
-hasModuleMap(File->getName(), Root, IsSystemHeaderDir);
+
+// FIXME: File->getName() *should* be the same as FileName, but because
+// of the VFS and various hacks in FileManager, that's not necessarily the
+// case. We should update this to use FileEntryRef instead

[PATCH] D123103: Revert "[VFS] RedirectingFileSystem only replace path if not already mapped"

2022-04-04 Thread Ben Barham via Phabricator via cfe-commits

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

This reverts commit 3fda0edc51fd68192a30e302d45db081bb02d7f9 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123103

Files:
  clang/lib/Basic/FileManager.cpp
  clang/test/VFS/external-names-multi-overlay.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

STAMPS
actor(@bnbarham) application(Differential) author(@bnbarham) herald(H224) 
herald(H225) herald(H423) herald(H576) herald(H615) herald(H688) herald(H864) 
monogram(D123103) object-type(DREV) phid(PHID-DREV-tt3oqfaxmuyojrbgy6z7) 
reviewer(@dexonsmith) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) subscriber(@hiraditya) subscriber(@llvm-commits) 
tag(#all) tag(#clang) tag(#llvm) via(conduit)

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1442,12 +1442,12 @@
   ErrorOr S = O->status("//root/file1");
   ASSERT_FALSE(S.getError());
   EXPECT_EQ("//root/foo/bar/a", S->getName());
-  EXPECT_TRUE(S->ExposesExternalVFSPath);
+  EXPECT_TRUE(S->IsVFSMapped);
 
   ErrorOr SLower = O->status("//root/foo/bar/a");
   EXPECT_EQ("//root/foo/bar/a", SLower->getName());
   EXPECT_TRUE(S->equivalent(*SLower));
-  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
+  EXPECT_FALSE(SLower->IsVFSMapped);
 
   // file after opening
   auto OpenedF = O->openFileForRead("//root/file1");
@@ -1455,7 +1455,7 @@
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
 
   // directory
   S = O->status("//root/");
@@ -1467,27 +1467,27 @@
   S = O->status("//root/mappeddir");
   ASSERT_FALSE(S.getError());
   EXPECT_TRUE(S->isDirectory());
-  EXPECT_TRUE(S->ExposesExternalVFSPath);
+  EXPECT_TRUE(S->IsVFSMapped);
   EXPECT_TRUE(S->equivalent(*O->status("//root/foo/bar")));
 
   SLower = O->status("//root/foo/bar");
   EXPECT_EQ("//root/foo/bar", SLower->getName());
   EXPECT_TRUE(S->equivalent(*SLower));
-  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
+  EXPECT_FALSE(SLower->IsVFSMapped);
 
   // file in remapped directory
   S = O->status("//root/mappeddir/a");
   ASSERT_FALSE(S.getError());
-  EXPECT_FALSE(S->isDirectory());
-  EXPECT_TRUE(S->ExposesExternalVFSPath);
-  EXPECT_EQ("//root/foo/bar/a", S->getName());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/foo/bar/a", S->getName());
 
   // file in remapped directory, with use-external-name=false
   S = O->status("//root/mappeddir2/a");
   ASSERT_FALSE(S.getError());
-  EXPECT_FALSE(S->isDirectory());
-  EXPECT_FALSE(S->ExposesExternalVFSPath);
-  EXPECT_EQ("//root/mappeddir2/a", S->getName());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/mappeddir2/a", S->getName());
 
   // file contents in remapped directory
   OpenedF = O->openFileForRead("//root/mappeddir/a");
@@ -1495,7 +1495,7 @@
   OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
 
   // file contents in remapped directory, with use-external-name=false
   OpenedF = O->openFileForRead("//root/mappeddir2/a");
@@ -1503,7 +1503,7 @@
   OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName());
-  EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
 
   // broken mapping
   EXPECT_EQ(O->status("//root/file2").getError(),
@@ -1535,12 +1535,12 @@
   ErrorOr S = O->status("//mappedroot/a");
   ASSERT_FALSE(S.getError());
   EXPECT_EQ("//root/foo/bar/a", S->getName());
-  EXPECT_TRUE(S->ExposesExternalVFSPath);
+  EXPECT_TRUE(S->IsVFSMapped);
 
   ErrorOr SLower = O->status("//root/foo/bar/a");
   EXPECT_EQ("//root/foo/bar/a", SLower->getName());
   EXPECT_TRUE(S->equivalent(*SLower));
-  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
+  EXPECT_FALSE(SLower->IsVFSMapped);
 
   // file after opening
   auto OpenedF = O->openFileForRead("//mappedroot/a");
@@ -1548,7 +1548,7 @@
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
 
   EXPECT_EQ(0,

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in FILE, __builtin_FILE(), and std::source_location

2022-04-04 Thread Alan Zhao via Phabricator via cfe-commits

ayzhao updated this revision to Diff 420350.
ayzhao added a comment.
Herald added a subscriber: dexonsmith.

Extract logic into a separate function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/file_test_windows.c

STAMPS
actor(@ayzhao) application(Differential) author(@ayzhao) herald(H208) 
herald(H223) herald(H225) herald(H254) herald(H255) herald(H259) herald(H263) 
herald(H277) herald(H278) herald(H288) herald(H310) herald(H311) herald(H313) 
herald(H322) herald(H337) herald(H349) herald(H351) herald(H352) herald(H354) 
herald(H356) herald(H371) herald(H376) herald(H423) herald(H465) herald(H480) 
herald(H515) herald(H519) herald(H524) herald(H576) herald(H615) herald(H688) 
herald(H696) herald(H849) herald(H856) herald(H864) monogram(D122766) 
object-type(DREV) phid(PHID-DREV-5h6hjc7p7oodu7braqun) reviewer(@aaron.ballman) 
reviewer(@hans) reviewer(@mstorsjo) reviewer(@thakis) revision-repository(rG) 
revision-status(needs-review) subscriber(@apazos) subscriber(@arichardson) 
subscriber(@asb) subscriber(@brucehoult) subscriber(@cfe-commits) 
subscriber(@dexonsmith) subscriber(@edward-jones) subscriber(@frasercrmck) 
subscriber(@hiraditya) subscriber(@Jim) subscriber(@jocewei) 
subscriber(@johnrusso) subscriber(@jrtc27) subscriber(@llvm-commits) 
subscriber(@luismarques) subscriber(@MartinMosbeck) subscriber(@MaskRay) 
subscriber(@niosHD) subscriber(@pcwang-thead) subscriber(@PkmX) 
subscriber(@rbar) subscriber(@rnk) subscriber(@rogfer01) subscriber(@s.egerton) 
subscriber(@sabuasal) subscriber(@sameer.abuasal) subscriber(@simoncook) 
subscriber(@the_o) subscriber(@zzheng) tag(#all) tag(#clang) tag(#llvm) 
via(conduit)

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -8,19 +8,19 @@
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
 
-// CHECK: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
-// CHECK: filename: "A:\\UNLIKELY_PATH\\empty/Inputs/include-file-test/file_test.h"
-// CHECK: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK: filename: "A:/UNLIKELY_PATH/empty/file_test_windows.c"
+// CHECK: filename: "A:/UNLIKELY_PATH/empty/Inputs/include-file-test/file_test.h"
+// CHECK: basefile: "A:/UNLIKELY_PATH/empty/file_test_windows.c"
 // CHECK-NOT: filename:
 
-// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
-// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
-// CHECK-EVIL: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL: filename: "A:/UNLIKELY_PATH=empty/file_test_windows.c"
+// CHECK-EVIL: filename: "A:/UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
+// CHECK-EVIL: basefile: "A:/UNLIKELY_PATH=empty/file_test_windows.c"
 // CHECK-EVIL-NOT: filename:
 
-// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
-// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test/file_test.h"
-// CHECK-CASE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE: filename: "A:/UNLIKELY_PATH_BASE/file_test_windows.c"
+// CHECK-CASE: filename: "A:/UNLIKELY_PATH_INC/include-file-test/file_test.h"
+// CHECK-CASE: basefile: "A:/UNLIKELY_PATH_BASE/file_test_windows.c"
 // CHECK-CASE-NOT: filename:
 
 // CHECK-REMOVE: filename: "file_test_windows.c"
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1511,7 +1511,7 @@
   } else {
 FN += PLoc.getFilename();
   }
-  getLangOpts().remapPathPrefix(FN);
+  processPathForFileMacro(FN, getLangOpts(), getTargetInfo());
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
@@ -1886,3 +1886,18 @@
 WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
   MI->setIsUsed(true);
 }
+
+void Preprocessor::processPathForFileMacro(SmallVectorImpl ,
+   const LangOptions ,
+   const TargetInfo ) {
+  LangOpts.remapPathPrefix(Path);
+  if (TI.getTriple().isOSWindows()) {
+// The path separator character depends on the environment where Clang
+// _itself_ is built (backslash for Windows, forward slash for *nix). To
+// make the output of the __FILE__ macro deterministic, we make the path
+// use forward slashes when targeting Windows independently of how Clang is
+// built.
+llvm::sys::path::make_preferred(Path,
+

[PATCH] D123101: [clang] Use forward slash as the path separator for Windows in FILE, __builtin_FILE(), and std::source_location

2022-04-04 Thread Alan Zhao via Phabricator via cfe-commits

ayzhao abandoned this revision.
ayzhao added a comment.

Accidental duplicate of D122766 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123101

STAMPS
actor(@ayzhao) application(Differential) author(@ayzhao) herald(H223) 
herald(H337) herald(H423) herald(H576) herald(H688) herald(H864) 
monogram(D123101) object-type(DREV) phid(PHID-DREV-zhl7ek66tkkebw7mewsb) 
revision-repository(rG) revision-status(abandoned) subscriber(@cfe-commits) 
subscriber(@dexonsmith) tag(#all) tag(#clang) via(web)

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

[PATCH] D123101: [clang] Use forward slash as the path separator for Windows in FILE, __builtin_FILE(), and std::source_location

2022-04-04 Thread Alan Zhao via Phabricator via cfe-commits

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

When targeting Windows, the path separator used when targeting Windows
depends on the build environment when Clang _itself_ is built. This
leads to inconsistencies in Chrome builds where Clang running on
non-Windows environments uses the forward slash (/) path separator
while Clang running on Windows builds uses the backslash (\) path
separator. To fix this, we make Clang use forward slashes whenever it is
targeting Windows.

We chose to use forward slashes instead of backslashes in builds
targeting Windows because according to the C standard, backslashes in
\#include directives result in undefined behavior[1]. We may change this
decision though depending on the review.

[0]: https://crbug.com/1310767
[1]: https://stackoverflow.com/a/5790259


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123101

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/file_test_windows.c

STAMPS
actor(@ayzhao) application(Differential) author(@ayzhao) herald(H223) 
herald(H337) herald(H423) herald(H576) herald(H688) herald(H864) 
monogram(D123101) object-type(DREV) phid(PHID-DREV-zhl7ek66tkkebw7mewsb) 
revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) 
subscriber(@dexonsmith) tag(#all) tag(#clang) via(conduit)

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -8,19 +8,19 @@
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
 
-// CHECK: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
-// CHECK: filename: "A:\\UNLIKELY_PATH\\empty/Inputs/include-file-test/file_test.h"
-// CHECK: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK: filename: "A:/UNLIKELY_PATH/empty/file_test_windows.c"
+// CHECK: filename: "A:/UNLIKELY_PATH/empty/Inputs/include-file-test/file_test.h"
+// CHECK: basefile: "A:/UNLIKELY_PATH/empty/file_test_windows.c"
 // CHECK-NOT: filename:
 
-// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
-// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
-// CHECK-EVIL: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL: filename: "A:/UNLIKELY_PATH=empty/file_test_windows.c"
+// CHECK-EVIL: filename: "A:/UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
+// CHECK-EVIL: basefile: "A:/UNLIKELY_PATH=empty/file_test_windows.c"
 // CHECK-EVIL-NOT: filename:
 
-// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
-// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test/file_test.h"
-// CHECK-CASE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE: filename: "A:/UNLIKELY_PATH_BASE/file_test_windows.c"
+// CHECK-CASE: filename: "A:/UNLIKELY_PATH_INC/include-file-test/file_test.h"
+// CHECK-CASE: basefile: "A:/UNLIKELY_PATH_BASE/file_test_windows.c"
 // CHECK-CASE-NOT: filename:
 
 // CHECK-REMOVE: filename: "file_test_windows.c"
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1511,7 +1511,7 @@
   } else {
 FN += PLoc.getFilename();
   }
-  getLangOpts().remapPathPrefix(FN);
+  processPathForFileMacro(FN, getLangOpts(), getTargetInfo());
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
@@ -1886,3 +1886,18 @@
 WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
   MI->setIsUsed(true);
 }
+
+void Preprocessor::processPathForFileMacro(SmallVectorImpl ,
+   const LangOptions ,
+   const TargetInfo ) {
+  LangOpts.remapPathPrefix(Path);
+  if (TI.getTriple().isOSWindows()) {
+// The path separator character depends on the environment where Clang
+// _itself_ is built (backslash for Windows, forward slash for *nix). To
+// make the output of the __FILE__ macro deterministic, we make the path
+// use forward slashes when targeting Windows independently of how Clang is
+// built.
+llvm::sys::path::make_preferred(Path,
+llvm::sys::path::Style::windows_slash);
+  }
+}
Index: clang/lib/Basic/LangOptions.cpp
===
--- clang/lib/Basic/LangOptions.cpp
+++ clang/lib/Basic/LangOptions.cpp
@@ -62,7 +62,7 @@
   llvm_unreachable("Unknown OpenCL version");
 }
 
-void LangOptions::remapPathPrefix(SmallString<256>

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi created this revision.
Herald added subscribers: ayermolo, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, 
dcaballe, cota, teijeong, dexonsmith, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, hiraditya, arichardson, emaste.
Herald added a reviewer: jhenderson.
Herald added a reviewer: rriddle.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
akyrtzi requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, yota9, StephenFan, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

Returning `std::array` is better ergonomics for the hashing 
functions usage, instead of a `StringRef`:

- When returning `StringRef`, client code is "jumping through hoops" to do 
string manipulations instead of dealing with array of bytes directly, which is 
more natural
- Returning `std::array` avoids the need for the hasher classes to 
keep a field just for the purpose of wrapping it and returning it as a 
`StringRef`

As part of this patch also change the `BLAKE3` class API to match 
`HashBuilder`'s generic hash API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123100

Files:
  bolt/lib/Core/DebugData.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Serialization/ASTWriter.cpp
  lld/ELF/Writer.cpp
  lld/MachO/SyntheticSections.cpp
  llvm/include/llvm/Support/BLAKE3.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/include/llvm/Support/MD5.h
  llvm/include/llvm/Support/SHA1.h
  llvm/include/llvm/Support/SHA256.h
  llvm/include/llvm/Support/raw_sha1_ostream.h
  llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/DebugInfo/CodeView/TypeHashing.cpp
  llvm/lib/ObjCopy/MachO/MachOWriter.cpp
  llvm/lib/Support/BLAKE3/README.md
  llvm/lib/Support/MD5.cpp
  llvm/lib/Support/SHA1.cpp
  llvm/lib/Support/SHA256.cpp
  llvm/unittests/Support/BLAKE3Test.cpp
  llvm/unittests/Support/HashBuilderTest.cpp
  llvm/unittests/Support/MD5Test.cpp
  llvm/unittests/Support/SHA256.cpp
  llvm/unittests/Support/raw_sha1_ostream_test.cpp
  mlir/lib/Pass/IRPrinting.cpp

STAMPS
actor(@akyrtzi) application(Differential) author(@akyrtzi) herald(H105) 
herald(H123) herald(H208) herald(H224) herald(H225) herald(H311) herald(H423) 
herald(H452) herald(H530) herald(H532) herald(H533) herald(H538) herald(H540) 
herald(H541) herald(H544) herald(H546) herald(H547) herald(H550) herald(H551) 
herald(H554) herald(H557) herald(H560) herald(H561) herald(H565) herald(H576) 
herald(H592) herald(H597) herald(H598) herald(H607) herald(H610) herald(H615) 
herald(H625) herald(H629) herald(H645) herald(H672) herald(H685) herald(H688) 
herald(H698) herald(H723) herald(H727) herald(H729) herald(H739) herald(H802) 
herald(H805) herald(H809) herald(H827) herald(H842) herald(H844) herald(H845) 
herald(H847) herald(H854) herald(H855) herald(H861) herald(H864) 
monogram(D123100) object-type(DREV) phid(PHID-DREV-ngvkfimyomxukhcmhald) 
reviewer(#lld-macho) reviewer(@Amir) reviewer(@jhenderson) reviewer(@maksfb) 
reviewer(@MaskRay) reviewer(@rafauler) reviewer(@rriddle) 
revision-repository(rG) revision-status(needs-review) subscriber(@aartbik) 
subscriber(@antiagainst) subscriber(@arichardson) subscriber(@arpith-jacob) 
subscriber(@ayermolo) subscriber(@cfe-commits) subscriber(@Chia-hungDuan) 
subscriber(@cota) subscriber(@dcaballe) subscriber(@dexonsmith) 
subscriber(@emaste) subscriber(@grosul1) subscriber(@hiraditya) 
subscriber(@Joonsoo) subscriber(@jurahul) subscriber(@Kayjukh) 
subscriber(@liufengdb) subscriber(@llvm-commits) subscriber(@mehdi_amini) 
subscriber(@mgester) subscriber(@msifontes) subscriber(@nicolasvasilache) 
subscriber(@rdzhabarov) subscriber(@rriddle) subscriber(@sdasgup3) 
subscriber(@shauheen) subscriber(@StephenFan) subscriber(@stephenneuendorffer) 
subscriber(@tatianashp) subscriber(@teijeong) subscriber(@wenzhicui) 
subscriber(@wrengr) subscriber(@yota9) tag(#all) tag(#clang) tag(#lld-macho) 
tag(#llvm) tag(#mlir) via(conduit)

Index: mlir/lib/Pass/IRPrinting.cpp
===
--- mlir/lib/Pass/IRPrinting.cpp
+++ mlir/lib/Pass/IRPrinting.cpp
@@ -66,7 +66,7 @@
 ArrayRef(reinterpret_cast(), sizeof(T)));
   }
 
-  SmallString<20> hash;
+  std::array hash;
 };
 
 //===--===//
Index: llvm/unittests/Support/raw_sha1_ostream_test.cpp
===
--- llvm/unittests/Support/raw_sha1_ostream_test.cpp
+++ llvm/unittests/Support/raw_sha1_ostream_test.cpp
@@ -14,7 +14,7 @@
 
 using namespace llvm;
 
-static std::string toHex(StringRef Input) {
+static std::string toHex(ArrayRef Input) {

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D118948#3427401 , @hctim wrote:

> In D118948#3427344 , @tschuett 
> wrote:
>
>> Is `-fsanitize=memtag-heap` Android specific or target independent? It 
>> passes Android flags to the linker?!?
>
> The frontend flag should be target-independent. The clang driver knows to 
> only pass Android flags to the linker where `TC.getTriple().isAndroid()` is 
> true. This should allow a glibc-based implementation to live under the same 
> `-fsanitize=memtag-heap` flag, just chainging the implementation based on the 
> target.

Since this driver option has no-op for glibc/musl, by convention it should 
report an `err_drv_unsupported_opt_for_target` error.
This allows configure-time detection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

STAMPS
actor(@MaskRay) application(Differential) author(@hctim) herald(H45) 
herald(H69) herald(H74) herald(H105) herald(H123) herald(H208) herald(H311) 
herald(H343) herald(H423) herald(H477) herald(H501) herald(H576) herald(H597) 
herald(H615) herald(H649) herald(H678) herald(H688) herald(H697) herald(H704) 
herald(H723) herald(H864) mention(@hctim) mention(@tschuett) monogram(D118948) 
object-type(DREV) phid(PHID-DREV-pdirnu4zuvlfhlklabzj) reviewer(@eugenis) 
reviewer(@MaskRay) revision-repository(rG) revision-status(needs-review) 
subscriber(@arichardson) subscriber(@cfe-commits) subscriber(@dang) 
subscriber(@dexonsmith) subscriber(@emaste) subscriber(@llvm-commits) 
subscriber(@pcc) subscriber(@StephenFan) subscriber(@tschuett) tag(#all) 
tag(#clang) tag(#llvm) via(web)

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

[PATCH] D121375: [clang] NFC, move the utility function CompilerInvocation::setLangDefaults to LangOptions.h

2022-04-04 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

This makes sense to me! A few comments inline.

Also, the description doesn't talk about timeline for removing the wrapper. 
Ideally we wouldn't leave behind the wrapper behind... just long enough that 
you can migrate the callers and delete the old function in separate incremental 
commit(s) (or if there are very few callers, all in this commit would be fine, 
but I'm guessing that's not the case here). Or were you thinking something else?




Comment at: clang/include/clang/Basic/LangOptions.h:511
 
+/// s language defaults for the given input language and language standard in
+/// the given LangOptions object.

Looks like a copy/paste typo in the first word.



Comment at: clang/include/clang/Basic/LangOptions.h:519-521
+void setLangDefaults(LangOptions , Language Lang, const llvm::Triple ,
+ std::vector ,
+ LangStandard::Kind LangStd);

I think this would be cleaner as:
```
lang=c++
class LangOpts {
// ...
  void setDefaults(Language Lang, const llvm::Triple , ...);
};
```
Or `setLangDefaults` or `setDefaultsFor` (I don't care about the name, just 
feel like it makes more sense as a member function if we're updating all the 
callers anyway).

Also, you should include a default for `LangStd` or it'll be hard to migrate 
over callers.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:222
 
-  /// Set language defaults for the given input language and
-  /// language standard in the given LangOptions object.
-  ///
-  /// \param Opts - The LangOptions object to set up.
-  /// \param IK - The input language.
-  /// \param T - The target triple.
-  /// \param Includes - The affected list of included files.
-  /// \param LangStd - The input language standard.
+  /// This is deprecated. Please use `clang::setLangDefaults` instead.
   static void

I suggest having this comment phrased to explain what it does before saying 
what people should do. Something like:
```
lang=c++
/// Forwards to [...].
///
/// TODO: Remove this wrapper once all callers have been updated.
```



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:223-226
   static void
   setLangDefaults(LangOptions , InputKind IK, const llvm::Triple ,
   std::vector ,
   LangStandard::Kind LangStd = LangStandard::lang_unspecified);

I think you should inline the new one-liner implementation into the header, 
documenting what people should do instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121375

STAMPS
actor(@dexonsmith) application(Differential) author(@hokein) herald(H75) 
herald(H337) herald(H423) herald(H438) herald(H576) herald(H628) herald(H688) 
herald(H864) monogram(D121375) object-type(DREV) 
phid(PHID-DREV-3totfj2qha3xuifsvz6o) reviewer(@dexonsmith) reviewer(@jdoerfert) 
reviewer(@sammccall) revision-repository(rG) revision-status(needs-revision) 
subscriber(@cfe-commits) subscriber(@dexonsmith) subscriber(@sstefan1) 
tag(#all) tag(#clang) via(web)

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

[PATCH] D123097: [RISCV] Remove redundant enabling of IAS for Clang

2022-04-04 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: StephenFan.

It's conventional to add NFC to the subject line to indicate "no functional 
change".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123097

STAMPS
actor(@MaskRay) application(Differential) author(@brad) herald(H208) 
herald(H254) herald(H255) herald(H259) herald(H263) herald(H277) herald(H278) 
herald(H288) herald(H300) herald(H303) herald(H310) herald(H311) herald(H313) 
herald(H322) herald(H349) herald(H351) herald(H352) herald(H354) herald(H356) 
herald(H371) herald(H376) herald(H423) herald(H447) herald(H449) herald(H465) 
herald(H477) herald(H480) herald(H485) herald(H515) herald(H519) herald(H524) 
herald(H563) herald(H576) herald(H686) herald(H696) herald(H721) herald(H723) 
herald(H825) herald(H826) herald(H843) herald(H849) herald(H856) herald(H864) 
herald(H888) monogram(D123097) object-type(DREV) 
phid(PHID-DREV-jjml2cj3psrp4dk2ehp7) reviewer(@kito-cheng) reviewer(@MaskRay) 
revision-repository(rG) revision-status(accepted) subscriber(@apazos) 
subscriber(@arichardson) subscriber(@asb) subscriber(@benna) 
subscriber(@brucehoult) subscriber(@cfe-commits) subscriber(@edward-jones) 
subscriber(@eopXD) subscriber(@evandro) subscriber(@frasercrmck) 
subscriber(@Jim) subscriber(@jocewei) subscriber(@johnrusso) 
subscriber(@jrtc27) subscriber(@luismarques) subscriber(@luke957) 
subscriber(@MartinMosbeck) subscriber(@MaskRay) subscriber(@niosHD) 
subscriber(@pcwang-thead) subscriber(@PkmX) subscriber(@psnobl) 
subscriber(@rbar) subscriber(@rogfer01) subscriber(@s.egerton) 
subscriber(@sabuasal) subscriber(@sameer.abuasal) subscriber(@simoncook) 
subscriber(@StephenFan) subscriber(@sunshaoce) subscriber(@the_o) 
subscriber(@VincentWu) subscriber(@vkmr) subscriber(@zzheng) tag(#all) 
tag(#clang) via(web)

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

[PATCH] D123097: [RISCV] Remove redundant enabling of IAS for Clang

2022-04-04 Thread Brad Smith via Phabricator via cfe-commits

brad created this revision.
brad added a reviewer: kito-cheng.
brad added a project: clang.
Herald added subscribers: sunshaoce, VincentWu, luke957, vkmr, frasercrmck, 
evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
brad requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.

Generic_GCC::IsIntegratedAssemblerDefault() already takes care of RISCV. Remove 
the redundant enabling of IAS with Clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123097

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.h


Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -22,7 +22,6 @@
  const llvm::opt::ArgList );
 
   static bool hasGCCToolchain(const Driver , const llvm::opt::ArgList );
-  bool IsIntegratedAssemblerDefault() const override { return true; }
   void addClangTargetOptions(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
  Action::OffloadKind) const override;


STAMPS
actor(@brad) application(Differential) author(@brad) herald(H208) herald(H254) 
herald(H255) herald(H259) herald(H263) herald(H277) herald(H278) herald(H288) 
herald(H300) herald(H303) herald(H310) herald(H311) herald(H313) herald(H322) 
herald(H349) herald(H351) herald(H352) herald(H354) herald(H356) herald(H371) 
herald(H376) herald(H423) herald(H447) herald(H449) herald(H465) herald(H477) 
herald(H480) herald(H485) herald(H515) herald(H519) herald(H524) herald(H563) 
herald(H576) herald(H686) herald(H696) herald(H721) herald(H825) herald(H826) 
herald(H843) herald(H849) herald(H856) herald(H864) herald(H888) 
monogram(D123097) object-type(DREV) phid(PHID-DREV-jjml2cj3psrp4dk2ehp7) 
reviewer(@kito-cheng) revision-repository(rG) revision-status(needs-review) 
subscriber(@apazos) subscriber(@arichardson) subscriber(@asb) 
subscriber(@benna) subscriber(@brucehoult) subscriber(@cfe-commits) 
subscriber(@edward-jones) subscriber(@eopXD) subscriber(@evandro) 
subscriber(@frasercrmck) subscriber(@Jim) subscriber(@jocewei) 
subscriber(@johnrusso) subscriber(@jrtc27) subscriber(@luismarques) 
subscriber(@luke957) subscriber(@MartinMosbeck) subscriber(@MaskRay) 
subscriber(@niosHD) subscriber(@pcwang-thead) subscriber(@PkmX) 
subscriber(@psnobl) subscriber(@rbar) subscriber(@rogfer01) 
subscriber(@s.egerton) subscriber(@sabuasal) subscriber(@sameer.abuasal) 
subscriber(@simoncook) subscriber(@sunshaoce) subscriber(@the_o) 
subscriber(@VincentWu) subscriber(@vkmr) subscriber(@zzheng) tag(#all) 
tag(#clang) via(web)

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -22,7 +22,6 @@
  const llvm::opt::ArgList );
 
   static bool hasGCCToolchain(const Driver , const llvm::opt::ArgList );
-  bool IsIntegratedAssemblerDefault() const override { return true; }
   void addClangTargetOptions(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
  Action::OffloadKind) const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Victoria Mitchell via Phabricator via cfe-commits

QuietMisdreavus added a comment.

In D123045#3427992 , @zixuw wrote:

> In D123045#3427699 , 
> @QuietMisdreavus wrote:
>
>> After a quick scan comparing the current output of these symbol graphs with 
>> the primary library used for reading them 
>> , the last thing i can spot 
>> that's "off" is that the "function signature" is currently being serialized 
>> under a `parameters` field instead of the required `functionSignature`.
>
> Hmm, I was looking at the format specification at 
> https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L307,
>  and I searched the term `functionSignature` in the spec but found no 
> property with that name (except for the `FunctionSignature` schema that the 
> `parameters` property is referring to). But anyway this should be a easy fix.

It seems like the specification and implementation have diverged. The parser in 
swift-docc-symbolkit is looking for a `functionSignature` field by virtue of 
how it names its "coding key" 
.
 By comparison, here is the functionSignature emitter in the Swift symbol-graph 
generator 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

STAMPS
actor(@QuietMisdreavus) application(Differential) author(@dang) herald(H423) 
herald(H576) herald(H864) herald(H875) herald(H876) mention(@QuietMisdreavus) 
mention(@zixuw) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

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

[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

2022-04-04 Thread Fabio Rossini Sluzala via Phabricator via cfe-commits

FabioRS updated this revision to Diff 420336.
FabioRS added a comment.

Thanks!

I removed the excess setting/resseting of the SyntacticDC and SemanticDC

  ExtractedFunc.SyntacticDC =
  ExtZone.EnclosingFunction->getLexicalDeclContext();
  ExtractedFunc.SemanticDC = ExtZone.EnclosingFunction->getDeclContext();



  if (const auto *Method =
  llvm::dyn_cast(ExtZone.EnclosingFunction))
captureMethodInfo(ExtractedFunc, Method);

I place the  ExtractedFunc.ForwardDeclarationSyntacticDC = 
ExtractedFunc.SemanticDC inside the if 
(ExtZone.EnclosingFunction->isOutOfLine()) branch since the out-of-line ns:f() 
needs it too. What do you think?


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

https://reviews.llvm.org/D122698

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp

STAMPS
actor(@FabioRS) application(Differential) author(@FabioRS) herald(H243) 
herald(H311) herald(H365) herald(H378) herald(H391) herald(H502) herald(H576) 
herald(H744) herald(H864) monogram(D122698) object-type(DREV) 
phid(PHID-DREV-cfxk4igkfserwacndt5w) reviewer(@sammccall) 
revision-status(accepted) subscriber(@arphaman) subscriber(@cfe-commits) 
subscriber(@ilya-biryukov) subscriber(@kadircet) subscriber(@MaskRay) 
subscriber(@nridge) subscriber(@usaxena95) tag(#all) tag(#clang-tools-extra) 
via(web)

Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -114,16 +114,48 @@
   // Don't extract when we need to make a function as a parameter.
   EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
 
-  // We don't extract from methods for now since they may involve multi-file
-  // edits
-  std::string MethodFailInput = R"cpp(
+  std::string MethodInput = R"cpp(
 class T {
   void f() {
 [[int x;]]
   }
 };
   )cpp";
-  EXPECT_EQ(apply(MethodFailInput), "unavailable");
+  std::string MethodCheckOutput = R"cpp(
+class T {
+  void extracted() {
+int x;
+}
+void f() {
+extracted();
+  }
+};
+  )cpp";
+  EXPECT_EQ(apply(MethodInput), MethodCheckOutput);
+
+  std::string OutOfLineMethodInput = R"cpp(
+class T {
+  void f();
+};
+
+void T::f() {
+  [[int x;]]
+}
+  )cpp";
+  std::string OutOfLineMethodCheckOutput = R"cpp(
+class T {
+  void extracted();
+void f();
+};
+
+void T::extracted() {
+int x;
+}
+void T::f() {
+  extracted();
+}
+  )cpp";
+  EXPECT_EQ(apply(OutOfLineMethodInput), OutOfLineMethodCheckOutput);
 
   // We don't extract from templated functions for now as templates are hard
   // to deal with.
@@ -159,6 +191,333 @@
   EXPECT_EQ(apply(CompoundFailInput), "unavailable");
 }
 
+TEST_F(ExtractFunctionTest, DifferentHeaderSourceTest) {
+  Header = R"cpp(
+class SomeClass {
+  void f();
+};
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+void SomeClass::f() {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+void SomeClass::extracted() {
+int x;
+}
+void SomeClass::f() {
+  extracted();
+}
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+class SomeClass {
+  void extracted();
+void f();
+};
+  )cpp";
+
+  llvm::StringMap EditedFiles;
+
+  EXPECT_EQ(apply(OutOfLineSource, ), OutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, HeaderOutputCheck);
+}
+
+TEST_F(ExtractFunctionTest, DifferentFilesNestedTest) {
+  Header = R"cpp(
+class T {
+class SomeClass {
+  void f();
+};
+};
+  )cpp";
+
+  std::string NestedOutOfLineSource = R"cpp(
+void T::SomeClass::f() {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string NestedOutOfLineSourceOutputCheck = R"cpp(
+void T::SomeClass::extracted() {
+int x;
+}
+void T::SomeClass::f() {
+  extracted();
+}
+  )cpp";
+
+  std::string NestedHeaderOutputCheck = R"cpp(
+class T {
+class SomeClass {
+  void extracted();
+void f();
+};
+};
+  )cpp";
+
+  llvm::StringMap EditedFiles;
+
+  EXPECT_EQ(apply(NestedOutOfLineSource, ),
+NestedOutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, NestedHeaderOutputCheck);
+}
+
+TEST_F(ExtractFunctionTest, ConstexprDifferentHeaderSourceTest) {
+  Header = R"cpp(
+class SomeClass {
+  constexpr void f() const;
+};
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+constexpr void SomeClass::f() const {
+  [[int x;]]
+}
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+constexpr void SomeClass::extracted() const {
+int x;
+}
+constexpr void SomeClass::f() const {
+  extracted();
+}
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+class SomeClass {
+

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

zixuw wrote:
> QuietMisdreavus wrote:
> > zixuw wrote:
> > > What's the cost/would it worth it to wrap the `emplace_back`s and 
> > > `pop_back`s of `PathComponentContext` in meaningful APIs like 
> > > `enterPathComponentContext` and `exitPathComponentContext`?
> > > That way the code is more self-explanatory and easier to read. It took me 
> > > some time and mental resources to figure out why the `pop_back` is placed 
> > > here.
> > What's the use of having the `emplace_back` call inside 
> > `serializeAPIRecord` but to pop it outside? It seems like it's easier to 
> > mess up for new kinds of records.
> The reason to `emplace_back` the path component inside `serializeAPIRecord` 
> is that the `pathComponent` field of the `Record` is serialized in there so 
> you want to include the name of the symbol itself in the path component list.
> The `pop_back` is delayed til all other additional serialization for a 
> specific kind of record, for example `serializeEnumRecord` handles all the 
> enum cases after processing the enum record itself using 
> `serializeAPIRecord`. So in order to correctly include the enum name in the 
> path components of all the enum cases, the enum name has to stay in 
> `PathComponentContext` until all members are serialized.
> 
> This is exactly the reason why I wanted a clearer API to make it easier to 
> see.
Hmm now that I thought about this, it seems that it would be easier to 
understand and avoid bugs if we lift 
`PathComponentContext.emplace_back`/`enterPathComponentContext` out of 
`serializeAPIRecord`, because we have access to the record name before that 
anyway.

So we establish a pre-condition of `serializeAPIRecord` that the correct path 
components would be set up in `PathComponentContext` before the call so we 
could still serialize the field inside that method. And in specific 
`serialize*Record` methods we push the record name, and pop out at the end.

This way the push and pop operations would appear in pairs in a single block, 
saving the confusion and mental work of jumping across functions to see how 
`PathComponentContext` is evolving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

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

[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

2022-04-04 Thread Fabio Rossini Sluzala via Phabricator via cfe-commits

FabioRS added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:765
+
+  ExtractedFunc.ForwardDeclarationSyntacticDC = ExtractedFunc.SemanticDC =
+  ExtractedFunc.SyntacticDC = ExtZone.EnclosingFunction->getDeclContext();

sammccall wrote:
> You're setting/resetting these in lots of different places, but no need for 
> that:
> 
> SyntacticDC = EnclosingFunction->getLexicalDeclContext()
> SemanticDC = getDeclContext();
> 
> and set ForwardDeclarationSyntacticDC in captureMethodInfo(), leave it null 
> if this isn't a method.
Thanks!
The out-of-line ns:f() needs the ForwardDeclarationSyntacticDC too, I think 
inside the branch if (ExtZone.EnclosingFunction->isOutOfLine()) is the place to 
put it, I will submit a diff.



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

https://reviews.llvm.org/D122698

STAMPS
actor(@FabioRS) application(Differential) author(@FabioRS) herald(H243) 
herald(H311) herald(H365) herald(H378) herald(H391) herald(H502) herald(H576) 
herald(H744) herald(H864) monogram(D122698) object-type(DREV) 
phid(PHID-DREV-cfxk4igkfserwacndt5w) reviewer(@sammccall) 
revision-status(accepted) subscriber(@arphaman) subscriber(@cfe-commits) 
subscriber(@ilya-biryukov) subscriber(@kadircet) subscriber(@MaskRay) 
subscriber(@nridge) subscriber(@usaxena95) tag(#all) tag(#clang-tools-extra) 
via(web)

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

[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

2022-04-04 Thread Fabio Rossini Sluzala via Phabricator via cfe-commits

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

In D122698#3426237 , @sammccall wrote:

> Thanks, this looks great! Just a couple of nits left.
> Appreciate you fixing the out-of-line `ns::f()` case too. It's easier to 
> understand the fixed logic than the broken logic.
>
> I guess you don't have commit access yet, I can land this for you if you 
> like. Let me know your preferred name/email for attribution.

Thanks! I'm glad to help.

I don't have commit access. You can land it for me please, thanks!
Name: Fabio Rossini Sluzala
e-mail: fabio...@gmail.com


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

https://reviews.llvm.org/D122698

STAMPS
actor(@FabioRS) application(Differential) author(@FabioRS) herald(H243) 
herald(H311) herald(H365) herald(H378) herald(H391) herald(H502) herald(H576) 
herald(H744) herald(H864) mention(@sammccall) monogram(D122698) 
object-type(DREV) phid(PHID-DREV-cfxk4igkfserwacndt5w) reviewer(@sammccall) 
revision-status(accepted) subscriber(@arphaman) subscriber(@cfe-commits) 
subscriber(@ilya-biryukov) subscriber(@kadircet) subscriber(@MaskRay) 
subscriber(@nridge) subscriber(@usaxena95) tag(#all) tag(#clang-tools-extra) 
via(web)

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

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw updated this revision to Diff 420333.
zixuw marked an inline comment as done.
zixuw added a comment.

Address review comment:

- Use `llvm_unreachable` for the Objective-C category case in 
`serializeSymbolKind`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

STAMPS
actor(@zixuw) application(Differential) author(@zixuw) herald(H423) 
herald(H576) herald(H864) herald(H875) monogram(D122774) object-type(DREV) 
phid(PHID-DREV-o2c75ab3w5lvo3s7frw5) reviewer(@dang) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) tag(#all) tag(#clang) via(conduit)

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %t/reference.output.json
+// RUN: %clang -extract-api -x objective-c-header -target arm64-apple-macosx \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+@protocol Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@end
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling":

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

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



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:396
 break;
+  case APIRecord::RK_ObjCCategory:
+Kind["identifier"] = AddLangPrefix("category");

dang wrote:
> Since we currently never actually emit a standalone category symbol object, 
> should we not just label this as `llvm_unreachable` with a comment explaining 
> why that is?
Yep, makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122774

STAMPS
actor(@zixuw) application(Differential) author(@zixuw) herald(H423) 
herald(H576) herald(H864) herald(H875) monogram(D122774) object-type(DREV) 
phid(PHID-DREV-o2c75ab3w5lvo3s7frw5) reviewer(@dang) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) tag(#all) tag(#clang) via(web)

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

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

QuietMisdreavus wrote:
> zixuw wrote:
> > What's the cost/would it worth it to wrap the `emplace_back`s and 
> > `pop_back`s of `PathComponentContext` in meaningful APIs like 
> > `enterPathComponentContext` and `exitPathComponentContext`?
> > That way the code is more self-explanatory and easier to read. It took me 
> > some time and mental resources to figure out why the `pop_back` is placed 
> > here.
> What's the use of having the `emplace_back` call inside `serializeAPIRecord` 
> but to pop it outside? It seems like it's easier to mess up for new kinds of 
> records.
The reason to `emplace_back` the path component inside `serializeAPIRecord` is 
that the `pathComponent` field of the `Record` is serialized in there so you 
want to include the name of the symbol itself in the path component list.
The `pop_back` is delayed til all other additional serialization for a specific 
kind of record, for example `serializeEnumRecord` handles all the enum cases 
after processing the enum record itself using `serializeAPIRecord`. So in order 
to correctly include the enum name in the path components of all the enum 
cases, the enum name has to stay in `PathComponentContext` until all members 
are serialized.

This is exactly the reason why I wanted a clearer API to make it easier to see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

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

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added a comment.

In D123045#3427699 , @QuietMisdreavus 
wrote:

> After a quick scan comparing the current output of these symbol graphs with 
> the primary library used for reading them 
> , the last thing i can spot 
> that's "off" is that the "function signature" is currently being serialized 
> under a `parameters` field instead of the required `functionSignature`.

Hmm, I was looking at the format specification at 
https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L307,
 and I searched the term `functionSignature` in the spec but found no property 
with that name (except for the `FunctionSignature` schema that the `parameters` 
property is referring to). But anyway this should be a easy fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) mention(@QuietMisdreavus) 
monogram(D123045) object-type(DREV) phid(PHID-DREV-d5goxxqici5xa3w2bgjy) 
reviewer(@QuietMisdreavus) reviewer(@ributzka) reviewer(@zixuw) 
revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) 
tag(#all) tag(#clang) via(web)

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 420332.
void added a comment.

Second attempt to fix Windows errors. Expecting one more iteration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

STAMPS
actor(@void) application(Differential) author(@void) herald(H157) herald(H311) 
herald(H337) herald(H343) herald(H423) herald(H477) herald(H576) herald(H617) 
herald(H638) herald(H649) herald(H678) herald(H688) herald(H864) 
monogram(D121556) object-type(DREV) phid(PHID-DREV-pdinu5qraknmmd3qzmtm) 
reviewer(@aaron.ballman) reviewer(@connorkuehl) reviewer(@jfb) 
reviewer(@rsmith) reviewer(@timpugh) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) subscriber(@connorkuehl) 
subscriber(@daloni) subscriber(@Dan) subscriber(@dang) subscriber(@dexonsmith) 
subscriber(@ebevhan) subscriber(@hintonda) subscriber(@jdoerfert) 
subscriber(@kees) subscriber(@MaskRay) subscriber(@mgorny) 
subscriber(@nickdesaulniers) subscriber(@pcc) subscriber(@riccibruno) 
subscriber(@shawnl) subscriber(@tschuett) subscriber(@vlad.tsyrklevich) 
subscriber(@xbolva00) tag(#all) tag(#clang) via(conduit)

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,461 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::randstruct;
+
+using field_names = std::vector;
+
+namespace {
+
+std::unique_ptr makeAST(const std::string ) {
+  std::vector Args = getCommandLineArgsForTesting(Lang_C99);
+  Args.push_back("-frandomize-layout-seed=1234567890abcdef");
+
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(SourceCode, Args);
+
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+RecordDecl *getRecordDeclFromAST(const ASTContext , const std::string ) {
+  RecordDecl *RD = FirstDeclMatcher().match(
+  C.getTranslationUnitDecl(), recordDecl(hasName(Name)));
+  return RD;
+}
+
+std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+
+  return Fields;
+}
+
+bool isSubsequence(const field_names , const field_names ) {
+  unsigned SeqLen = Seq.size();
+  unsigned SubLen = Subseq.size();
+
+  bool IsSubseq = false;
+  for (unsigned I = 0; I < SeqLen; ++I)
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (unsigned J = 0; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+
+  return IsSubseq;
+}
+
+} // end anonymous namespace
+
+namespace clang {
+namespace ast_matchers {
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const field_names Seq = {"a", "b", "c", "d"};
+
+  ASSERT_TRUE(isSubsequence(Seq, {"b",

[PATCH] D123085: CGExprCXX: emit allocptr attributes for operator delete

2022-04-04 Thread Augie Fackler via Phabricator via cfe-commits

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

With this change, we're almost ready to drop the entire table out of
MemoryBuiltins.cpp and rely solely on attributes to detect allocator
functions.

Depends on D123084 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123085

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-aligned-allocation.cpp
  clang/test/CodeGenCXX/delete-two-arg.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/microsoft-abi-structors.cpp

STAMPS
actor(@durin42) application(Differential) author(@durin42) herald(H343) 
herald(H423) herald(H576) herald(H678) herald(H864) monogram(D123085) 
object-type(DREV) phid(PHID-DREV-cgee2pk3uj3hbw3hynvx) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(conduit)

Index: clang/test/CodeGenCXX/microsoft-abi-structors.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-structors.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-structors.cpp
@@ -57,7 +57,7 @@
 //
 // DTORS:  [[CALL_DELETE_LABEL]]
 // DTORS-NEXT:   %[[THIS_AS_VOID:[0-9a-z]+]] = bitcast %"struct.basic::C"* %[[THIS]] to i8*
-// DTORS-NEXT:   call void @"??3@YAXPAX@Z"(i8* %[[THIS_AS_VOID]])
+// DTORS-NEXT:   call void @"??3@YAXPAX@Z"(i8* allocptr %[[THIS_AS_VOID]])
 // DTORS-NEXT:   br label %[[CONTINUE_LABEL]]
 //
 // DTORS:  [[CONTINUE_LABEL]]
@@ -119,7 +119,7 @@
 // CHECK-NEXT:   %[[PVDTOR:.*]] = getelementptr inbounds i8* (%"struct.basic::C"*, i32)*, i8* (%"struct.basic::C"*, i32)** %[[VTABLE]], i64 0
 // CHECK-NEXT:   %[[VDTOR:.*]] = load i8* (%"struct.basic::C"*, i32)*, i8* (%"struct.basic::C"*, i32)** %[[PVDTOR]]
 // CHECK-NEXT:   %[[CALL:.*]] = call x86_thiscallcc i8* %[[VDTOR]](%"struct.basic::C"* {{[^,]*}} %[[OBJ_PTR_VALUE]], i32 0)
-// CHECK-NEXT:   call void @"??3@YAXPAX@Z"(i8* %[[CALL]])
+// CHECK-NEXT:   call void @"??3@YAXPAX@Z"(i8* allocptr %[[CALL]])
 // CHECK:  ret void
 }
 
Index: clang/test/CodeGenCXX/exceptions.cpp
===
--- clang/test/CodeGenCXX/exceptions.cpp
+++ clang/test/CodeGenCXX/exceptions.cpp
@@ -37,7 +37,7 @@
 // CHECK-NEXT: [[CAST:%.*]] = bitcast i8* [[NEW]] to [[A]]*
 // CHECK-NEXT: invoke void @_ZN5test11AC1Ei([[A]]* {{[^,]*}} [[CAST]], i32 5)
 // CHECK:  ret [[A]]* [[CAST]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 return new A(5);
   }
 
@@ -48,7 +48,7 @@
 // CHECK-NEXT: [[FOO:%.*]] = invoke i32 @_ZN5test13fooEv()
 // CHECK:  invoke void @_ZN5test11AC1Ei([[A]]* {{[^,]*}} [[CAST]], i32 [[FOO]])
 // CHECK:  ret [[A]]* [[CAST]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 extern int foo();
 return new A(foo());
   }
@@ -74,7 +74,7 @@
 // CHECK:  ret [[A]]* [[CAST]]
 // CHECK:  [[ISACTIVE:%.*]] = load i1, i1* [[ACTIVE]]
 // CHECK-NEXT: br i1 [[ISACTIVE]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 return new A(B().x);
   }
 
@@ -102,7 +102,7 @@
 // CHECK:  ret [[A]]* [[CAST]]
 // CHECK:  [[ISACTIVE:%.*]] = load i1, i1* [[ACTIVE]]
 // CHECK-NEXT: br i1 [[ISACTIVE]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 return new A(B());
   }
 
@@ -128,7 +128,7 @@
 // CHECK:  ret [[A]]* [[CAST]]
 // CHECK:  [[ISACTIVE:%.*]] = load i1, i1* [[ACTIVE]]
 // CHECK-NEXT: br i1 [[ISACTIVE]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 return new A(B(), B());
   }
   A *f() {
@@ -165,7 +165,7 @@
 // CHECK:  ret [[A]]* [[RET]]
 // CHECK:  [[ISACTIVE:%.*]] = load i1, i1* [[ACTIVE]]
 // CHECK-NEXT: br i1 [[ISACTIVE]]
-// CHECK:  call void @_ZdlPv(i8* [[NEW]])
+// CHECK:  call void @_ZdlPv(i8* allocptr [[NEW]])
 A *x;
 return (x = new A(makeB()), makeB(), x);
   }
@@ -458,7 +458,7 @@
   }
   // CHECK: define{{.*}} {{%.*}}* @_ZN5test94testEv
   // CHECK: [[TEST9_NEW:%.*]] = call noalias nonnull i8* @_Znam
-  // CHECK: call void @_ZdaPv(i8* [[TEST9_NEW]])
+  // CHECK: call void @_ZdaPv(i8* allocptr [[TEST9_NEW]])
 }
 
 // In a destructor with a function-try-block, a return statement in a
Index: clang/test/CodeGenCXX/delete.cpp
===
--- clang/test/CodeGenCXX/delete.cpp
+++ clang/test/CodeGenCXX/delete.cpp
@@ -100,7 +100,7 @@
 // CHECK-NEXT:

[PATCH] D123064: [Clang][C++23][WIP] P2071 Named universal character escapes

2022-04-04 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin updated this revision to Diff 420319.
cor3ntin added a comment.

Add Unit tests for nameToCodepoint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123064

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/unittests/Support/UnicodeTest.cpp
  llvm/utils/UnicodeData/CMakeLists.txt
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

STAMPS
actor(@cor3ntin) application(Differential) author(@cor3ntin) herald(H78) 
herald(H157) herald(H224) herald(H225) herald(H279) herald(H328) herald(H423) 
herald(H576) herald(H615) herald(H688) herald(H864) monogram(D123064) 
object-type(DREV) phid(PHID-DREV-4tkfplixy6vvmzuzyeyp) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) subscriber(@dexonsmith) 
subscriber(@dschuff) subscriber(@hiraditya) subscriber(@llvm-commits) 
subscriber(@mgorny) subscriber(@mgrang) tag(#all) tag(#clang) tag(#llvm) 
via(conduit)

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

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

2022-04-04 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513
+  const RecordDecl *RD = BaseTy->getDecl();
+  if (RD->getIdentifier() == nullptr || RD->getName() != "Message")
+return false;

Not sure how often is this invoked but we could reduce the number of string 
comparisons by caching the identifier ptr and do a pointer comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123032

STAMPS
actor(@xazax.hun) application(Differential) author(@ymandel) herald(H256) 
herald(H423) herald(H570) herald(H576) herald(H678) herald(H832) herald(H853) 
herald(H857) herald(H864) monogram(D123032) object-type(DREV) 
phid(PHID-DREV-mwcfw3jkwlzkhod5l25r) reviewer(@sgatev) reviewer(@xazax.hun) 
revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) 
subscriber(@gribozavr2) subscriber(@kinu) subscriber(@rnkovacs) 
subscriber(@steakhal) subscriber(@tschuett) tag(#all) tag(#clang) via(web)

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 420312.
void added a comment.

First pass at fixing the Windows unit test errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,453 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::randstruct;
+
+using field_names = std::vector;
+
+namespace {
+
+std::unique_ptr makeAST(const std::string ) {
+  std::vector Args = getCommandLineArgsForTesting(Lang_C99);
+  Args.push_back("-frandomize-layout-seed=1234567890abcdef");
+
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(SourceCode, Args);
+
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+RecordDecl *getRecordDeclFromAST(const ASTContext , const std::string ) {
+  RecordDecl *RD = FirstDeclMatcher().match(
+  C.getTranslationUnitDecl(), recordDecl(hasName(Name)));
+  return RD;
+}
+
+std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+
+  return Fields;
+}
+
+bool isSubsequence(const field_names , const field_names ) {
+  unsigned SeqLen = Seq.size();
+  unsigned SubLen = Subseq.size();
+
+  bool IsSubseq = false;
+  for (unsigned I = 0; I < SeqLen; ++I)
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (unsigned J = 0; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+
+  return IsSubseq;
+}
+
+} // end anonymous namespace
+
+namespace clang {
+namespace ast_matchers {
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const field_names Seq = {"a", "b", "c", "d"};
+
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a", "b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a"}));
+  ASSERT_FALSE(isSubsequence(Seq, {"a", "d"}));
+}
+
+#define RANDSTRUCT_TEST StructureLayoutRandomization
+
+TEST(RANDSTRUCT_TEST, UnmarkedStruct) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+int bacon;
+long lettuce;
+long long tomato;
+float mayonnaise;
+};
+  )c");
+
+  const RecordDecl *RD = getRecordDeclFromAST(AST->getASTContext(), "test");
+  const field_names Expected = {"bacon", "lettuce", "tomato", "mayonnaise"};
+
+  ASSERT_FALSE(RD->hasAttr());
+  ASSERT_FALSE(RD->isRandomized());
+  ASSERT_EQ(Expected, getFieldNamesFromRecord(RD));
+}
+
+TEST(RANDSTRUCT_TEST, MarkedNoRandomize) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+int bacon;
+long lettuce;
+

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Victoria Mitchell via Phabricator via cfe-commits

QuietMisdreavus added a comment.

After a quick scan comparing the current output of these symbol graphs with the 
primary library used for reading them 
, the last thing i can spot 
that's "off" is that the "function signature" is currently being serialized 
under a `parameters` field instead of the required `functionSignature`.




Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

zixuw wrote:
> What's the cost/would it worth it to wrap the `emplace_back`s and `pop_back`s 
> of `PathComponentContext` in meaningful APIs like `enterPathComponentContext` 
> and `exitPathComponentContext`?
> That way the code is more self-explanatory and easier to read. It took me 
> some time and mental resources to figure out why the `pop_back` is placed 
> here.
What's the use of having the `emplace_back` call inside `serializeAPIRecord` 
but to pop it outside? It seems like it's easier to mess up for new kinds of 
records.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 420307.
void added a comment.

- Add an entry in the Release Notes.
- Change the command line flags to better match the attribute naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,425 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::randstruct;
+
+using field_names = std::vector;
+
+namespace {
+
+std::unique_ptr makeAST(const std::string ) {
+  std::vector Args = getCommandLineArgsForTesting(Lang_C99);
+  Args.push_back("-frandomize-layout-seed=1234567890abcdef");
+
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(SourceCode, Args);
+
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+RecordDecl *getRecordDeclFromAST(const ASTContext , const std::string ) {
+  RecordDecl *RD = FirstDeclMatcher().match(
+  C.getTranslationUnitDecl(), recordDecl(hasName(Name)));
+  return RD;
+}
+
+std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+
+  return Fields;
+}
+
+bool isSubsequence(const field_names , const field_names ) {
+  unsigned SeqLen = Seq.size();
+  unsigned SubLen = Subseq.size();
+
+  bool IsSubseq = false;
+  for (unsigned I = 0; I < SeqLen; ++I)
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (unsigned J = 0; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+
+  return IsSubseq;
+}
+
+} // end anonymous namespace
+
+namespace clang {
+namespace ast_matchers {
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const field_names Seq = {"a", "b", "c", "d"};
+
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a", "b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a"}));
+  ASSERT_FALSE(isSubsequence(Seq, {"a", "d"}));
+}
+
+#define RANDSTRUCT_TEST StructureLayoutRandomization
+
+TEST(RANDSTRUCT_TEST, UnmarkedStruct) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+int bacon;
+long lettuce;
+long long tomato;
+float mayonnaise;
+};
+  )c");
+
+  const RecordDecl *RD = getRecordDeclFromAST(AST->getASTContext(), "test");
+  const field_names Expected = {"bacon", "lettuce", "tomato", "mayonnaise"};
+
+  ASSERT_FALSE(RD->hasAttr());
+  ASSERT_FALSE(RD->isRandomized());
+  ASSERT_EQ(Expected, getFieldNamesFromRecord(RD));
+}
+
+TEST(RANDSTRUCT_TEST, MarkedNoRandomize) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a comment.

In D122983#3426716 , @erichkeane 
wrote:

> We typically avoid doing -verify in CodeGen (unless the diagnostic is 
> ACTUALLY in CodeGen) as a matter of business.

I hope that `-verify` and `// expected-no-diagnostics` in CodeGen tests is 
compatible with the above. I believe it is valuable to confirm that the test 
itself is not written problematically.

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

https://reviews.llvm.org/D122983

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

[PATCH] D122954: [clang] Extend target_clones tests to exercise declarations that are not definitions.

2022-04-04 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added a comment.

> but I DO have the opposite problem: Figuring out what the associated tests 
> are for a patch

I also have that issue, but I don't see the relevance here. The changes in 
D122958  that fixes the issues revealed by 
these tests includes the test updates. So that commit reveals exactly which 
tests are relevant.

> It loses my ability to make sure that your patch covers all of the cases that 
> are important, and it loses my ability to figure out what the patch is doing 
> from the test itself.

I'm not following here. There are two distinct issues; a testing gap that is 
addressed by this review, and a code issue that is addressed by D122958 
. Addressing these separately is good 
separation of concerns. Should there be additional tests of declarations that 
are not definitions? If so, that would be appropriate to discuss in this review?

> this actually disables a LOT of other tests. So in the 'meantime' between the 
> two patches, we are actually losing test coverage thanks to the 'XFAIL'.

I do see `XFAIL` used for such temporary purposes based on a quick review of 
`git log`. See commit 9001168cf8b8c85ec9af9b91756b39d2da0130bf 
 for 
example. I've used this approach elsewhere for many years.

> I want the patch to be making one logical change so that it remains 
> manageable to review, but it needs to be completely self-contained 
> (functional changes, docs, tests, et al) even if that makes the patch a bit 
> bigger.

I strongly agree; that is exactly why these commits are split as they are. Both 
this review and D122958  are self-contained.

> This is especially important because it makes it far easier to revert 
> problematic commits -- if the functional change has an issue, having to 
> separately revert several other related commits is a burden.

I agree that would be a burden, but it isn't the case here.

> It also makes git blame more useful when doing code archeology; you can git 
> blame a test to get to the functional changes more quickly.

That is exactly what this separation enables. `git blame` on the tests will get 
you both the change for the testing gap as well as the functional change that 
fixes the behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122954

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

[PATCH] D123019: [clang][extract-api] Add support for typedefs

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:15
+#include "TypedefUnderlyingTypeResolver.h"
+
 #include "clang/ExtractAPI/DeclarationFragments.h"

Empty line



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:598-605
+  // Typedefs of anonymous types have their entries unified with the underlying
+  // type.
+  bool ShouldDrop = Record.UnderlyingType.Name.empty();
+  // enums declared with `NS_OPTION` have a named enum and a named typedef, 
with
+  // the same name
+  ShouldDrop |= (Record.UnderlyingType.Name == Record.Name);
+  if (ShouldDrop)

Consider move the should-drop logic into 
`SymbolGraphSerializer::shouldSkip(const APIRecord ) const` so that we 
have a central place to see and manage which symbols get skipped.
This would also simplify things here as the filtering will automatically get 
handled in the following `serializeAPIRecord(Record)` line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123019

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

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"

Not needed



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

What's the cost/would it worth it to wrap the `emplace_back`s and `pop_back`s 
of `PathComponentContext` in meaningful APIs like `enterPathComponentContext` 
and `exitPathComponentContext`?
That way the code is more self-explanatory and easier to read. It took me some 
time and mental resources to figure out why the `pop_back` is placed here.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:520-532
   for (const auto  : Record.Constants) {
 auto EnumConstant = serializeAPIRecord(*Constant);
+
+PathComponentContext.pop_back();
+
 if (!EnumConstant)
   continue;

Same comment as above, would be nice to have clear APIs for entering and 
exiting path component contexts.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:657
   Root["symbols"] = std::move(Symbols);
-  Root["relationhips"] = std::move(Relationships);
+  Root["relationships"] = std::move(Relationships);
 

oops  



Comment at: clang/test/ExtractAPI/macro_undefined.c:51
 {
+  "accessLevel": "public"m
   "declarationFragments": [

s/m/,/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D121556#3427620 , @void wrote:

> In D121556#3419184 , @aaron.ballman 
> wrote:
>
>> Generally I think things are looking pretty good, but there's still an open 
>> question and Precommit CI is still failing on Windows:
>>
>>   Failed Tests (7):
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.AnonymousStructsAndUnionsRetainFieldOrder
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.CheckAdjacentBitfieldsRemainAdjacentAfterRandomization
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.CheckVariableLengthArrayMemberRemainsAtEndOfStructure
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.MarkedRandomize
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.MultipleAttrsRandomize
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.RandstructDoesNotOverrideThePackedAttr
>> Clang-Unit :: 
>> AST/./ASTTests.exe/StructureLayoutRandomization.ZeroWidthBitfieldsSeparateAllocationUnits
>
> Do you have a URL for this failure or the output message?

https://reviews.llvm.org/D121556
->
Harbormaster completed remote builds in B156477 
: Diff 418480. 
(https://reviews.llvm.org/harbormaster/build/233718/)
->
x64 windows failed 
(https://buildkite.com/llvm-project/premerge-checks/builds/86170#c5c61c34-004b-4e62-a3b4-4654fb8d1dc7)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D121556#3427604 , @xbolva00 wrote:

> Release note is missing

Sorry about that. I'll add one with the next update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-04-04 Thread Thorsten via Phabricator via cfe-commits

tschuett added a comment.

https://reviews.llvm.org/D54604 used a really ugly flag:
`-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

2022-04-04 Thread Alex Brachet via Phabricator via cfe-commits

abrachet updated this revision to Diff 420299.
abrachet added a comment.

Fixing failing tests in Modules/


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

https://reviews.llvm.org/D122335

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/Driver/Compilation.h
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/crash-diagnostics-dir.c
  clang/test/Driver/crash-report-clang-cl.cpp
  clang/test/Driver/crash-report-header.h
  clang/test/Driver/crash-report-modules.m
  clang/test/Driver/crash-report-save-temps.c
  clang/test/Driver/crash-report-spaces.c
  clang/test/Driver/crash-report-with-asserts.c
  clang/test/Driver/crash-report.cpp
  clang/test/Driver/rewrite-map-in-diagnostics.c
  clang/test/Index/create-libclang-completion-reproducer.c
  clang/test/Index/create-libclang-parsing-reproducer.c
  clang/test/Modules/crash-vfs-path-emptydir-entries.m
  clang/test/Modules/crash-vfs-path-symlink-component.m
  clang/test/Modules/crash-vfs-path-symlink-topheader.m
  clang/test/Modules/crash-vfs-path-traversal.m
  clang/test/Modules/crash-vfs-relative-overlay.m
  clang/test/Modules/crash-vfs-umbrella-frameworks.m

Index: clang/test/Modules/crash-vfs-umbrella-frameworks.m
===
--- clang/test/Modules/crash-vfs-umbrella-frameworks.m
+++ clang/test/Modules/crash-vfs-umbrella-frameworks.m
@@ -9,19 +9,20 @@
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework
 
-// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: not %clang -nostdinc -fsyntax-only %s \
-// RUN: -F %/t/i/Frameworks -fmodules \
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= CLANG_CRASH_SAVE_TEMPS= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang -nostdinc -fsyntax-only %s   \
+// RUN: -F %/t/i/Frameworks -fmodules   \
 // RUN: -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s
 
-// RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \
-// RUN: %t/crash-vfs-*.cache/vfs/vfs.yaml
-// RUN: find %t/crash-vfs-*.cache/vfs | \
-// RUN:   grep "B.framework/Headers/B.h" | count 1
+// RUN: tar xOf %t/*.tar --wildcards "*/tmp/vfs/vfs.yaml" \
+// RUN:  | FileCheck --check-prefix=CHECKYAML %s
+// RUN: tar tf %t/*.tar | FileCheck --check-prefix=CHECKTAR %s
 
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
 // CHECK-NEXT: note: diagnostic msg: {{.*}}.m
 // CHECK-NEXT: note: diagnostic msg: {{.*}}.cache
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.sh
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.tar
 
 // CHECKYAML:  'type': 'directory',
 // CHECKYAML:  'name': "/[[PATH:.*]]/i/Frameworks/A.framework/Frameworks/B.framework/Headers",
@@ -39,6 +40,8 @@
 // CHECKYAML-NEXT:  'name': "B.h",
 // CHECKYAML-NEXT:  'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h"
 
+// CHECKTAR: B.framework/Headers/B.h
+
 @import I;
 
 // Run the reproducer script - regular exit code is enough to test it works. The
@@ -51,5 +54,6 @@
 // RUN: cd %t
 // RUN: rm -rf i
 // RUN: rm -rf crash-vfs-umbrella-*.cache/modules/*
-// RUN: chmod 755 crash-vfs-*.sh
-// RUN: ./crash-vfs-*.sh
+// RUN: tar xOf %t/*.tar --wildcards "*/repro.sh" > repro.sh
+// RUN: chmod 755 repro.sh
+// RUN: ./repro.sh
Index: clang/test/Modules/crash-vfs-relative-overlay.m
===
--- clang/test/Modules/crash-vfs-relative-overlay.m
+++ clang/test/Modules/crash-vfs-relative-overlay.m
@@ -6,23 +6,26 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
 
-// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: not %clang -fsyntax-only -nostdinc %s \
-// RUN: -I %S/Inputs/crash-recovery/usr/include -isysroot %/t/i/ \
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= CLANG_CRASH_SAVE_TEMPS= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang -fsyntax-only -nostdinc %s   \
+// RUN: -I %S/Inputs/crash-recovery/usr/include -isysroot %/t/i/\
 // RUN: -fmodules -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s
 
-// RUN: FileCheck --check-prefix=CHECKSRC %s -input-file %t/crash-vfs-*.m
-// RUN: FileCheck --check-prefix=CHECKSH %s -input-file %t/crash-vfs-*.sh
-// RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \
-// RUN: %t/crash-vfs-*.cache/vfs/vfs.yaml
-// RUN: find %t/crash-vfs-*.cache/vfs | \
-// RUN:   grep "Inputs/crash-recovery/usr/include/stdio.h" | count 1
+// RUN: tar xOf %t/*.tar --wildcards "*/tmp/crash-vfs-*.m"  \
+// RUN:  | FileCheck --check-prefix=CHECKSRC %s
+// RUN: tar xOf %t/*.tar --wildcards "*/repro.sh"   \
+// RUN:  | FileCheck --check-prefix=CHECKSH %s
+// RUN: tar xOf

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D121556#3419184 , @aaron.ballman 
wrote:

> Generally I think things are looking pretty good, but there's still an open 
> question and Precommit CI is still failing on Windows:
>
>   Failed Tests (7):
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.AnonymousStructsAndUnionsRetainFieldOrder
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.CheckAdjacentBitfieldsRemainAdjacentAfterRandomization
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.CheckVariableLengthArrayMemberRemainsAtEndOfStructure
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.MarkedRandomize
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.MultipleAttrsRandomize
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.RandstructDoesNotOverrideThePackedAttr
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.ZeroWidthBitfieldsSeparateAllocationUnits

Do you have a URL for this failure or the output message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

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

xbolva00 added a comment.

Release note is missing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[PATCH] D123074: [clang-tidy][run-clang-tidy.py] Fix typo in new -config-file option

2022-04-04 Thread Aaron Siddhartha Mondal via Phabricator via cfe-commits

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

The new `-config-file` option introduced by 9e1f4f1 
 was 
accidentally referenced
as `args.config_path` on the python side. This patch renames `args.config_path`
to `args.config_file`.

To avoid confusion with python `file` objects, the input argument for
`get_tidy_invocation` has been renamed from `config_path` to `config_file_path`.

See GitHub issue #54728  for 
a discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123074

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -90,7 +90,7 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config_path,
+extra_arg, extra_arg_before, quiet, config_file_path,
 config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
@@ -121,8 +121,8 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config_path:
-  start.append('--config-file=' + config_path)
+  if config_file_path:
+  start.append('--config-file=' + config_file_path)
   elif config:
   start.append('-config=' + config)
   start.append(f)
@@ -194,7 +194,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
@@ -304,7 +304,7 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -90,7 +90,7 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config_path,
+extra_arg, extra_arg_before, quiet, config_file_path,
 config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
@@ -121,8 +121,8 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config_path:
-  start.append('--config-file=' + config_path)
+  if config_file_path:
+  start.append('--config-file=' + config_file_path)
   elif config:
   start.append('-config=' + config)
   start.append(f)
@@ -194,7 +194,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -304,7 +304,7 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
-

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-04-04 Thread Peter Klausler via Phabricator via cfe-commits

klausler added a comment.

There's 128037 lines of code and documentation outside of lowering and drivers 
in flang/, and "git blame" tells me that I wrote 63% of them, including most of 
the Fortran semantics and nearly all of the Fortran parsing and runtime.  Based 
on my knowledge of the code base, the Fortran language, and where we stand in 
the process of shaking things out with applications and test suites, it is 
clear to me that things are not yet ready to be opened up to a larger user 
base.  We certainly don't need wider exposure yet to flush out more bugs.  
Premature exposure now would disrupt the critical path to overall success.  So 
I prefer option (3).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 420294.
void marked an inline comment as done.
void added a comment.

Simplify the attributes by using an accessor instead of a separate 
"no_ranomize_layout" attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,425 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::randstruct;
+
+using field_names = std::vector;
+
+namespace {
+
+std::unique_ptr makeAST(const std::string ) {
+  std::vector Args = getCommandLineArgsForTesting(Lang_C99);
+  Args.push_back("-frandstruct-seed=1234567890abcdef");
+
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(SourceCode, Args);
+
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+RecordDecl *getRecordDeclFromAST(const ASTContext , const std::string ) {
+  RecordDecl *RD = FirstDeclMatcher().match(
+  C.getTranslationUnitDecl(), recordDecl(hasName(Name)));
+  return RD;
+}
+
+std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+
+  return Fields;
+}
+
+bool isSubsequence(const field_names , const field_names ) {
+  unsigned SeqLen = Seq.size();
+  unsigned SubLen = Subseq.size();
+
+  bool IsSubseq = false;
+  for (unsigned I = 0; I < SeqLen; ++I)
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (unsigned J = 0; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+
+  return IsSubseq;
+}
+
+} // end anonymous namespace
+
+namespace clang {
+namespace ast_matchers {
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const field_names Seq = {"a", "b", "c", "d"};
+
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a", "b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a"}));
+  ASSERT_FALSE(isSubsequence(Seq, {"a", "d"}));
+}
+
+#define RANDSTRUCT_TEST StructureLayoutRandomization
+
+TEST(RANDSTRUCT_TEST, UnmarkedStruct) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+int bacon;
+long lettuce;
+long long tomato;
+float mayonnaise;
+};
+  )c");
+
+  const RecordDecl *RD = getRecordDeclFromAST(AST->getASTContext(), "test");
+  const field_names Expected = {"bacon", "lettuce", "tomato", "mayonnaise"};
+
+  ASSERT_FALSE(RD->hasAttr());
+  ASSERT_FALSE(RD->isRandomized());
+  ASSERT_EQ(Expected, getFieldNamesFromRecord(RD));
+}
+
+TEST(RANDSTRUCT_TEST, MarkedNoRandomize) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+

[PATCH] D123056: [clang][extract-api] Undefining macros should not result in a crash

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

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

Ha! Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123056

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

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

xbolva00 added a comment.

Could you please check that https://github.com/llvm/llvm-test-suite is 
buildable with your patch?


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

https://reviews.llvm.org/D122983

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

[PATCH] D122954: [clang] Extend target_clones tests to exercise declarations that are not definitions.

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D122954#3427422 , @erichkeane 
wrote:

> In D122954#3427406 , @tahonermann 
> wrote:
>
>>> FWIW, I dislike this idea of doing tests in separate commits from the patch 
>>> itself, it makes the review of the patch more difficult, and makes looking 
>>> through history more difficult.
>>
>> Here is my perspective on this. When adding a new feature, I agree that 
>> including tests with the code is useful (the tests usually have no 
>> meaningful behavior without the new code in that case). I think the 
>> motivation is different for bug fixes though, particularly when there is an 
>> existing testing gap. Had these tests been added with the initial 
>> `target_clones` work, then the bug fix commit would reveal the precise 
>> change in behavior that is the result of the bug fix. I think that is quite 
>> valuable both as a review tool and as part of the code history. By adding 
>> the test in a separate commit from the fix, it is possible to observe both 
>> the previous undesirable behavior as well as precisely what changed with the 
>> fix. This also helps to enable someone that is looking to identify a commit 
>> responsible for a bug fix (e.g, someone that is looking to resolve a bug 
>> report as works-for-me-in-release-X) to definitively identify a commit as 
>> exhibiting the expected fix.
>>
>> I've practiced this approach at other places I've worked and never found it 
>> to make the review process more difficult. If you can elaborate on why you 
>> feel it makes review more challenging, perhaps there is another way to 
>> address that.
>
> I understand it and just disagree. I've NEVER had a tough time 'seeing the 
> previous behavior' vs 'the current behavior', but I DO have the opposite 
> problem: Figuring out what the associated tests are for a patch.
>
> It loses my ability to make sure that your patch covers all of the cases that 
> are important, and it loses my ability to figure out what the patch is doing 
> from the test itself.

Strong +1 to this. As a reviewer, I want the patch to be making one logical 
change so that it remains manageable to review, but it needs to be completely 
self-contained (functional changes, docs, tests, et al) even if that makes the 
patch a bit bigger. This is especially important because it makes it far easier 
to revert problematic commits -- if the functional change has an issue, having 
to separately revert several other related commits is a burden. It also makes 
git blame more useful when doing code archeology; you can git blame a test to 
get to the functional changes more quickly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122954

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman updated this revision to Diff 420290.
aaron.ballman added a comment.

Fixed a failing test case caught by precommit CI.


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

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes

[PATCH] D123070: [Driver][NFC] Simplify handling of flags in Options.td

2022-04-04 Thread Emil Kieri via Phabricator via cfe-commits

ekieri added a comment.

This patch is the result of discussions in https://reviews.llvm.org/D122542

There are probably a developer or two on the clang side that ought to have a 
say on this. I do not know the community that well, could you please help me 
with adding the appropriate people?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123070

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

[PATCH] D123070: [Driver][NFC] Simplify handling of flags in Options.td

2022-04-04 Thread Emil Kieri via Phabricator via cfe-commits

ekieri created this revision.
ekieri added reviewers: awarzynski, thakis.
Herald added a reviewer: sscalpone.
Herald added a project: All.
ekieri requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

We aim at improving the readability and maintainability of Options.td,
and in particular its handling of 'Flags', by

- limiting the extent of 'let Flags = [...] in {'s, and
- adding closing comments to matching '}'s.

More concretely,

- we do not let a 'let Flags' span across several headline comments. When all 
'def's in two consecutive headlines share the same flags, we stil close and 
start a new 'let Flags' at the intermediate headline.
- when a 'let Flags' span just one or two 'def's, set 'Flags' within the 'def's 
instead.
- we remove nested 'let Flags'.

Note that nested 'let Flags' can be quite confusing, especially when
the outer was started long before the inner. Moving a 'def' out of the
inner 'let Flags' and setting 'Flags' within the 'def' will not have the
intended effect, as those flags will be overridden by the outer
'let Flags'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123070

Files:
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4690,15 +4690,16 @@
 //===--===//
 // FLangOption + CoreOption + NoXarchOption
 //===--===//
-let Flags = [FlangOption, FlangOnlyOption, NoXarchOption, CoreOption] in {
+
 def Xflang : Separate<["-"], "Xflang">,
   HelpText<"Pass  to the flang compiler">, MetaVarName<"">,
-  Flags<[NoXarchOption, CoreOption]>, Group;
-}
+  Flags<[FlangOption, FlangOnlyOption, NoXarchOption, CoreOption]>,
+  Group;
 
 //===--===//
 // FlangOption and FC1 Options
 //===--===//
+
 let Flags = [FC1Option, FlangOption, FlangOnlyOption] in {
 
 def cpp : Flag<["-"], "cpp">, Group,
@@ -4745,7 +4746,8 @@
 
 def fno_automatic : Flag<["-"], "fno-automatic">, Group,
   HelpText<"Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE">;
-}
+
+} // let Flags = [FC1Option, FlangOption, FlangOnlyOption]
 
 def J : JoinedOrSeparate<["-"], "J">,
   Flags<[RenderJoined, FlangOption, FC1Option, FlangOnlyOption]>,
@@ -4755,6 +4757,7 @@
 //===--===//
 // FC1 Options
 //===--===//
+
 let Flags = [FC1Option, FlangOnlyOption] in {
 
 def fget_definition : MultiArg<["-"], "fget-definition", 3>,
@@ -4809,13 +4812,7 @@
   HelpText<"Build the parse tree, then lower it to MLIR">;
 def emit_fir : Flag<["-"], "emit-fir">, Alias;
 
-}
-
-//===--===//
-// CC1 Options
-//===--===//
-
-let Flags = [CC1Option, NoDriverOption] in {
+} // let Flags = [FC1Option, FlangOnlyOption]
 
 //===--===//
 // Target Options (cc1 + cc1as)
@@ -4850,6 +4847,7 @@
 //===--===//
 // Target Options (cc1 + cc1as + fc1)
 //===--===//
+
 let Flags = [CC1Option, CC1AsOption, FC1Option, NoDriverOption] in {
 
 def triple : Separate<["-"], "triple">,
@@ -4863,6 +4861,8 @@
 // Target Options (other)
 //===--===//
 
+let Flags = [CC1Option, NoDriverOption] in {
+
 def target_linker_version : Separate<["-"], "target-linker-version">,
   HelpText<"Target linker version">,
   MarshallingInfoString>;
@@ -4877,10 +4877,14 @@
   NegFlag>,
   ShouldParseIf;
 
+} // let Flags = [CC1Option, NoDriverOption]
+
 //===--===//
 // Analyzer Options
 //===--===//
 
+let Flags = [CC1Option, NoDriverOption] in {
+
 def analysis_UnoptimizedCFG : Flag<["-"], "unoptimized-cfg">,
   HelpText<"Generate unoptimized CFGs for all analyses">,
   MarshallingInfoFlag>;
@@ -5031,15 +5035,20 @@
   HelpText<"Emit analyzer results as errors rather than warnings">,
   MarshallingInfoFlag>;
 
+} // let Flags = [CC1Option, NoDriverOption]
+
 //===--===//
 // Migrator Options

[PATCH] D123064: [Clang][C++23][WIP] P2071 Named universal character escapes

2022-04-04 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin updated this revision to Diff 420286.
cor3ntin added a comment.

Fix generator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123064

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/CMakeLists.txt
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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

[PATCH] D122952: [clang] NFC: Extend comdat validation in target multiversion function tests.

2022-04-04 Thread Erich Keane via Phabricator via cfe-commits

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

I'm ok with treating the COMDAT issue on non-resolvers as a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122952

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

[PATCH] D122954: [clang] Extend target_clones tests to exercise declarations that are not definitions.

2022-04-04 Thread Erich Keane via Phabricator via cfe-commits

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

In D122954#3427406 , @tahonermann 
wrote:

>> FWIW, I dislike this idea of doing tests in separate commits from the patch 
>> itself, it makes the review of the patch more difficult, and makes looking 
>> through history more difficult.
>
> Here is my perspective on this. When adding a new feature, I agree that 
> including tests with the code is useful (the tests usually have no meaningful 
> behavior without the new code in that case). I think the motivation is 
> different for bug fixes though, particularly when there is an existing 
> testing gap. Had these tests been added with the initial `target_clones` 
> work, then the bug fix commit would reveal the precise change in behavior 
> that is the result of the bug fix. I think that is quite valuable both as a 
> review tool and as part of the code history. By adding the test in a separate 
> commit from the fix, it is possible to observe both the previous undesirable 
> behavior as well as precisely what changed with the fix. This also helps to 
> enable someone that is looking to identify a commit responsible for a bug fix 
> (e.g, someone that is looking to resolve a bug report as 
> works-for-me-in-release-X) to definitively identify a commit as exhibiting 
> the expected fix.
>
> I've practiced this approach at other places I've worked and never found it 
> to make the review process more difficult. If you can elaborate on why you 
> feel it makes review more challenging, perhaps there is another way to 
> address that.

I understand it and just disagree. I've NEVER had a tough time 'seeing the 
previous behavior' vs 'the current behavior', but I DO have the opposite 
problem: Figuring out what the associated tests are for a patch.

It loses my ability to make sure that your patch covers all of the cases that 
are important, and it loses my ability to figure out what the patch is doing 
from the test itself.

Additionally, and I just noticed, this actually disables a LOT of other tests.  
So in the 'meantime' between the two patches, we are actually losing test 
coverage thanks to the 'XFAIL'.  I think this is unacceptable enough for me to 
'request changes'.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122954

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

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-04-04 Thread Steve Scalpone via Phabricator via cfe-commits

sscalpone added a comment.

As I said this morning, I prefer to wait with this patch until the upstreaming 
is finished.  The reason is not only to do with upstreaming, but also with a 
concurrent effort to button up any assertion and runtime errors so the initial 
user experience is better.   While some posit the the news of your patch won't 
be heard around the world, I think it is a major accomplishment & the user 
community will be thrilled to try flang.  My preference is for that first wave 
of users to come a way with the impression that flang is a high-quality work in 
progress.

@awarzynski The CMAKE PR is closed.  Do you plan to revitalize it?Will you 
check that this patch, along with the CMAKE PR, is indeed enough to satisfy the 
requirement for the cmake reviewers as well as provides enough functionality to 
support cmake in a real project?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D122954: [clang] Extend target_clones tests to exercise declarations that are not definitions.

2022-04-04 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added a comment.

> FWIW, I dislike this idea of doing tests in separate commits from the patch 
> itself, it makes the review of the patch more difficult, and makes looking 
> through history more difficult.

Here is my perspective on this. When adding a new feature, I agree that 
including tests with the code is useful (the tests usually have no meaningful 
behavior without the new code in that case). I think the motivation is 
different for bug fixes though, particularly when there is an existing testing 
gap. Had these tests been added with the initial `target_clones` work, then the 
bug fix commit would reveal the precise change in behavior that is the result 
of the bug fix. I think that is quite valuable both as a review tool and as 
part of the code history. By adding the test in a separate commit from the fix, 
it is possible to observe both the previous undesirable behavior as well as 
precisely what changed with the fix. This also helps to enable someone that is 
looking to identify a commit responsible for a bug fix (e.g, someone that is 
looking to resolve a bug report as works-for-me-in-release-X) to definitively 
identify a commit as exhibiting the expected fix.

I've practiced this approach at other places I've worked and never found it to 
make the review process more difficult. If you can elaborate on why you feel it 
makes review more challenging, perhaps there is another way to address that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122954

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

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Mitch Phillips via Phabricator via cfe-commits

hctim marked an inline comment as done.
hctim added a comment.

In D118948#3427344 , @tschuett wrote:

> Is `-fsanitize=memtag-heap` Android specific or target independent? It passes 
> Android flags to the linker?!?

The frontend flag should be target-independent. The clang driver knows to only 
pass Android flags to the linker where `TC.getTriple().isAndroid()` is true. 
This should allow a glibc-based implementation to live under the same 
`-fsanitize=memtag-heap` flag, just chainging the implementation based on the 
target.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

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

[PATCH] D123065: [clang-tidy] support --load in clang-tidy-diff.py/run-clang-tidy.py

2022-04-04 Thread Bernhard Manfred Gruber via Phabricator via cfe-commits

bernhardmgruber added a comment.

I did not base this revision on my latest main 
(422d05e792dbd6a97f5afd4cdd5e8aa677e97444 
) since I 
could not run the python scripts with the changes introduced in 
9e1f4f13984186984ba37513372c1b7e0c4ba4fd 
.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D123065

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

[PATCH] D123065: [clang-tidy] support --load in clang-tidy-diff.py/run-clang-tidy.py

2022-04-04 Thread Bernhard Manfred Gruber via Phabricator via cfe-commits

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

Support for loading shared objects as plugins into clang-tidy was added in 
https://reviews.llvm.org/D00. Unfortunately, the utility scripts 
`clang-tidy-diff.py` and `run-clang-tidy.py` did not receive corresponding 
arguments to forward such plugins to clang-tidy. This diff adds a 
`-load=plugin` option to both scripts.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D123065

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -91,7 +91,7 @@
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
 extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+line_filter, use_color, plugins):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -123,6 +123,8 @@
   start.append('-quiet')
   if config:
   start.append('-config=' + config)
+  for plugin in plugins:
+  start.append('-load=' + plugin)
   start.append(f)
   return start
 
@@ -193,7 +195,7 @@
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.use_color, args.plugins)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -270,6 +272,9 @@
   'command line.')
   parser.add_argument('-quiet', action='store_true',
   help='Run clang-tidy in quiet mode')
+  parser.add_argument('-load', dest='plugins',
+  action='append', default=[],
+  help='Make clang-tidy load the specified plugin')
   args = parser.parse_args()
 
   db_path = 'compile_commands.json'
@@ -296,7 +301,7 @@
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.use_color, args.plugins)
 invocation.append('-list-checks')
 invocation.append('-')
 if args.quiet:
Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -160,6 +160,10 @@
   'command line.')
   parser.add_argument('-quiet', action='store_true', default=False,
   help='Run clang-tidy in quiet mode')
+  parser.add_argument('-load', dest='plugins',
+  action='append', default=[],
+  help='Make clang-tidy load the specified plugin')
+
   clang_tidy_args = []
   argv = sys.argv[1:]
   if '--' in argv:
@@ -233,6 +237,8 @@
 common_clang_tidy_args.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
 common_clang_tidy_args.append('-extra-arg-before=%s' % arg)
+  for plugin in args.plugins:
+common_clang_tidy_args.append('-load=%s' % plugin)
 
   for name in lines_by_file:
 line_filter_json = json.dumps(


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -91,7 +91,7 @@
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
 extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+line_filter, use_color, plugins):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -123,6 +123,8 @@
   start.append('-quiet')
   if config:
   start.append('-config=' + config)
+  for plugin in plugins:
+  start.append('-load=' + plugin)

[PATCH] D123064: [Clang][C++23][WIP] P2071 Named universal character escapes

2022-04-04 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin updated this revision to Diff 420278.
cor3ntin added a comment.
Herald added a subscriber: mgrang.

Committed the wrong file!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123064

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/CMakeLists.txt
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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

[PATCH] D123064: [Clang][C++23][WIP] P2071 Named universal character escapes

2022-04-04 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin created this revision.
Herald added subscribers: dexonsmith, hiraditya, mgorny, dschuff.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

! Missing tests, some cleanup still needed.

- Add a function in LLVM to map a name to a codepoint.

This using a try to minimize memory usage,
while allowing fast access.

- Add an utility to regenerate this data.

- Support named escape sequences with an extension warning.

I have not yet dealt with C++23 conformance extension warning,


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123064

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/CMakeLists.txt
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Thorsten via Phabricator via cfe-commits

tschuett added a comment.

Is `-fsanitize=memtag-heap` Android specific or target independent? It passes 
Android flags to the linker?!?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

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

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Mitch Phillips via Phabricator via cfe-commits

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



Comment at: clang/test/Driver/memtag-ld.c:36
+
+// CHECK-ASYNC: ld{{.*}} "-memtag-mode=async"
+// CHECK-DEFAULT-MODE-NOT: ld{{.*}} "--android-memtag-mode=

MaskRay wrote:
> This will be easier to read if you make the strings after `:` aligned.
> 
> Is `-memtag-mode=async` `--android-memtag-mode=async` now?
thanks for the catch



Comment at: clang/test/Driver/memtag-ld.c:38
+// CHECK-DEFAULT-MODE-NOT: ld{{.*}} "--android-memtag-mode=
+// CHECK-HEAP: "--android-memtag-heap"
+// CHECK-NO-HEAP-NOT: "--android-memtag-heap"

MaskRay wrote:
> Prefer `CHECK-HEAP-SAME` to `CHECK-HEAP` whenever feasible.
There's no other CHECK-HEAP before this, so `-SAME` not possible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

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

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Mitch Phillips via Phabricator via cfe-commits

hctim updated this revision to Diff 420274.
hctim marked 2 inline comments as done.
hctim added a comment.

(comments)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/memtag-attr.cpp
  clang/test/Driver/fsanitize.c
  clang/test/Driver/memtag-ld.c
  clang/test/Driver/memtag-stack.c
  clang/test/Driver/memtag.c
  clang/test/Lexer/has_feature_memtag.cpp
  clang/test/Lexer/has_feature_memtag_sanitizer.cpp

Index: clang/test/Lexer/has_feature_memtag_sanitizer.cpp
===
--- clang/test/Lexer/has_feature_memtag_sanitizer.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// RUN: %clang_cc1 -E -fsanitize=memtag %s -o - | FileCheck --check-prefix=CHECK-MEMTAG %s
-// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-MEMTAG %s
-
-#if __has_feature(memtag_sanitizer)
-int MemTagSanitizerEnabled();
-#else
-int MemTagSanitizerDisabled();
-#endif
-
-// CHECK-MEMTAG: MemTagSanitizerEnabled
-// CHECK-NO-MEMTAG: MemTagSanitizerDisabled
Index: clang/test/Lexer/has_feature_memtag.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_memtag.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -E -fsanitize=memtag-stack %s -o - | FileCheck --check-prefix=CHECK-MEMTAG-STACK %s
+// RUN: %clang_cc1 -E -fsanitize=memtag-heap %s -o -  | FileCheck --check-prefix=CHECK-MEMTAG-HEAP %s
+// RUN: %clang -E -fsanitize=memtag --target=aarch64-unknown-linux -march=armv8a+memtag %s -o - \
+// RUN: | FileCheck --check-prefixes=CHECK-MEMTAG-STACK,CHECK-MEMTAG-HEAP %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-MEMTAG %s
+
+#if __has_feature(memtag_stack)
+int MemTagSanitizerStack();
+#else
+int MemTagSanitizerNoStack();
+#endif
+
+#if __has_feature(memtag_heap)
+int MemTagSanitizerHeap();
+#else
+int MemTagSanitizerNoHeap();
+#endif
+
+// CHECK-MEMTAG-STACK: MemTagSanitizerStack
+// CHECK-MEMTAG-HEAP: MemTagSanitizerHeap
+
+// CHECK-NO-MEMTAG: MemTagSanitizerNoStack
+// CHECK-NO-MEMTAG: MemTagSanitizerNoHeap
Index: clang/test/Driver/memtag.c
===
--- /dev/null
+++ clang/test/Driver/memtag.c
@@ -1,20 +0,0 @@
-// RUN: %clang -target aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm -stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SAFETY
-// RUN: %clang -O1 -target aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm -stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
-// RUN: %clang -O2 -target aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm -stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
-// RUN: %clang -O3 -target aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm -stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
-
-// REQUIRES: aarch64-registered-target
-
-int z;
-__attribute__((noinline)) void use(int *p) { *p = z; }
-int foo() {
-  int x;
-  use();
-  return x;
-}
-
-// CHECK-NO-SAFETY-NOT: allocas uses
-
-// CHECK-SAFETY-LABEL: @foo
-// CHECK-SAFETY-LABEL: allocas uses:
-// CHECK-SAFETY-NEXT: [4]: [0,4)
Index: clang/test/Driver/memtag-ld.c
===
--- /dev/null
+++ clang/test/Driver/memtag-ld.c
@@ -0,0 +1,46 @@
+// RUN: %clang -### --target=aarch64-linux-android -march=armv8+memtag \
+// RUN:   -fsanitize=memtag %s 2>&1 | FileCheck %s \
+// RUN:   --check-prefixes=CHECK-SYNC,CHECK-HEAP,CHECK-STACK
+
+// RUN: %clang -### --target=aarch64-linux-android -march=armv8+memtag \
+// RUN:   -fsanitize=memtag-stack %s 2>&1 | FileCheck %s \
+// RUN:   --check-prefixes=CHECK-SYNC,CHECK-NO-HEAP,CHECK-STACK
+
+// RUN: %clang -### --target=aarch64-linux-android -march=armv8+memtag \
+// RUN:   -fsanitize=memtag-heap %s 2>&1 | FileCheck %s \
+// RUN:   --check-prefixes=CHECK-SYNC,CHECK-HEAP,CHECK-NO-STACK
+
+// RUN: %clang -### --target=aarch64-linux-android -march=armv8+memtag \
+// RUN:   -fsanitize=memtag -fsanitize-memtag-mode=async %s 2>&1 | FileCheck %s \
+// RUN:   --check-prefixes=CHECK-ASYNC,CHECK-HEAP,CHECK-STACK
+
+// RUN: %clang -### --target=aarch64-linux-android -march=armv8+memtag \
+// RUN:   -fsanitize=memtag-stack -fsanitize-memtag-mode=async %s 2>&1 \
+// RUN:   | FileCheck %s \
+// RUN:   --check-prefixes=CHECK-ASYNC,CHECK-NO-HEAP,CHECK-STACK
+
+// RUN: %clang

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-04-04 Thread Valentin Clement via Phabricator via cfe-commits

clementval added a comment.

In D122008#3426847 , @awarzynski 
wrote:

> We discussed this patch in our community call today and some people expressed 
> their reservations about merging this just now. As I mentioned, I would like 
> to have this merged to unblock the CMake PR 
> . As promised, 
> here are the options that we've considered:
>
> 1. merge this patch "today",
> 2. merge this patch "today", but add a flag so that by default `flang-new 
> file.f90` wouldn't generate an executable (we would remove this flag at a 
> later time),
> 3. wait until the upstreaming of fir-dev is complete (based on today's 
> update, I think that that would be in June/July the earliest).
>
> Could you tell me what your preference is?
>
> Please keep in mind that even once CMake support is available, it's going to 
> land in the "latest" version of CMake. Folks will have to download the latest 
> binaries from https://github.com/Kitware/CMake/releases in order to benefit 
> from this.  With time, CMake shipped with various OSes will catch-up and 
> become "modern" enough. But there's going to be a delay and hence it makes 
> sense to get the CMake support sooner rather than later.
>
> Please comment if I missed or misinterpreted anything. Also, I will add more 
> reviewers - basically everyone who discussed this in the call today. Please 
> add anyone that I've missed!
>
> Thanks for taking a look!

I'll personally in favor of number 3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

2022-04-04 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D121556#3419184 , @aaron.ballman 
wrote:

> Generally I think things are looking pretty good, but there's still an open 
> question and Precommit CI is still failing on Windows:
>
>   Failed Tests (7):
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.AnonymousStructsAndUnionsRetainFieldOrder
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.CheckAdjacentBitfieldsRemainAdjacentAfterRandomization
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.CheckVariableLengthArrayMemberRemainsAtEndOfStructure
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.MarkedRandomize
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.MultipleAttrsRandomize
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.RandstructDoesNotOverrideThePackedAttr
> Clang-Unit :: 
> AST/./ASTTests.exe/StructureLayoutRandomization.ZeroWidthBitfieldsSeparateAllocationUnits

Strange. I wonder if Windows is using a different algorithm for the 
randomization function. I'll investigate and see what can be done here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

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

[PATCH] D123059: re-roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".""

2022-04-04 Thread Devin Jeanpierre via Phabricator via cfe-commits

devin.jeanpierre created this revision.
devin.jeanpierre added a reviewer: gribozavr2.
Herald added subscribers: dexonsmith, pengfei.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
devin.jeanpierre requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit b0bc93da926a943cdc2d8b04f8dcbe23a774520c 
.

Changes: `s/_WIN32/_WIN64/g`.

The calling convention kind is specific to 64-bit windows.
It's even in the name: `CCK_MicrosoftWin64`.

After this, the test passes with both `-triple i686-pc-win32` and `-triple 
x86_64-pc-win32`. Phew!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123059

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};

[clang-tools-extra] 6f3f1e9 - [clangd] Remove trivial uses of FileEntry::getName

2022-04-04 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-04-04T20:59:51+02:00
New Revision: 6f3f1e98686575004bef4257cbc6c825de5382af

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

LOG: [clangd] Remove trivial uses of FileEntry::getName

It's deprecated; migrate to FileEntryRef::getName where it doesn't matter.
Also change one subtle case of implicit FileEntry::getName to be explicit.

After this patch, all the remaining FileEntry::getName calls are subtle
cases where we may be relying on exactly which filename variant is returned
(for indexing, IWYU directive handling, etc).

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/IndexActionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 762906615f323..46a16a9f2eeb3 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -196,16 +196,16 @@ bool tryMoveToMainFile(Diag , FullSourceLoc DiagLoc) {
 return false;
 
   // Add a note that will point to real diagnostic.
-  const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
+  auto FE = SM.getFileEntryRefForID(SM.getFileID(DiagLoc)).getValue();
   D.Notes.emplace(D.Notes.begin());
   Note  = D.Notes.front();
-  N.AbsFile = std::string(FE->tryGetRealPathName());
-  N.File = std::string(FE->getName());
+  N.AbsFile = std::string(FE.getFileEntry().tryGetRealPathName());
+  N.File = std::string(FE.getName());
   N.Message = "error occurred here";
   N.Range = D.Range;
 
   // Update diag to point at include inside main file.
-  D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+  D.File = SM.getFileEntryRefForID(SM.getMainFileID())->getName().str();
   D.Range = std::move(R);
   D.InsideMainFile = true;
   // Update message to mention original file.

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 04dbf12410cf7..44acd34086af9 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -246,14 +246,14 @@ static bool mayConsiderUnused(const Inclusion , 
ParsedAST ) {
   // Headers without include guards have side effects and are not
   // self-contained, skip them.
   assert(Inc.HeaderID);
-  auto FE = AST.getSourceManager().getFileManager().getFile(
+  auto FE = AST.getSourceManager().getFileManager().getFileRef(
   AST.getIncludeStructure().getRealPath(
   static_cast(*Inc.HeaderID)));
   assert(FE);
   if 
(!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
-  *FE)) {
+  >getFileEntry())) {
 dlog("{0} doesn't have header guard and will not be considered unused",
- (*FE)->getName());
+ FE->getName());
 return false;
   }
   return true;
@@ -418,7 +418,7 @@ std::vector issueUnusedIncludesDiagnostics(ParsedAST 
,
   std::vector Result;
   std::string FileName =
   AST.getSourceManager()
-  .getFileEntryForID(AST.getSourceManager().getMainFileID())
+  .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
   ->getName()
   .str();
   for (const auto *Inc : computeUnusedIncludes(AST)) {

diff  --git a/clang-tools-extra/clangd/Preamble.cpp 
b/clang-tools-extra/clangd/Preamble.cpp
index aab6dc78092f7..b3a3aec985ba0 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -677,7 +677,7 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData 
) {
 SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
   const SourceManager ) {
   auto DefFile = SM.getFileID(Loc);
-  if (auto *FE = SM.getFileEntryForID(DefFile)) {
+  if (auto FE = SM.getFileEntryRefForID(DefFile)) {
 auto IncludeLoc = SM.getIncludeLoc(DefFile);
 // Preamble patch is included inside the builtin file.
 if (IncludeLoc.isValid() && SM.isWrittenInBuiltinFile(IncludeLoc) &&

diff  --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp 
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 7067f1771b94f..ce02f97bb7d8c 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -66,11 +66,12 @@ collectIWYUHeaderMaps(CanonicalIncludes *Includes) {

[PATCH] D122808: [clang] Fix warnings when `-Wdeprecated-enum-enum-conversion` is enabled

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D122808#3425703 , 
@antoniofrighetto wrote:

> Looks definitely better! How about this slightly changed version protecting 
> the interface?
>
>   /// Helper which adds two underlying types of enumeration type.
>   template  typename EnumTy2,
> typename UT1 = std::enable_if_t, 
> std::underlying_type_t>,
> typename UT2 = std::enable_if_t, 
> std::underlying_type_t>>
>   constexpr auto addEnumValues(EnumTy1 LHS, EnumTy2 RHS) {
> return static_cast(LHS) + static_cast(RHS);
>   }
>
> I feel like this is something we may wish to readopt in the future for other 
> similar cases as well  (e.g., `-Wdeprecated-anon-enum-enum-conversion` in 
> `Comment.h`).

I think that's a great interface for it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122808

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

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM, thanks!


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

https://reviews.llvm.org/D122155

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

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D119609#3426279 , @erichkeane 
wrote:

> I think LGTM on technical perspective, Please don't commit until 
> @aaron.ballman confirms he is OK with the direction, or would like to wait 
> longer for GCC.

I spotted some technical issues with the diagnostic wording. As for waiting for 
GCC, my concern there is largely with the fact that people use statement 
expressions in macros so that they can define local variables that don't impact 
the outer scope where the macro is expanded. I could see such a macro being 
used as a default argument, so I worry a bit about breaking code in that case. 
However, C doesn't have default arguments, so this isn't an issue there. And 
C++ has lambdas which can be called in a default argument, so users have some 
ability to fix their code in the event of an error so long as they're in C++11 
or later. Because of that, I would be okay moving forward with this, but 
cautiously in case someone reports code that breaks and can't easily be fixed 
(or significant breakage in an important 3rd party header).

Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4383-4384
   "%select{default|copy|move}0 constructor">;
+def err_expr_statement_in_default_arg : Error<
+  "expression statement not permitted in default argument">;

I get mixed up every time, but I double-checked -- these are statement 
expressions, not expression statements. I also think the diagnostic should be 
more clear about default argument vs default non-type template argument.

Comment at: clang/lib/Parse/ParseDecl.cpp:7073
+  Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+  // Skip the expression statement and continue parsing
+  SkipUntil(tok::comma, StopBeforeMatch);

Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:1
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+

You should rename this test to `stmt-expr-in-default-arg.cpp`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

2022-04-04 Thread Wang Yihan via Phabricator via cfe-commits

yihanaa updated this revision to Diff 420254.
yihanaa added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -5,95 +5,122 @@
 
 // CHECK: @__const.unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"short a = %hd\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"short a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [28 x i8] c"unsigned short a = %hu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned short a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int a = %d\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [13 x i8] c"int a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [25 x i8] c"unsigned int a = %u\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [22 x i8] c"unsigned int a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"long a = %ld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [27 x i8] c"unsigned long a = %lu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [23 x i8] c"unsigned long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"long long a = %lld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"long long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

2022-04-04 Thread Daniel Grumberg 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 rG422d05e792db: [clang][extract-api][NFC] Add documentation 
(authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

Files:
  clang/include/clang/ExtractAPI/API.h


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 422d05e - [clang][extract-api][NFC] Add documentation

2022-04-04 Thread Daniel Grumberg via cfe-commits


Author: Daniel Grumberg
Date: 2022-04-04T18:59:44+01:00
New Revision: 422d05e792dbd6a97f5afd4cdd5e8aa677e97444

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

LOG: [clang][extract-api][NFC] Add documentation

Add struct level documentation for MacroDefinitionRecord.

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

Added: 


Modified: 
clang/include/clang/ExtractAPI/API.h

Removed: 




diff  --git a/clang/include/clang/ExtractAPI/API.h 
b/clang/include/clang/ExtractAPI/API.h
index 142dd7a45de47..62e30b1d4c57b 100644
--- a/clang/include/clang/ExtractAPI/API.h
+++ b/clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@ struct ObjCProtocolRecord : ObjCContainerRecord {
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,



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

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

2022-04-04 Thread Daniel Grumberg via Phabricator via cfe-commits

dang added a comment.

In D122798#3427004 , @zixuw wrote:

> Has this landed yet?

Doing it now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman updated this revision to Diff 420242.
aaron.ballman added a comment.
Herald added subscribers: kbarton, nemanjai.

Updated the release note and hopefully addressed the remaining test failures 
found by precommit CI.


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

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 |

[PATCH] D123056: [clang][extract-api] Undefining macros should not result in a crash

2022-04-04 Thread Daniel Grumberg via Phabricator via cfe-commits

dang created this revision.
dang added reviewers: zixuw, ributzka, QuietMisdreavus.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes the situation where a undefining a not previously defined
macro resulted in a crash. Before trying to remove a definition from
PendingMacros we first check to see if the macro did indeed have a
previous definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123056

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/macro_undefined.c


Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -19,6 +19,8 @@
 FUNC_GEN(foo)
 FUNC_GEN(bar, const int *, unsigned);
 #undef FUNC_GEN
+// Undefining a not previously defined macro should not result in a crash.
+#undef FOO
 
 //--- reference.output.json.in
 {
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -534,6 +534,11 @@
   // macro definition for it.
   void MacroUndefined(const Token , const MacroDefinition ,
   const MacroDirective *Undef) override {
+// If this macro wasn't previously defined we don't need to do anything
+// here.
+if (!Undef)
+  return;
+
 llvm::erase_if(PendingMacros, [](const PendingMacro ) {
   return MD.getMacroInfo()->getDefinitionLoc() ==
  PM.MD->getMacroInfo()->getDefinitionLoc();


Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -19,6 +19,8 @@
 FUNC_GEN(foo)
 FUNC_GEN(bar, const int *, unsigned);
 #undef FUNC_GEN
+// Undefining a not previously defined macro should not result in a crash.
+#undef FOO
 
 //--- reference.output.json.in
 {
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -534,6 +534,11 @@
   // macro definition for it.
   void MacroUndefined(const Token , const MacroDefinition ,
   const MacroDirective *Undef) override {
+// If this macro wasn't previously defined we don't need to do anything
+// here.
+if (!Undef)
+  return;
+
 llvm::erase_if(PendingMacros, [](const PendingMacro ) {
   return MD.getMacroInfo()->getDefinitionLoc() ==
  PM.MD->getMacroInfo()->getDefinitionLoc();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits

zixuw added a comment.

Has this landed yet?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

2022-04-04 Thread Wang Yihan via Phabricator via cfe-commits

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > Not a fan of bool parameters, they make calls harder to read.
> > > I can add comments for this parameter or this function, what do you 
> > > think, or do you have a better idea? I would like to improve it.
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  
> > It seems that what you ACTUALLY care about here is whether this is a 'top 
> > level', and thus has a FieldName to print, right? So you actually want to 
> > dumpFieldName?  Or am I still misunderstanding something?
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  
> > It seems that what you ACTUALLY care about here is whether this is a 'top 
> > level', and thus has a FieldName to print, right? So you actually want to 
> > dumpFieldName?  Or am I still misunderstanding something?
> 
> Yeah, you are right. A little  different is that this patch will DO NOT dump 
> the typename, field name and '=' in recursive dumpRecord call. 
Yeah, you are right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

I pushed 5d90004874c7b040cd43597826aabb34757d25ab 
 to 
hopefully address the lion's share of the currently failing precommit CI tests. 
I'll address the remaining few as an update to this patch.


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

https://reviews.llvm.org/D122983

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

2022-04-04 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

LGTM since currently there is only one HIP/SPIRV implementation. If in the 
future there is another HIP/SPIRV implementation that does not need this, it 
could disable it by triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123049

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

2022-04-04 Thread Shangwu Yao via Phabricator via cfe-commits

shangwuyao added a comment.

In D123049#3426849 , @yaxunl wrote:

> Is this because your HIP threadIdx etc are implemented using OpenCL builtins 
> so that the emitted LLVM IR contains calls of OpenCL builtins?

Yes, that's correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123049

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

1 2 >

1 - 100 of 186 matches

Mail list logo