[PATCH] D119722: [clang][lex] Use `SearchDirIterator` types in for loops

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 408724.
jansvoboda11 added a comment.

Remove `Optional<...>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119722

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -79,11 +79,6 @@
 
 ExternalHeaderFileInfoSource::~ExternalHeaderFileInfoSource() = default;
 
-const DirectoryLookup ::operator*() const {
-  assert(*this && "Invalid iterator.");
-  return HS->SearchDirs[Idx];
-}
-
 HeaderSearch::HeaderSearch(std::shared_ptr HSOpts,
SourceManager , DiagnosticsEngine ,
const LangOptions ,
@@ -304,21 +299,20 @@
SourceLocation ImportLoc,
bool AllowExtraModuleMapSearch) {
   Module *Module = nullptr;
-  unsigned Idx;
+  SearchDirIterator It = nullptr;
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (Idx = 0; Idx != SearchDirs.size(); ++Idx) {
-if (SearchDirs[Idx].isFramework()) {
+  for (It = search_dir_begin(); It != search_dir_end(); ++It) {
+if (It->isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules
   // named FooPrivate in buggy frameworks named Foo.
   SmallString<128> FrameworkDirName;
-  FrameworkDirName += SearchDirs[Idx].getFrameworkDir()->getName();
+  FrameworkDirName += It->getFrameworkDir()->getName();
   llvm::sys::path::append(FrameworkDirName, SearchName + ".framework");
   if (auto FrameworkDir = FileMgr.getDirectory(FrameworkDirName)) {
-bool IsSystem
-  = SearchDirs[Idx].getDirCharacteristic() != SrcMgr::C_User;
+bool IsSystem = It->getDirCharacteristic() != SrcMgr::C_User;
 Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem);
 if (Module)
   break;
@@ -328,12 +322,12 @@
 // FIXME: Figure out how header maps and module maps will work together.
 
 // Only deal with normal search directories.
-if (!SearchDirs[Idx].isNormalDir())
+if (!It->isNormalDir())
   continue;
 
-bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory();
+bool IsSystem = It->isSystemHeaderDirectory();
 // Search for a module map file in this directory.
-if (loadModuleMapFile(SearchDirs[Idx].getDir(), IsSystem,
+if (loadModuleMapFile(It->getDir(), IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded) {
   // We just loaded a module map file; check whether the module is
   // available now.
@@ -345,7 +339,7 @@
 // Search for a module map in a subdirectory with the same name as the
 // module.
 SmallString<128> NestedModuleMapDirName;
-NestedModuleMapDirName = SearchDirs[Idx].getDir()->getName();
+NestedModuleMapDirName = It->getDir()->getName();
 llvm::sys::path::append(NestedModuleMapDirName, ModuleName);
 if (loadModuleMapFile(NestedModuleMapDirName, IsSystem,
   /*IsFramework*/false) == LMM_NewlyLoaded){
@@ -357,13 +351,13 @@
 
 // If we've already performed the exhaustive search for module maps in this
 // search directory, don't do it again.
-if (SearchDirs[Idx].haveSearchedAllModuleMaps())
+if (It->haveSearchedAllModuleMaps())
   continue;
 
 // Load all module maps in the immediate subdirectories of this search
 // directory if ModuleName was from @import.
 if (AllowExtraModuleMapSearch)
-  loadSubdirectoryModuleMaps(SearchDirs[Idx]);
+  loadSubdirectoryModuleMaps(*It);
 
 // Look again for the module.
 Module = ModMap.findModule(ModuleName);
@@ -372,7 +366,7 @@
   }
 
   if (Module)
-noteLookupUsage(Idx, ImportLoc);
+noteLookupUsage(It.Idx, ImportLoc);
 
   return Module;
 }
@@ -498,7 +492,7 @@
   // The case where the target file **exists** is handled by callee of this
   // function as part of the regular logic that applies to include search paths.
   // The case where the target file **does not exist** is handled here:
-  HS.noteLookupUsage(*HS.searchDirIdx(*this), IncludeLoc);
+  HS.noteLookupUsage(HS.searchDirIdx(*this), IncludeLoc);
   return None;
 }
 
@@ -707,7 +701,7 @@
   ConstSearchDirIterator HitIt,
   SourceLocation Loc) {
   CacheLookup.HitIt = HitIt;
-  noteLookupUsage(&*HitIt - &*search_dir_begin(), Loc);
+  noteLookupUsage(HitIt.Idx, Loc);
 }
 
 void HeaderSearch::noteLookupUsage(unsigned HitIdx, SourceLocation Loc) {
@@ -1452,11 +1446,8 @@
 + 

[PATCH] D119722: [clang][lex] Use `SearchDirIterator` types in for loops

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1450
 Optional HeaderSearch::searchDirIdx(const DirectoryLookup ) const 
{
-  for (unsigned I = 0; I < SearchDirs.size(); ++I)
-if ([I] == )
-  return I;
-  return None;
+  return  - &*SearchDirs.begin();
 }

ahoppen wrote:
> Could we change this function to return an `unsigned` instead of 
> `Optional` now?
> 
> Also, is ` - &*SearchDirs.begin()` safe and doesn’t trigger the issues we 
> saw previously if start and end are allocated in different memory regions, 
> because I don’t know.
Yes, updating the return type makes sense now, thanks!

Yes, this should be safe as of this patch, since we're still storing 
`DirectoryLookup` objects in `std::vector`. I'll be updating 
this code when we change the storage (separate `std::vector` 
for quoted, angled and system search paths).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119722

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

[PATCH] D116924: [clang-extdef-mapping] Allow clang-extdef-mapping tool to output customized filename for each index entry

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116924

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

[PATCH] D102614: [index] Add support for type of pointers to class members

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

ping.


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

https://reviews.llvm.org/D102614

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

[PATCH] D119806: [clangd][NFC] includes missing headers

[Sam McCall via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119806

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

I think I have found out the reason for the problem, and it proved my guesses.

When executing the test case of the static analyzer, we usually use 
`%clang_analyze_cc1` as the entry, which is `%clang_cc1 -analyze`.  And we 
usually do not set a target triple, as it is not required by the analyzer. 
However, things are complicated in Darwin Unix.

In Darwin, the default target triple is `ARCH-apple-darwinXX.XX.XX`, where 
`ARCH` is the architecture (e.g. `x86_64`) and `XX.XX.XX` is the version of the 
Darwin system. And the default target triple will be then adjusted to 
`ARCH-apple-SYSTEMXX.XX.XX` by the 
`Driver::Darwin::ComputeEffectiveClangTriple` in driver related code, where 
`SYSTEM` can be `watchos`, `tvos`, `ios` and `macosx`.

In the clang driver, the adjusted target triple will be passed to the new cc1 
process; whereas in tooling and `ASTUnit::LoadFromCommandLine`, the adjusted 
target triple will be used to generate cc1 arguments to create 
`CompilerInvocation`. But when executing bare `clang -cc1`, if the target 
triple argument is not provided, it remains the default 
`ARCH-apple-darwinXX.XX.XX`. And this is the reason for the conflict.

The CTU on-demand-parsing mechanism uses `ASTUnit::LoadFromCommandLine` to load 
external ASTs. The tool `clang-check` uses clang tooling to parse the entry 
file. Therefore, both target triples are the adjusted ones, which can be 
matched. And so is the driver (`clang --analyze ...`). But not the bare 
`%clang_cc1`, its target triple is the default one.

---

Let's have a look at the simple example, suppose externalDefMap.txt and 
invocations.yaml are generated correctly.

input.cc:

  void bar();
  void foo() { bar(); }

importee.cc:

  void bar() { }



---

Using driver:

  clang -v --analyze input.cc -Xanalyzer -analyzer-config -Xanalyzer 
experimental-enable-naive-ctu-analysis=true,ctu-dir=.

Output: OK, adjusted to adjusted

  (in-process)
  /path/to/clang-15 -cc1 -triple x86_64-apple-macosx10.15.0 ...



---

Using clang-check:

  clang-check -analyze input.cc -- -v -Xanalyzer -analyzer-config -Xanalyzer 
experimental-enable-naive-ctu-analysis=true,ctu-dir=.

Output: OK, adjusted to adjusted

  clang Invocation:
   "/path/to/clang-tool" "-cc1" "-triple" "x86_64-apple-macosx10.15.0"



---

Using cc1:

  clang -cc1 -v -analyze input.cc  -analyzer-checker=core -analyzer-config 
experimental-enable-naive-ctu-analysis=true,ctu-dir=.

Output: ERROR, default to adjusted

  warning: imported AST from 'importee.cc' had been generated for a different 
target, current: x86_64-apple-darwin19.6.0, imported: 
x86_64-apple-macosx10.15.0 [-Wctu]



---

What do you think is a better way to fix this problem? @gamesh411 @steakhal 
@martong 
Using `clang-check` to run the test case seems to be a good way to overcome the 
problem, but the problem still exists.
However, IMO it is not a good idea to make clang cc1 to adjust the default 
target triple manually.


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

https://reviews.llvm.org/D102669

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

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In my testing I was able to find a case where we go around `requires` like

  // module.modulemap
  module Main {
header "main.h"
export *
  
extern module A "extra.modulemap"
requires nonsense {
  extern module B "extra.modulemap"
}
  }

  // extra.modulemap
  module Main.A {}
  module Main.B {}

In this case we can do `@import Main.A` and `@import Main.B` despite 
unsatisfied `requires` in the module.modulemap. I'm not sure it is a bug but 
I'm not really in a position to predict the expectations of other developers, 
especially those not deeply familiar with module maps.

Other than this example, I haven't found any other strange `requires`/`extern` 
interactions. For example, attributes like `[extern_c]` are communicated 
through module/sub-module relationship and not through `extern` parsing 
location, so block `requires` does not affect the attributes (as far as I can 
tell).




Comment at: clang/lib/Lex/ModuleMap.cpp:2315-2320
+  bool Satisfied = true;
+  for (const RequiresFeature  : RFL) {
+if (Module::hasFeature(F.Feature, Map.LangOpts, *Map.Target) !=
+F.RequiredState)
+  Satisfied = false;
+  }

You can try using `all_of` for this computation. But I don't know without 
trying if it would improve readability, to be honest. If it looks clunky, I'm 
perfectly happy to keep the "for" loop.



Comment at: clang/lib/Lex/ModuleMap.cpp:3059
+case MMToken::RequiresKeyword:
+  parseRequiresDecl(true);
+  break;

`/*TopLevel=*/` would help understand the meaning of `true`.

Also I'm not sure if `parseRequiresDecl` parameter should have default value as 
we can update all the call places. But I don't have a strong opinion about that 
and leave the decision to you.



Comment at: clang/test/Modules/requires-block.m:8
+
+//--- include/module.modulemap
+

After reading the code it seems more like testing `skipUntil`, so I'm not sure 
it is a useful test. I was thinking about testing the closing brace as a token, 
i.e.,

```
requires checkClosingBraceNotAsSeparateToken {
  //}
}
```

If you think it doesn't add extra value, no need to add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118311

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

[PATCH] D119806: [clangd][NFC] includes missing headers

[Christopher Di Bella via Phabricator via cfe-commits]

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

`Shutdown.h` was transitively depending on two headers, but this isn't
allowed under a modules build, so they're now explicitly included.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119806

Files:
  clang-tools-extra/clangd/support/Shutdown.h


Index: clang-tools-extra/clangd/support/Shutdown.h
===
--- clang-tools-extra/clangd/support/Shutdown.h
+++ clang-tools-extra/clangd/support/Shutdown.h
@@ -45,6 +45,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {


Index: clang-tools-extra/clangd/support/Shutdown.h
===
--- clang-tools-extra/clangd/support/Shutdown.h
+++ clang-tools-extra/clangd/support/Shutdown.h
@@ -45,6 +45,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 408699.
li.zhe.hua added a comment.

Attempt fix for Windows compilation issue

Work around ambiguous function call by foregoing the delgating constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119745

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -26,6 +26,7 @@
 using ::clang::transformer::before;
 using ::clang::transformer::cat;
 using ::clang::transformer::changeTo;
+using ::clang::transformer::editList;
 using ::clang::transformer::makeRule;
 using ::clang::transformer::member;
 using ::clang::transformer::name;
@@ -36,6 +37,8 @@
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::UnorderedElementsAre;
 
 constexpr char KHeaderContents[] = R"cc(
   struct string {
@@ -120,10 +123,11 @@
 return *ChangedCode;
   }
 
-  Transformer::ChangeConsumer consumer() {
-return [this](Expected C) {
+  Transformer::ChangeSetConsumer consumer() {
+return [this](Expected> C) {
   if (C) {
-Changes.push_back(std::move(*C));
+Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
+   std::make_move_iterator(C->end()));
   } else {
 // FIXME: stash this error rather then printing.
 llvm::errs() << "Error generating changes: "
@@ -799,7 +803,6 @@
 }
 
 TEST_F(TransformerTest, EditList) {
-  using clang::transformer::editList;
   std::string Input = R"cc(
 void foo() {
   if (10 > 1.0)
@@ -827,7 +830,6 @@
 }
 
 TEST_F(TransformerTest, Flatten) {
-  using clang::transformer::editList;
   std::string Input = R"cc(
 void foo() {
   if (10 > 1.0)
@@ -1638,4 +1640,46 @@
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), format(Header));
 }
+
+// A single change set can span multiple files.
+TEST_F(TransformerTest, MultiFileEdit) {
+  // NB: The fixture is unused for this test, but kept for the test suite name.
+  std::string Header = R"cc(void Func(int id);)cc";
+  std::string Source = R"cc(#include "input.h"
+void Caller() {
+  int id = 0;
+  Func(id);
+})cc";
+  int ErrorCount = 0;
+  std::vector ChangeSets;
+  clang::ast_matchers::MatchFinder MatchFinder;
+  Transformer T(
+  makeRule(callExpr(callee(functionDecl(hasName("Func"))),
+forEachArgumentWithParam(expr().bind("arg"),
+ parmVarDecl().bind("param"))),
+   editList({changeTo(node("arg"), cat("ARG")),
+ changeTo(node("param"), cat("PARAM"))})),
+  [&](Expected> Changes) {
+if (Changes)
+  ChangeSets.push_back(AtomicChanges(Changes->begin(), Changes->end()));
+else
+  ++ErrorCount;
+  });
+  T.registerMatchers();
+  auto Factory = newFrontendActionFactory();
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  Factory->create(), Source, std::vector(), "input.cc",
+  "clang-tool", std::make_shared(),
+  {{"input.h", Header}}));
+
+  EXPECT_EQ(ErrorCount, 0);
+  EXPECT_THAT(
+  ChangeSets,
+  UnorderedElementsAre(UnorderedElementsAre(
+  ResultOf([](const AtomicChange ) { return C.getFilePath(); },
+   "input.cc"),
+  ResultOf([](const AtomicChange ) { return C.getFilePath(); },
+   "./input.h";
+}
+
 } // namespace
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
 }
   }
 
+  llvm::SmallVector Changes;
+  Changes.reserve(ChangesByFileID.size());
   for (auto  : ChangesByFileID)
-Consumer(std::move(IDChangePair.second));
+Changes.push_back(std::move(IDChangePair.second));
+  Consumer(llvm::MutableArrayRef(Changes));
 }
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,44 @@
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  using ChangeConsumer =
-  std::function Change)>;
+  using ChangeConsumer = std::function Change)>;
 
+  /// Provides the set of 

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

[Brad Smith 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 rGd241ce0f97e4: [Driver][NetBSD] -r: imply -nostdlib like GCC 
(authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119655

Files:
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Driver/netbsd.c


Index: clang/test/Driver/netbsd.c
===
--- clang/test/Driver/netbsd.c
+++ clang/test/Driver/netbsd.c
@@ -467,3 +467,11 @@
 // RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
+
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-netbsd -r \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=RELOCATABLE %s
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/NetBSD.cpp
===
--- clang/lib/Driver/ToolChains/NetBSD.cpp
+++ clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -236,7 +236,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
@@ -294,7 +295,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -330,7 +332,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));


Index: clang/test/Driver/netbsd.c
===
--- clang/test/Driver/netbsd.c
+++ clang/test/Driver/netbsd.c
@@ -467,3 +467,11 @@
 // RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
+
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-netbsd -r \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=RELOCATABLE %s
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/NetBSD.cpp
===
--- clang/lib/Driver/ToolChains/NetBSD.cpp
+++ clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -236,7 +236,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
@@ -294,7 +295,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -330,7 +332,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d241ce0 - [Driver][NetBSD] -r: imply -nostdlib like GCC

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2022-02-14T23:29:13-05:00
New Revision: d241ce0f97e47afa4f01a643ebbb4eafd729b403

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

LOG: [Driver][NetBSD] -r: imply -nostdlib like GCC

Similar to D116843 for Gnu.cpp

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/NetBSD.cpp
clang/test/Driver/netbsd.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/NetBSD.cpp 
b/clang/lib/Driver/ToolChains/NetBSD.cpp
index 37b1fc5215ff..d1eda14a51f0 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -236,7 +236,8 @@ void netbsd::Linker::ConstructJob(Compilation , const 
JobAction ,
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
@@ -294,7 +295,8 @@ void netbsd::Linker::ConstructJob(Compilation , const 
JobAction ,
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -330,7 +332,8 @@ void netbsd::Linker::ConstructJob(Compilation , const 
JobAction ,
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));

diff  --git a/clang/test/Driver/netbsd.c b/clang/test/Driver/netbsd.c
index 812889309a0f..b549b3cde9b5 100644
--- a/clang/test/Driver/netbsd.c
+++ b/clang/test/Driver/netbsd.c
@@ -467,3 +467,11 @@
 // RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
+
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-netbsd -r \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=RELOCATABLE %s
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o



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

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D119655#3318586 , @joerg wrote:

> Well, it doesn't work with GCC either, that's why I don't care much about 
> this change. It just attempts to legalize a user bug (using a linker option 
> directly as a compiler flag). But I don't care enough to object either.

-r is different from -Wl,-r.
GCC is free to assign specific semantics to -r, like -no-pie, -pie, -shared, to 
alter crt files and -l libraries a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119655

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

[PATCH] D119656: [Driver][DragonFly] -r: imply -nostdlib like GCC

[Brad Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcbd9d136ef81: [Driver][DragonFly] -r: imply -nostdlib like 
GCC (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119656

Files:
  clang/lib/Driver/ToolChains/DragonFly.cpp
  clang/test/Driver/dragonfly.c


Index: clang/test/Driver/dragonfly.c
===
--- clang/test/Driver/dragonfly.c
+++ clang/test/Driver/dragonfly.c
@@ -4,4 +4,9 @@
 // CHECK: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" 
"/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" 
"-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" 
"{{.*}}crtn.o"
 
-
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
+// RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/DragonFly.cpp
===
--- clang/lib/Driver/ToolChains/DragonFly.cpp
+++ clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -91,7 +91,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
 CmdArgs.push_back(
@@ -119,7 +120,8 @@
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 CmdArgs.push_back("-L/usr/lib/gcc80");
 
 if (!Args.hasArg(options::OPT_static)) {
@@ -158,7 +160,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));


Index: clang/test/Driver/dragonfly.c
===
--- clang/test/Driver/dragonfly.c
+++ clang/test/Driver/dragonfly.c
@@ -4,4 +4,9 @@
 // CHECK: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" "-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" "{{.*}}crtn.o"
 
-
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
+// RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/DragonFly.cpp
===
--- clang/lib/Driver/ToolChains/DragonFly.cpp
+++ clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -91,7 +91,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
 CmdArgs.push_back(
@@ -119,7 +120,8 @@
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 CmdArgs.push_back("-L/usr/lib/gcc80");
 
 if (!Args.hasArg(options::OPT_static)) {
@@ -158,7 +160,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cbd9d13 - [Driver][DragonFly] -r: imply -nostdlib like GCC

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2022-02-14T23:24:26-05:00
New Revision: cbd9d136ef8164970957317737cf51182acd236c

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

LOG: [Driver][DragonFly] -r: imply -nostdlib like GCC

Similar to D116843 for Gnu.cpp

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/DragonFly.cpp
clang/test/Driver/dragonfly.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/DragonFly.cpp 
b/clang/lib/Driver/ToolChains/DragonFly.cpp
index 9568b47e89e67..8cfec6a6c4e05 100644
--- a/clang/lib/Driver/ToolChains/DragonFly.cpp
+++ b/clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -91,7 +91,8 @@ void dragonfly::Linker::ConstructJob(Compilation , const 
JobAction ,
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
 CmdArgs.push_back(
@@ -119,7 +120,8 @@ void dragonfly::Linker::ConstructJob(Compilation , const 
JobAction ,
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 CmdArgs.push_back("-L/usr/lib/gcc80");
 
 if (!Args.hasArg(options::OPT_static)) {
@@ -158,7 +160,8 @@ void dragonfly::Linker::ConstructJob(Compilation , const 
JobAction ,
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));

diff  --git a/clang/test/Driver/dragonfly.c b/clang/test/Driver/dragonfly.c
index 20921bb38af0e..6c6d1909fe1b0 100644
--- a/clang/test/Driver/dragonfly.c
+++ b/clang/test/Driver/dragonfly.c
@@ -4,4 +4,9 @@
 // CHECK: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" 
"/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" 
"-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" 
"{{.*}}crtn.o"
 
-
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
+// RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o



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

[PATCH] D119714: [clang][lex] Remove `Preprocessor::GetCurDirLookup()`

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119714

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

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local 
)?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local 
)?}}constant i32 3,

urnathan wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > I don;t think this is correct.  That should still be a linkonce odr, 
> > > otherwise you'll get conflicts with other module implementation units.
> > It is still linkonce_odr in the module it get defined. See the new added 
> > test case: inline-variable-in-module.cpp for example. The attribute 
> > `available_externally` is equivalent to external from the perspective of 
> > linker. See https://llvm.org/docs/LangRef.html#linkage-types. According to 
> > [dcl.inline]p7, inline variable attached to named module should be defined 
> > in that domain. Note that the variable attached to global module fragment 
> > and private module fragment shouldn't be accessed outside the module, so it 
> > implies that all the variable defined in the module could only be defined 
> > in the module unit itself.
> There's a couple of issues with this.  module.cppm is emitting a (linkonce) 
> definition of inlne_var_exported, but only because it itself is ODR-using 
> that variable.  If you take out the ODR-use in noninline_exported, there is 
> no longer a symbol emitted.
> 
> But, even if you forced inline vars to be emitted in their defining-module's 
> interface unit, that would be an ABI change.  inline vars are emitted 
> whereever ODR-used.  They have no fixed home TU.  Now, we could alter the ABI 
> and allow interface units to define a home location for inline vars and 
> similar entities (eg, vtables for keyless classes).  But we'd need buy-in 
> from other compilers to do that.
> 
> FWIW such a discussion did come up early in implementing modules-ts, but we 
> decided there was enough going on just getting the TS implemented.  I'm fine 
> with revisiting that, but it is a more significant change.
> 
> And it wouldn't apply to (eg) templated variables, which of course could be 
> instantiated anywhere.
Oh, now the key point here is what the correct behavior is instead of the 
patch. Let's discuss it first.

According to [[ http://eel.is/c++draft/dcl.inline#7 | [dcl.inline]p7 ]], 
> If an inline function or variable that is attached to a named module is 
> declared in a definition domain, it shall be defined in that domain.

I think the intention of the sentence is to define inline variable in the 
module interface. So if it is required by the standard, I think other compiler 
need to follow up. As I described in the summary, it might be a difference 
between C++20 module and ModuleTS. Do you think it is necessary to send the 
question to WG21? (I get the behavior from reading the words. Maybe I misread 
or the word is not intentional).

Maybe the ABI standard group need to discuss what the linkage should be. Now it 
may be weak_odr or linkonce_odr. It depends on how we compile the file. If we 
compile the .cppm file directly, it would be linkonce_odr. And if we compile it 
to *.pcm file first, it would be weak_odr. I have registered an issue for this: 
https://github.com/llvm/llvm-project/issues/53838.


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

https://reviews.llvm.org/D119409

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

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 408694.
ChuanqiXu added a comment.

Update tests.


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

https://reviews.llvm.org/D119409

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
  clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
  clang/test/CodeGenCXX/inline-variable-in-module.cpp
  clang/test/CodeGenCXX/static-variable-in-module.cpp
  clang/test/CodeGenCXX/use-inline-variable-in-module.cpp

Index: clang/test/CodeGenCXX/use-inline-variable-in-module.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/use-inline-variable-in-module.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple \
+// RUN:  -emit-module-interface %S/inline-variable-in-module.cpp -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++20 -I%t %s -fprebuilt-module-path=%t \
+// RUN: -triple %itanium_abi_triple -S -emit-llvm -o - | FileCheck %s
+import m;
+void use() {
+  (void)
+}
+// CHECK: @a = available_externally global %class.A zeroinitializer
Index: clang/test/CodeGenCXX/static-variable-in-module.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/static-variable-in-module.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: echo "struct S { S(); };" >> %t/foo.h
+// RUN: echo "static S s = S();" >> %t/foo.h
+// RUN: %clang_cc1 -std=c++20 -I%t %s -emit-module-interface -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++20 %t/m.pcm -triple %itanium_abi_triple -S -emit-llvm -o - | FileCheck %s
+module;
+#include "foo.h"
+export module m;
+class A {
+public:
+  A();
+};
+static A a = A();
+
+// CHECK: @_ZW1mEL1s = internal global %struct.S zeroinitializer
+// CHECK: @_ZW1mE1a = {{(dso_local )?}}global %class.A zeroinitializer
+// CHECK: @llvm.global_ctors = appending global{{.*}}@_GLOBAL__sub_I_m.pcm
+// CHECK: define {{.*}}__cxx_global_var_init[[SUFFIX:[^)]*]]
+// CHECK: call void @_ZN1SC1Ev
+// CHECK: define {{.*}}__cxx_global_var_init[[SUFFIX2:[^)]*]]
+// CHECK: call void @_ZW1mEN1AC1Ev
+// CHECK: define {{.*}}@_GLOBAL__sub_I_m.pcm
+// CHECK: call void @__cxx_global_var_init[[SUFFIX]]
+// CHECK: call void @__cxx_global_var_init[[SUFFIX2]]
Index: clang/test/CodeGenCXX/inline-variable-in-module.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/inline-variable-in-module.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -I%t %s -triple %itanium_abi_triple -emit-module-interface -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++20 %t/m.pcm -triple %itanium_abi_triple -S -emit-llvm -o - | FileCheck %s
+module;
+# 3 __FILE__ 1
+struct S {
+  S();
+};
+inline S s = S();
+inline int si = 43;
+# 6 "" 2
+export module m;
+class A {
+public:
+  A();
+};
+export inline A a = A();
+export inline int b = 44;
+export inline int used_inline_int = 45;
+export void use() {
+  (void)_inline_int;
+}
+
+// CHECK: @used_inline_int = weak_odr global
+// CHECK: @a = weak_odr global
+// CHECK: @b = weak_odr global
Index: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
===
--- clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
+++ clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s --implicit-check-not=unused --implicit-check-not=global_module
 
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local )?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3
 
 import Module;
Index: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
===
--- clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
+++ clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
@@ -2,11 +2,11 @@
 // RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s --implicit-check-not=unused --implicit-check-not=global_module
 
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local )?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3,
 //
 // CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external {{(dso_local )?}}global
-// CHECK-DAG: @_ZW6ModuleE25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: 

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I am curious about the behavior if we removed the guard `if 
(OrigResultType.getBeginLoc().isValid())`.




Comment at: clang/lib/Sema/SemaStmt.cpp:3804-3807
+if (DAR != DAR_Succeeded) {
+  if (OrigResultType.getBeginLoc().isValid())
+Diag(OrigResultType.getBeginLoc(), diag::note_deducing_return_type_for)
+<< FD << OrigResultType.getSourceRange();

What if merge this one into above block where the diagnostic happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119778

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

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The Clang driver change should be in a separate patch with clang/test/Driver 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`GetTls` is about the static TLS block size. It consists of the main 
executable's TLS, and initially loaded shared objects' TLS, `struct pthread`, 
and padding.
Solaris should be able to just use the code path like Linux musl:

  #elif SANITIZER_FREEBSD || SANITIZER_LINUX
uptr align;
GetStaticTlsBoundary(addr, size, );

I think NetBSD should use this code path as well, but I don't have NetBSD for 
testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

In D119528#3319955 , @aaron.ballman 
wrote:

> LGTM, thank you for the additional test coverage! Do you need me to commit on 
> your behalf? If so, what name and email address would you like me to use for 
> patch attribution?

Thanks but I can push it on my own ;^)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119528

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

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Jun Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeffd6dd63a65: [Clang][Sema] Add a missing regression test 
about Wliteral-range (authored by junaire).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119528

Files:
  clang/test/Sema/warn-literal-range.c


Index: clang/test/Sema/warn-literal-range.c
===
--- /dev/null
+++ clang/test/Sema/warn-literal-range.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 -verify %s
+
+float f0 = 0.42f; // no-warning
+
+float f1 = 1.4E-46f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f2 = 3.4E+39f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+float f3 = 0x4.2p42f; // no-warning
+
+float f4 = 0x0.42p-1000f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f5 = 0x0.42p+1000f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+double d0 = 0.42; // no-warning
+
+double d1 = 3.6E-4952; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d2 = 1.7E+309; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+double d3 = 0x0.42p42; // no-warning
+
+double d4 = 0x0.42p-4200; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d5 = 0x0.42p+4200; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+long double ld0 = 0.42L; // no-warning
+
+long double ld1 = 3.6E-4952L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld2 = 1.2E+4932L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}
+
+long double ld3 = 0x0.42p42L; // no-warning
+
+long double ld4 = 0x0.42p-42000L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld5 = 0x0.42p+42000L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}


Index: clang/test/Sema/warn-literal-range.c
===
--- /dev/null
+++ clang/test/Sema/warn-literal-range.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 -verify %s
+
+float f0 = 0.42f; // no-warning
+
+float f1 = 1.4E-46f; // expected-warning {{magnitude of floating-point constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f2 = 3.4E+39f; // expected-warning {{magnitude of floating-point constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+float f3 = 0x4.2p42f; // no-warning
+
+float f4 = 0x0.42p-1000f; // expected-warning {{magnitude of floating-point constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f5 = 0x0.42p+1000f; // expected-warning {{magnitude of floating-point constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+double d0 = 0.42; // no-warning
+
+double d1 = 3.6E-4952; // expected-warning {{magnitude of floating-point constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d2 = 1.7E+309; // expected-warning {{magnitude of floating-point constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+double d3 = 0x0.42p42; // no-warning
+
+double d4 = 0x0.42p-4200; // expected-warning {{magnitude of floating-point constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d5 = 0x0.42p+4200; // expected-warning {{magnitude of floating-point constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+long double ld0 = 0.42L; // no-warning
+
+long double ld1 = 3.6E-4952L; // expected-warning {{magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951}}
+
+long double ld2 = 1.2E+4932L; // expected-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932}}
+
+long double ld3 = 0x0.42p42L; // no-warning
+
+long double ld4 = 0x0.42p-42000L; // expected-warning {{magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951}}
+
+long double ld5 = 0x0.42p+42000L; // expected-warning 

[clang] effd6dd - [Clang][Sema] Add a missing regression test about Wliteral-range

[Jun Zhang via cfe-commits]

Author: Jun Zhang
Date: 2022-02-15T09:46:42+08:00
New Revision: effd6dd63a65f201b4f8f1be6a025b0608604449

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

LOG: [Clang][Sema] Add a missing regression test about Wliteral-range

This patch adds a missing test, covering the different kinds of floating-point
literals, like hexadecimal floats, and testing extreme cases & normal cases.

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

Added: 
clang/test/Sema/warn-literal-range.c

Modified: 


Removed: 




diff  --git a/clang/test/Sema/warn-literal-range.c 
b/clang/test/Sema/warn-literal-range.c
new file mode 100644
index 0..8014b058bd886
--- /dev/null
+++ b/clang/test/Sema/warn-literal-range.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 -verify %s
+
+float f0 = 0.42f; // no-warning
+
+float f1 = 1.4E-46f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f2 = 3.4E+39f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+float f3 = 0x4.2p42f; // no-warning
+
+float f4 = 0x0.42p-1000f; // expected-warning {{magnitude of floating-point 
constant too small for type 'float'; minimum is 1.40129846E-45}}
+
+float f5 = 0x0.42p+1000f; // expected-warning {{magnitude of floating-point 
constant too large for type 'float'; maximum is 3.40282347E+38}}
+
+double d0 = 0.42; // no-warning
+
+double d1 = 3.6E-4952; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d2 = 1.7E+309; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+double d3 = 0x0.42p42; // no-warning
+
+double d4 = 0x0.42p-4200; // expected-warning {{magnitude of floating-point 
constant too small for type 'double'; minimum is 4.9406564584124654E-324}}
+
+double d5 = 0x0.42p+4200; // expected-warning {{magnitude of floating-point 
constant too large for type 'double'; maximum is 1.7976931348623157E+308}}
+
+long double ld0 = 0.42L; // no-warning
+
+long double ld1 = 3.6E-4952L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld2 = 1.2E+4932L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}
+
+long double ld3 = 0x0.42p42L; // no-warning
+
+long double ld4 = 0x0.42p-42000L; // expected-warning {{magnitude of 
floating-point constant too small for type 'long double'; minimum is 
3.64519953188247460253E-4951}}
+
+long double ld5 = 0x0.42p+42000L; // expected-warning {{magnitude of 
floating-point constant too large for type 'long double'; maximum is 
1.18973149535723176502E+4932}}



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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Hyeongyu Kim via Phabricator via cfe-commits]

hyeongyukim added a comment.

Thank you for your clarification. I'll change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D119479: [clang][extract-api] Add global record support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 408673.
zixuw added a comment.

Convert file path to forward slashes for the URI scheme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119479

Files:
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/SymbolGraph/API.h
  clang/include/clang/SymbolGraph/AvailabilityInfo.h
  clang/include/clang/SymbolGraph/DeclarationFragments.h
  clang/include/clang/SymbolGraph/Serialization.h
  clang/lib/AST/RawCommentList.cpp
  clang/lib/CMakeLists.txt
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/ExtractAPIConsumer.cpp
  clang/lib/SymbolGraph/API.cpp
  clang/lib/SymbolGraph/CMakeLists.txt
  clang/lib/SymbolGraph/DeclarationFragments.cpp
  clang/lib/SymbolGraph/Serialization.cpp
  clang/test/Driver/extract-api.c
  clang/test/SymbolGraph/global_record.c

Index: clang/test/SymbolGraph/global_record.c
===
--- /dev/null
+++ clang/test/SymbolGraph/global_record.c
@@ -0,0 +1,363 @@
+// 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 -target arm64-apple-macosx \
+// RUN: %t/input.c -o %t/output.json | FileCheck -allow-empty %s
+// RUN: diff %t/reference.output.json %t/output.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.c
+int num;
+
+/**
+ * \brief Add two numbers.
+ * \param [in]  x   A number.
+ * \param [in]  y   Another number.
+ * \param [out] res The result of x + y.
+ */
+void add(const int x, const int y, int *res);
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "clang"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declaration": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "num"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@num"
+  },
+  "kind": {
+"displayName": "Variable",
+"identifier": "c.variable"
+  },
+  "location": {
+"character": 5,
+"line": 1,
+"uri": "file://INPUT_DIR/input.c"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "num"
+  }
+],
+"title": "num"
+  }
+},
+{
+  "declaration": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "add"
+},
+{
+  "kind": "text",
+  "spelling": "("
+},
+{
+  "kind": "keyword",
+  "spelling": "const"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "internalParam",
+  "spelling": "x"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "keyword",
+  "spelling": "const"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "internalParam",
+  "spelling": "y"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " *"
+},
+{
+  "kind": "internalParam",
+  "spelling": "res"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+   

[PATCH] D119761: [OpenMP]Fix parsing of OpenMP directive nested in a metadirective

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83a407d176f8: [OpenMP]Fix parsing of OpenMP directive nested 
in a metadirective (authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119761

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/metadirective_ast_print.c


Index: clang/test/OpenMP/metadirective_ast_print.c
===
--- clang/test/OpenMP/metadirective_ast_print.c
+++ clang/test/OpenMP/metadirective_ast_print.c
@@ -47,6 +47,16 @@
: parallel) default(parallel for)
   for (int i = 0; i < 100; i++)
 ;
+
+// Test metadirective with nested OpenMP directive.
+  int array[16];
+  #pragma omp metadirective when(user = {condition(1)} \
+ : parallel for)
+  for (int i = 0; i < 16; i++) {
+#pragma omp simd
+for (int j = 0; j < 16; j++)
+  array[i] = i;
+  }
 }
 
 // CHECK: void bar();
@@ -69,5 +79,9 @@
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
 // CHECK: #pragma omp parallel
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
+// CHECK: #pragma omp parallel for
+// CHECK-NEXT: for (int i = 0; i < 16; i++) {
+// CHECK-NEXT: #pragma omp simd
+// CHECK-NEXT: for (int j = 0; j < 16; j++)
 
 #endif
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2451,9 +2451,8 @@
 /// for simd' | 'target teams distribute simd' | 'masked' {clause}
 /// annot_pragma_openmp_end
 ///
-StmtResult
-Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) 
{
-  static bool ReadDirectiveWithinMetadirective = false;
+StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
+ParsedStmtContext StmtCtx, bool ReadDirectiveWithinMetadirective) {
   if (!ReadDirectiveWithinMetadirective)
 assert(Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp) &&
"Not an OpenMP directive!");
@@ -2615,9 +2614,9 @@
   }
 
   // Parse Directive
-  ReadDirectiveWithinMetadirective = true;
-  Directive = ParseOpenMPDeclarativeOrExecutableDirective(StmtCtx);
-  ReadDirectiveWithinMetadirective = false;
+  Directive = ParseOpenMPDeclarativeOrExecutableDirective(
+  StmtCtx,
+  /*ReadDirectiveWithinMetadirective=*/true);
   break;
 }
 break;
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -3291,8 +3291,10 @@
   /// Parses declarative or executable directive.
   ///
   /// \param StmtCtx The context in which we're parsing the directive.
-  StmtResult
-  ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx);
+  /// \param ReadDirectiveWithinMetadirective true if directive is within a
+  /// metadirective and therefore ends on the closing paren.
+  StmtResult ParseOpenMPDeclarativeOrExecutableDirective(
+  ParsedStmtContext StmtCtx, bool ReadDirectiveWithinMetadirective = 
false);
   /// Parses clause of kind \a CKind for directive of a kind \a Kind.
   ///
   /// \param DKind Kind of current directive.


Index: clang/test/OpenMP/metadirective_ast_print.c
===
--- clang/test/OpenMP/metadirective_ast_print.c
+++ clang/test/OpenMP/metadirective_ast_print.c
@@ -47,6 +47,16 @@
: parallel) default(parallel for)
   for (int i = 0; i < 100; i++)
 ;
+
+// Test metadirective with nested OpenMP directive.
+  int array[16];
+  #pragma omp metadirective when(user = {condition(1)} \
+ : parallel for)
+  for (int i = 0; i < 16; i++) {
+#pragma omp simd
+for (int j = 0; j < 16; j++)
+  array[i] = i;
+  }
 }
 
 // CHECK: void bar();
@@ -69,5 +79,9 @@
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
 // CHECK: #pragma omp parallel
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
+// CHECK: #pragma omp parallel for
+// CHECK-NEXT: for (int i = 0; i < 16; i++) {
+// CHECK-NEXT: #pragma omp simd
+// CHECK-NEXT: for (int j = 0; j < 16; j++)
 
 #endif
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2451,9 +2451,8 @@
 /// for simd' | 'target teams distribute simd' | 'masked' {clause}
 /// annot_pragma_openmp_end
 ///
-StmtResult
-Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
-  static bool ReadDirectiveWithinMetadirective = false;
+StmtResult 

[clang] 83a407d - [OpenMP]Fix parsing of OpenMP directive nested in a metadirective

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2022-02-14T16:15:20-08:00
New Revision: 83a407d176f84c53633a2ef259642ff184e0613f

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

LOG: [OpenMP]Fix parsing of OpenMP directive nested in a metadirective

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

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseOpenMP.cpp
clang/test/OpenMP/metadirective_ast_print.c

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 8c37ac721aed4..981800a7e2356 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3291,8 +3291,10 @@ class Parser : public CodeCompletionHandler {
   /// Parses declarative or executable directive.
   ///
   /// \param StmtCtx The context in which we're parsing the directive.
-  StmtResult
-  ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx);
+  /// \param ReadDirectiveWithinMetadirective true if directive is within a
+  /// metadirective and therefore ends on the closing paren.
+  StmtResult ParseOpenMPDeclarativeOrExecutableDirective(
+  ParsedStmtContext StmtCtx, bool ReadDirectiveWithinMetadirective = 
false);
   /// Parses clause of kind \a CKind for directive of a kind \a Kind.
   ///
   /// \param DKind Kind of current directive.

diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 4910e6ce9ba96..56960624c0bfd 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2451,9 +2451,8 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
 /// for simd' | 'target teams distribute simd' | 'masked' {clause}
 /// annot_pragma_openmp_end
 ///
-StmtResult
-Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) 
{
-  static bool ReadDirectiveWithinMetadirective = false;
+StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
+ParsedStmtContext StmtCtx, bool ReadDirectiveWithinMetadirective) {
   if (!ReadDirectiveWithinMetadirective)
 assert(Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp) &&
"Not an OpenMP directive!");
@@ -2615,9 +2614,9 @@ 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
   }
 
   // Parse Directive
-  ReadDirectiveWithinMetadirective = true;
-  Directive = ParseOpenMPDeclarativeOrExecutableDirective(StmtCtx);
-  ReadDirectiveWithinMetadirective = false;
+  Directive = ParseOpenMPDeclarativeOrExecutableDirective(
+  StmtCtx,
+  /*ReadDirectiveWithinMetadirective=*/true);
   break;
 }
 break;

diff  --git a/clang/test/OpenMP/metadirective_ast_print.c 
b/clang/test/OpenMP/metadirective_ast_print.c
index 582de7c606da5..fbd7e2291330f 100644
--- a/clang/test/OpenMP/metadirective_ast_print.c
+++ b/clang/test/OpenMP/metadirective_ast_print.c
@@ -47,6 +47,16 @@ void foo(void) {
: parallel) default(parallel for)
   for (int i = 0; i < 100; i++)
 ;
+
+// Test metadirective with nested OpenMP directive.
+  int array[16];
+  #pragma omp metadirective when(user = {condition(1)} \
+ : parallel for)
+  for (int i = 0; i < 16; i++) {
+#pragma omp simd
+for (int j = 0; j < 16; j++)
+  array[i] = i;
+  }
 }
 
 // CHECK: void bar();
@@ -69,5 +79,9 @@ void foo(void) {
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
 // CHECK: #pragma omp parallel
 // CHECK-NEXT: for (int i = 0; i < 100; i++)
+// CHECK: #pragma omp parallel for
+// CHECK-NEXT: for (int i = 0; i < 16; i++) {
+// CHECK-NEXT: #pragma omp simd
+// CHECK-NEXT: for (int j = 0; j < 16; j++)
 
 #endif



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

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In D115844#3321235 , @ychen wrote:

> In D115844#3321190 , @pcc wrote:
>
>> On the bug you have:
>>
>>   define internal fastcc void 
>> @_Z4callIiE4taskv.resume(%_Z4callIiE4taskv.Frame* noalias nonnull 
>> align 8 dereferenceable(24
>>   ) %FramePtr) #1 prologue <{ i32, i32 }> <{ i32 846595819, i32 
>> trunc (i64 sub (i64 ptrtoint (i8** @1 to i64), i64 ptrtoint (void ()* 
>> @_Z4callIiE4taskv to i64)) to i32) }> {...}
>>
>> Is it possible for the C/C++ code to take the address of the function 
>> `_Z4callIiE4taskv.resume` and call it indirectly?
>
> `*.resume` is a compiler inserted function that is opaque to the programmer. 
> It is called indirectly most of the time if not all the time.

Yes, but not indirectly called from C/C++ but rather from compiler-generated 
code right? That's presumably why this patch + D116130 
 worked for @zhuhan0.

>> If not, it seems like the right fix would be to arrange for the prologue 
>> data to be dropped on the `.resume` function instead of duplicating it 
>> there. I would also imagine that whatever signature you have on the 
>> `.resume` function would be incorrect since it appears that the coro 
>> splitting pass will use a different function signature for that function.
>
> That is addressed by D116130 . @rjmccall 
> suggested the direction of this patch (which I agreed) 
> https://reviews.llvm.org/D114728#3159303.

Okay. It seems unfortunate to have to special-case this just because it uses 
relative relocations. But that's probably the best that we can do without 
changing the global variable initializer representation (as well as 
prefix/prologue data) to be blob plus relocations.




Comment at: clang/lib/CodeGen/CGExpr.cpp:5171
   (!TargetDecl || !isa(TargetDecl))) {
+assert((CGM.getCodeGenOpts().CodeModel == "default" ||
+CGM.getCodeGenOpts().CodeModel == "small") &&

What happens when building with other code models? Hopefully we get an error of 
some sort before hitting this assertion failure, right?



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:843
+
+  // Emit the function prologue data for the indirect call sanitizer.
+  if (const MDNode *MD = F.getMetadata(LLVMContext::MD_func_sanitize)) {

What if we have both prologue data and this metadata? Should it be an error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115844

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

[PATCH] D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D118862#3321343 , @gulfem wrote:

> The following two tests started failing in our bots:
>
>   Clang :: Driver/darwin-ld-platform-version-target-version.c
>   Clang :: Driver/darwin-zippered-target-version.c
>
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8822219707275734081/overview
>
> The error message is as the following:
>
>   Script:
>   --
>   : 'RUN: at line 1';   touch 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>   : 'RUN: at line 3';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> x86_64-apple-ios13.1-macabi -darwin-target-variant x86_64-apple-macos10.15 
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=520 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   : 'RUN: at line 5';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> x86_64-apple-macos10.14.3 -darwin-target-variant x86_64-apple-ios13.1-macabi 
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=520 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> --check-prefix=CHECK-INV 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   : 'RUN: at line 8';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> arm64-apple-ios13.1-macabi -darwin-target-variant arm64-apple-macos10.15 
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=520 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> --check-prefix=ARM64_NEW 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   : 'RUN: at line 10';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> arm64-apple-macos10.15 -darwin-target-variant arm64-apple-ios13.1-macabi  
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=520 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> --check-prefix=ARM64_NEW-INV 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   : 'RUN: at line 12';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> arm64-apple-ios13.1-macabi -darwin-target-variant arm64-apple-macos10.15 
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=400 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> --check-prefix=ARM64_OLD 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   : 'RUN: at line 14';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
> arm64-apple-macos10.15 -darwin-target-variant arm64-apple-ios13.1-macabi 
> -isysroot 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
>  -mlinker-version=400 -### 
> /b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
>  2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
> --check-prefix=ARM64_OLD-INV 
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
>   --
>   Exit Code: 1
>   
>   Command Output (stderr):
>   --
>   
> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c:29:15:
>  error: ARM64_OLD: expected string not found in input
>   // ARM64_OLD: "-maccatalyst_version_min" "14.0.0" "-macosx_version_min" 
> "11.0.0"
> ^
>   :1:1: note: scanning from here
>   Fuchsia clang version 15.0.0 (https://llvm.googlesource.com/a/llvm-project 
> cccef321096c20825fe8738045c1d91d3b9fd57d)
>   ^
>   :5:109: note: possible intended match here
>"/b/s/w/ir/x/w/staging/llvm_build/bin/ld64.lld" "-demangle" "-dynamic" 
> "-arch" "arm64" "-platform_version" "mac catalyst" "14.0.0" "13.1" 
> "-platform_version" "macos" "11.0.0" "10.15" "-syslibroot" 
> "/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk"
>  "-o" "a.out" 
> 

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa4a0f1d31e2: [modules] Add a flag for TagDecl if it was a 
definition demoted to a… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/Decl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -773,7 +773,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
-  ED->setCompleteDefinition(false);
+  ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
   if (OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
@@ -828,7 +828,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
-  RD->setCompleteDefinition(false);
+  RD->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, RD);
 } else {
   OldDef = RD;
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4301,6 +4301,7 @@
   setEmbeddedInDeclarator(false);
   setFreeStanding(false);
   setCompleteDefinitionRequired(false);
+  TagDeclBits.IsThisDeclarationADemotedDefinition = false;
 }
 
 SourceLocation TagDecl::getOuterLocStart() const {
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1443,10 +1443,14 @@
 /// Has the full definition of this type been required by a use somewhere 
in
 /// the TU.
 uint64_t IsCompleteDefinitionRequired : 1;
+
+/// Whether this tag is a definition which was demoted due to
+/// a module merge.
+uint64_t IsThisDeclarationADemotedDefinition : 1;
   };
 
   /// Number of non-inherited bits in TagDeclBitfields.
-  enum { NumTagDeclBits = 9 };
+  enum { NumTagDeclBits = 10 };
 
   /// Stores the bits used by EnumDecl.
   /// If modified NumEnumDeclBit and the accessor
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3486,6 +3486,24 @@
   /// parameters.
   bool isDependentType() const { return isDependentContext(); }
 
+  /// Whether this declaration was a definition in some module but was forced
+  /// to be a declaration.
+  ///
+  /// Useful for clients checking if a module has a definition of a specific
+  /// symbol and not interested in the final AST with deduplicated definitions.
+  bool isThisDeclarationADemotedDefinition() const {
+return TagDeclBits.IsThisDeclarationADemotedDefinition;
+  }
+
+  /// Mark a definition as a declaration and maintain information it _was_
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() &&
+   "Should demote definitions only, not forward declarations");
+setCompleteDefinition(false);
+TagDeclBits.IsThisDeclarationADemotedDefinition = true;
+  }
+
   /// Starts the definition of this tag declaration.
   ///
   /// This method should be invoked at the beginning of the definition


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -773,7 +773,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
-  ED->setCompleteDefinition(false);
+  ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
   if (OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
@@ -828,7 +828,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
-  RD->setCompleteDefinition(false);
+  RD->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, RD);
 } else {
   OldDef = RD;
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4301,6 +4301,7 @@
   setEmbeddedInDeclarator(false);
   setFreeStanding(false);
   setCompleteDefinitionRequired(false);
+  TagDeclBits.IsThisDeclarationADemotedDefinition = false;
 }
 
 SourceLocation TagDecl::getOuterLocStart() const {
Index: clang/include/clang/AST/DeclBase.h

[clang] fa4a0f1 - [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2022-02-14T16:04:40-08:00
New Revision: fa4a0f1d31e2f797c3e27eb7cad15755e46a4726

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

LOG: [modules] Add a flag for TagDecl if it was a definition demoted to a 
declaration.

For redeclaration chains we maintain an invariant of having only a
single definition in the chain. In a single translation unit we make
sure not to create duplicates. But modules are separate translation
units and they can contain definitions for the same symbol
independently. When we load such modules together, we need to demote
duplicate definitions to keep the AST invariants.

Some AST clients are interested in distinguishing
declaration-that-was-demoted-from-definition and
declaration-that-was-never-a-definition. For that purpose introducing
`IsThisDeclarationADemotedDefinition`. No functional change intended.

rdar://84677782

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

Added: 


Modified: 
clang/include/clang/AST/Decl.h
clang/include/clang/AST/DeclBase.h
clang/lib/AST/Decl.cpp
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 862e8899d2751..7611bac83419d 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3486,6 +3486,24 @@ class TagDecl : public TypeDecl,
   /// parameters.
   bool isDependentType() const { return isDependentContext(); }
 
+  /// Whether this declaration was a definition in some module but was forced
+  /// to be a declaration.
+  ///
+  /// Useful for clients checking if a module has a definition of a specific
+  /// symbol and not interested in the final AST with deduplicated definitions.
+  bool isThisDeclarationADemotedDefinition() const {
+return TagDeclBits.IsThisDeclarationADemotedDefinition;
+  }
+
+  /// Mark a definition as a declaration and maintain information it _was_
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() &&
+   "Should demote definitions only, not forward declarations");
+setCompleteDefinition(false);
+TagDeclBits.IsThisDeclarationADemotedDefinition = true;
+  }
+
   /// Starts the definition of this tag declaration.
   ///
   /// This method should be invoked at the beginning of the definition

diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 06d2f17d14300..a89f776248c1f 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1443,10 +1443,14 @@ class DeclContext {
 /// Has the full definition of this type been required by a use somewhere 
in
 /// the TU.
 uint64_t IsCompleteDefinitionRequired : 1;
+
+/// Whether this tag is a definition which was demoted due to
+/// a module merge.
+uint64_t IsThisDeclarationADemotedDefinition : 1;
   };
 
   /// Number of non-inherited bits in TagDeclBitfields.
-  enum { NumTagDeclBits = 9 };
+  enum { NumTagDeclBits = 10 };
 
   /// Stores the bits used by EnumDecl.
   /// If modified NumEnumDeclBit and the accessor

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 060a6d1ad5ed5..030da7f55fac4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4301,6 +4301,7 @@ TagDecl::TagDecl(Kind DK, TagKind TK, const ASTContext 
, DeclContext *DC,
   setEmbeddedInDeclarator(false);
   setFreeStanding(false);
   setCompleteDefinitionRequired(false);
+  TagDeclBits.IsThisDeclarationADemotedDefinition = false;
 }
 
 SourceLocation TagDecl::getOuterLocStart() const {

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 1ab26e58a4040..25d7e9e6a2e68 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -773,7 +773,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
-  ED->setCompleteDefinition(false);
+  ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
   if (OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
@@ -828,7 +828,7 @@ void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) {
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
-  RD->setCompleteDefinition(false);
+  RD->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, RD);
 } else {
   OldDef = RD;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

The following two tests started failing in our bots:

  Clang :: Driver/darwin-ld-platform-version-target-version.c
  Clang :: Driver/darwin-zippered-target-version.c

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8822219707275734081/overview

The error message is as the following:

  Script:
  --
  : 'RUN: at line 1';   touch 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
  : 'RUN: at line 3';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
x86_64-apple-ios13.1-macabi -darwin-target-variant x86_64-apple-macos10.15 
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=520 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  : 'RUN: at line 5';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
x86_64-apple-macos10.14.3 -darwin-target-variant x86_64-apple-ios13.1-macabi 
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=520 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=CHECK-INV 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  : 'RUN: at line 8';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
arm64-apple-ios13.1-macabi -darwin-target-variant arm64-apple-macos10.15 
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=520 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=ARM64_NEW 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  : 'RUN: at line 10';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
arm64-apple-macos10.15 -darwin-target-variant arm64-apple-ios13.1-macabi  
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=520 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=ARM64_NEW-INV 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  : 'RUN: at line 12';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
arm64-apple-ios13.1-macabi -darwin-target-variant arm64-apple-macos10.15 
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=400 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=ARM64_OLD 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  : 'RUN: at line 14';   /b/s/w/ir/x/w/staging/llvm_build/bin/clang -target 
arm64-apple-macos10.15 -darwin-target-variant arm64-apple-ios13.1-macabi 
-isysroot 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk
 -mlinker-version=400 -### 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o
 2>&1| /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=ARM64_OLD-INV 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/darwin-ld-platform-version-target-version.c:29:15:
 error: ARM64_OLD: expected string not found in input
  // ARM64_OLD: "-maccatalyst_version_min" "14.0.0" "-macosx_version_min" 
"11.0.0"
^
  :1:1: note: scanning from here
  Fuchsia clang version 15.0.0 (https://llvm.googlesource.com/a/llvm-project 
cccef321096c20825fe8738045c1d91d3b9fd57d)
  ^
  :5:109: note: possible intended match here
   "/b/s/w/ir/x/w/staging/llvm_build/bin/ld64.lld" "-demangle" "-dynamic" 
"-arch" "arm64" "-platform_version" "mac catalyst" "14.0.0" "13.1" 
"-platform_version" "macos" "11.0.0" "10.15" "-syslibroot" 
"/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/MacOSX10.15.versioned.sdk"
 "-o" "a.out" 
"/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/darwin-ld-platform-version-target-version.c.tmp.o"
 "-lSystem"

  ^
  
  Input file: 
  Check file: 

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D105169#3321237 , @hyeongyukim 
wrote:

> In D105169#3319773 , @dblaikie 
> wrote:
>
>> In D105169#3315009 , @MaskRay 
>> wrote:
>>
>>> It may not be worth changing now, but I want to mention: it's more 
>>> conventional to have a `BoolOption` which adds `-[no-]noundef-analysis`. 
>>> Since both positive and negative forms exist. When we make the default 
>>> switch, existing users don't need to change the option. After the option 
>>> becomes quite stable and the workaround is deemed not useful, we can remove 
>>> the CC1 option.
>>
>> +1 to this (changing the name and the default at the same time makes 
>> migrations a bit more difficult - if the default is changed without renaming 
>> (by having both positive and negative flag names) then users can adopt their 
>> current default explicitly with no change ahead of picking up the patch that 
>> changes the default) & also this flag seems to have no tests? Could you 
>> (@hyeongyukim ) add some frontend test coverage for the flag - and yeah, 
>> maybe consider giving it a name that has both explicit on/off names, as 
>> @maskray suggested? (I think that's useful even after the default switch - 
>> since a user might want to override a previous argument on the command line, 
>> etc)
>
> Sure, I'll add some tests.
> By the way, is it right to change the flag's name to 
> `-[no-]enable-noundef-analysis`? or would it be better to use 
> `-[no-]noundef-analysis` as @MaskRay suggested?
> I prefer to use `-[no-]enable-noundef-analysis` to maintain backward 
> compatibility.

For driver and CC1 options, the convention of boolean options is to use 
`[no-]`, not use `enable-` or `disable-`.
That said, `-[no-]enable-noundef-analysis` sounds good to me since 
`no-noundef-analysis` may be odd and `-enable-noundef-analysis` maintains 
backward compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

Cases that show the difference (they're covered in tests, though do we need an 
AST test as well?):

  X test(bool B) {
if (B) {
  X y; // before: nrvo, after: nrvo (same)
  return y;
}
X x; // before: no nrvo, after: nrvo (better)
return x;
  }

  X test(bool B) {
X x; // before: no nrvo, after: no nrvo (same)
if (B)
  return x;
X y; // before: no nrvo, after: nrvo (better)
return y;
  }

  X test(bool A, bool B) {
{
  {
X x; // before: nrvo, after: nrvo (same)
if (A)
  return x;
  }
  X y; // before: no nrvo, after: nrvo (better)
  if (B)
return y;
}
X z;
retur n z; // before: no nrvo, after: nrvo (better)
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

There is a nice proposal (P2025) Guaranteed copy elision for return variables 
 by 
**Anton Zhilin**. The poll on the proposal showed that its ideas are very 
welcome: link to cplusplus/papers github 
. 
Although the proposal is not yet accepted into the Standard in its current 
wording, some ideas are still good and can be implemented "in advance".

> This proposal aims to provide guaranteed copy elision for common cases of 
> local variables being returned from a function, a.k.a. "guaranteed NRVO".

С++ сompiler implementations determine by their own rules whether there will be 
a NRVO (since it is not a required optimization). The proposal contains a 
collection of common cases of local variables being returned where copy elision 
is safe and makes senses and therefore is desirable to do.

The main requirement (omitting details for language-lawyers) is that:

> Copy elision is guaranteed for return x; if every return "seen" by x is 
> return x;

"seen by `x`" means here "all non-discarded return statements in `x`'s 
potential scope". There are more details in the proposal.

The collection of common cases contains 20 examples: §4.1. Examples 
.
Here is the current status of these examples:

- [OK] 13 out of 20 examples are working in Clang as expected.
- [FAIL] 13th example: should be considered separately (as part of fixing 
consteval code)
- [FAIL] 14th example: should be considered separately (as I haven't looked yet 
how `CXXCatchStmt` works).
- [FAIL] 18th example: is unfixable now because of Clang's architecture: my 
comment on the issue 
.
- **[OK with the patch]** 7th, 8th, 11th, 15th example: are working with this 
patch.

In order to make the last group of 4 examples working, there is a need to 
rewrite the logic for calculating the NRVO candidate.
The `clang::Scope` class has methods `void AddDecl(Decl *D)`, `void 
addNRVOCandidate(VarDecl *VD)`, `void setNoNRVO()`, that are called in the 
order of the scope's parsing.

- `void AddDecl(Decl *D)` is called when there is a new `Decl *D` in the scope. 
The `D`'s potential scope starts now.
- `void addNRVOCandidate(VarDecl *VD)` is called when there is a `return 
` in the scope or when a children scope has successfully 
calculated its single NRVO candidate.
- `void setNoNRVO()` is called when there is a `return ` in 
the scope or when a children scope is telling us to drop our NRVO candidates 
(nevertheless, the children scope now can still succesfully calculated the NRVO 
candidate).

We need to have a set of "unspoiled variables" to find the NRVO candidate 
effectively (yeah, introducing some made up terminology...) A variable is 
"unspoiled" if it is not forbidden to be the NRVO candidate yet. An `AddDecl` 
call adds the variable to this set. An `addNRVOCandidate` call "spoils" every 
variable except `VD`. A `setNoNRVO` "spoils" every variable. Only an "unspoiled 
variable" may be the NRVO candidate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron created this revision.
Izaron added reviewers: lebedev.ri, rsmith.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before the patch we calculated the NRVO candidate looking at the
variable's whole enclosing scope. The research in [P2025 
] shows that looking
at the variable's potential scope is better and covers more cases where NRVO
would be safe and desirable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119792

Files:
  clang/include/clang/Sema/Scope.h
  clang/test/CodeGenCXX/nrvo.cpp

Index: clang/test/CodeGenCXX/nrvo.cpp
===
--- clang/test/CodeGenCXX/nrvo.cpp
+++ clang/test/CodeGenCXX/nrvo.cpp
@@ -166,88 +166,19 @@
 
 // CHECK-LABEL: @_Z5test3b(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[X:%.*]] = alloca [[CLASS_X:%.*]], align 1
-// CHECK-NEXT:br i1 [[B:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
-// CHECK:   if.then:
 // CHECK-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[AGG_RESULT:%.*]]) #[[ATTR5]]
-// CHECK-NEXT:br label [[RETURN:%.*]]
-// CHECK:   if.end:
-// CHECK-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[CLASS_X]], %class.X* [[X]], i32 0, i32 0
-// CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR5]]
-// CHECK-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[X]]) #[[ATTR5]]
-// CHECK-NEXT:call void @_ZN1XC1ERKS_(%class.X* noundef nonnull align 1 dereferenceable(1) [[AGG_RESULT]], %class.X* noundef nonnull align 1 dereferenceable(1) [[X]]) #[[ATTR5]]
-// CHECK-NEXT:call void @_ZN1XD1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[X]]) #[[ATTR5]]
-// CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR5]]
-// CHECK-NEXT:br label [[RETURN]]
-// CHECK:   return:
 // CHECK-NEXT:ret void
 //
-// CHECK-EH-03-LABEL: @_Z5test3b(
-// CHECK-EH-03-NEXT:  entry:
-// CHECK-EH-03-NEXT:[[X:%.*]] = alloca [[CLASS_X:%.*]], align 1
-// CHECK-EH-03-NEXT:br i1 [[B:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
-// CHECK-EH-03:   if.then:
-// CHECK-EH-03-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[AGG_RESULT:%.*]])
-// CHECK-EH-03-NEXT:br label [[RETURN:%.*]]
-// CHECK-EH-03:   if.end:
-// CHECK-EH-03-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[CLASS_X]], %class.X* [[X]], i32 0, i32 0
-// CHECK-EH-03-NEXT:call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR7]]
-// CHECK-EH-03-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[X]])
-// CHECK-EH-03-NEXT:invoke void @_ZN1XC1ERKS_(%class.X* noundef nonnull align 1 dereferenceable(1) [[AGG_RESULT]], %class.X* noundef nonnull align 1 dereferenceable(1) [[X]])
-// CHECK-EH-03-NEXT:to label [[INVOKE_CONT:%.*]] unwind label [[LPAD:%.*]]
-// CHECK-EH-03:   invoke.cont:
-// CHECK-EH-03-NEXT:call void @_ZN1XD1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[X]])
-// CHECK-EH-03-NEXT:call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR7]]
-// CHECK-EH-03-NEXT:br label [[RETURN]]
-// CHECK-EH-03:   lpad:
-// CHECK-EH-03-NEXT:[[TMP1:%.*]] = landingpad { i8*, i32 }
-// CHECK-EH-03-NEXT:cleanup
-// CHECK-EH-03-NEXT:invoke void @_ZN1XD1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[X]])
-// CHECK-EH-03-NEXT:to label [[INVOKE_CONT1:%.*]] unwind label [[TERMINATE_LPAD:%.*]]
-// CHECK-EH-03:   invoke.cont1:
-// CHECK-EH-03-NEXT:call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR7]]
-// CHECK-EH-03-NEXT:resume { i8*, i32 } [[TMP1]]
-// CHECK-EH-03:   return:
-// CHECK-EH-03-NEXT:ret void
-// CHECK-EH-03:   terminate.lpad:
-// CHECK-EH-03-NEXT:[[TMP2:%.*]] = landingpad { i8*, i32 }
-// CHECK-EH-03-NEXT:catch i8* null
-// CHECK-EH-03-NEXT:[[TMP3:%.*]] = extractvalue { i8*, i32 } [[TMP2]], 0
-// CHECK-EH-03-NEXT:call void @__clang_call_terminate(i8* [[TMP3]]) #[[ATTR8]]
-// CHECK-EH-03-NEXT:unreachable
-//
-// CHECK-EH-11-LABEL: @_Z5test3b(
-// CHECK-EH-11-NEXT:  entry:
-// CHECK-EH-11-NEXT:[[X:%.*]] = alloca [[CLASS_X:%.*]], align 1
-// CHECK-EH-11-NEXT:br i1 [[B:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
-// CHECK-EH-11:   if.then:
-// CHECK-EH-11-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 dereferenceable(1) [[AGG_RESULT:%.*]])
-// CHECK-EH-11-NEXT:br label [[RETURN:%.*]]
-// CHECK-EH-11:   if.end:
-// CHECK-EH-11-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[CLASS_X]], %class.X* [[X]], i32 0, i32 0
-// CHECK-EH-11-NEXT:call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull [[TMP0]]) #[[ATTR7]]
-// CHECK-EH-11-NEXT:call void @_ZN1XC1Ev(%class.X* noundef nonnull align 1 

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Hyeongyu Kim via Phabricator via cfe-commits]

hyeongyukim added a comment.

In D105169#3319773 , @dblaikie wrote:

> In D105169#3315009 , @MaskRay wrote:
>
>> It may not be worth changing now, but I want to mention: it's more 
>> conventional to have a `BoolOption` which adds `-[no-]noundef-analysis`. 
>> Since both positive and negative forms exist. When we make the default 
>> switch, existing users don't need to change the option. After the option 
>> becomes quite stable and the workaround is deemed not useful, we can remove 
>> the CC1 option.
>
> +1 to this (changing the name and the default at the same time makes 
> migrations a bit more difficult - if the default is changed without renaming 
> (by having both positive and negative flag names) then users can adopt their 
> current default explicitly with no change ahead of picking up the patch that 
> changes the default) & also this flag seems to have no tests? Could you 
> (@hyeongyukim ) add some frontend test coverage for the flag - and yeah, 
> maybe consider giving it a name that has both explicit on/off names, as 
> @maskray suggested? (I think that's useful even after the default switch - 
> since a user might want to override a previous argument on the command line, 
> etc)

Sure, I'll add some tests.
By the way, is it right to change the flag's name to 
`-[no-]enable-noundef-analysis`? or would it be better to use 
`-[no-]noundef-analysis` as @MaskRay suggested?
I prefer to use `-[no-]enable-noundef-analysis` to maintain backward 
compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D115844#3321190 , @pcc wrote:

> On the bug you have:
>
>   define internal fastcc void 
> @_Z4callIiE4taskv.resume(%_Z4callIiE4taskv.Frame* noalias nonnull 
> align 8 dereferenceable(24
>   ) %FramePtr) #1 prologue <{ i32, i32 }> <{ i32 846595819, i32 
> trunc (i64 sub (i64 ptrtoint (i8** @1 to i64), i64 ptrtoint (void ()* 
> @_Z4callIiE4taskv to i64)) to i32) }> {...}
>
> Is it possible for the C/C++ code to take the address of the function 
> `_Z4callIiE4taskv.resume` and call it indirectly?

`*.resume` is a compiler inserted function that is opaque to the programmer. It 
is called indirectly most of the time if not all the time.

> If not, it seems like the right fix would be to arrange for the prologue data 
> to be dropped on the `.resume` function instead of duplicating it there. I 
> would also imagine that whatever signature you have on the `.resume` function 
> would be incorrect since it appears that the coro splitting pass will use a 
> different function signature for that function.

That is addressed by D116130 . @rjmccall 
suggested the direction of this patch (which I agreed) 
https://reviews.llvm.org/D114728#3159303.

> Note that D119296  will have the same 
> problem.

Thanks for the info!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115844

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

[PATCH] D117091: [Clang] Add attributes alloc_size and alloc_align to mm_malloc

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

ping


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

https://reviews.llvm.org/D117091

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

[PATCH] D119613: [OpenMP] Add support for CPU offloading in new driver

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

We should be able to test this with unit tests, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119613

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

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

On the bug you have:

  define internal fastcc void 
@_Z4callIiE4taskv.resume(%_Z4callIiE4taskv.Frame* noalias nonnull align 
8 dereferenceable(24
  ) %FramePtr) #1 prologue <{ i32, i32 }> <{ i32 846595819, i32 
trunc (i64 sub (i64 ptrtoint (i8** @1 to i64), i64 ptrtoint (void ()* 
@_Z4callIiE4taskv to i64)) to i32) }> {...}

Is it possible for the C/C++ code to take the address of the function 
`_Z4callIiE4taskv.resume` and call it indirectly? If not, it seems like the 
right fix would be to arrange for the prologue data to be dropped on the 
`.resume` function instead of duplicating it there. I would also imagine that 
whatever signature you have on the `.resume` function would be incorrect since 
it appears that the coro splitting pass will use a different function signature 
for that function.

Note that D119296  will have the same problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115844

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

[PATCH] D119788: [AArch64] Add support for -march=native for Apple M1 CPU

[Keith Smiley via Phabricator via cfe-commits]

keith added reviewers: beanz, efriedma.
keith added a comment.

Adding folks from https://reviews.llvm.org/D69597


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119788

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

[PATCH] D119788: [AArch64] Add support for -march=native for Apple M1 CPU

[Keith Smiley via Phabricator via cfe-commits]

keith created this revision.
Herald added subscribers: dexonsmith, pengfei, hiraditya, kristof.beyls.
keith requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This improves the getHostCPUName check for Apple M1 
 CPUs, which
previously would always be considered cyclone instead. This also enables
`-march=native` support when building on M1  CPUs 
which would previously
fail. This isn't as sophisticated as the X86 CPU feature checking which
consults the CPU via getHostCPUFeatures, but this is still better than
before. This CPU selection could also be invalid if this was run on an
iOS device instead, ideally we can improve those cases as they come up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119788

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  llvm/lib/Support/Host.cpp


Index: llvm/lib/Support/Host.cpp
===
--- llvm/lib/Support/Host.cpp
+++ llvm/lib/Support/Host.cpp
@@ -1299,11 +1299,7 @@
   bool HaveVectorSupport = CVT[244] & 0x80;
   return getCPUNameFromS390Model(Id, HaveVectorSupport);
 }
-#elif defined(__APPLE__) && defined(__aarch64__)
-StringRef sys::getHostCPUName() {
-  return "cyclone";
-}
-#elif defined(__APPLE__) && defined(__arm__)
+#elif defined(__APPLE__) && (defined(__arm__) || defined(__aarch64__))
 StringRef sys::getHostCPUName() {
   host_basic_info_data_t hostInfo;
   mach_msg_type_number_t infoCount;
@@ -1314,15 +1310,23 @@
 );
   mach_port_deallocate(mach_task_self(), hostPort);
 
-  if (hostInfo.cpu_type != CPU_TYPE_ARM) {
-assert(false && "CPUType not equal to ARM should not be possible on ARM");
-return "generic";
-  }
-  switch (hostInfo.cpu_subtype) {
+  if (hostInfo.cpu_type == CPU_TYPE_ARM) {
+switch (hostInfo.cpu_subtype) {
 case CPU_SUBTYPE_ARM_V7S:
   return "swift";
-default:;
+default:
+  break;
 }
+  } else if (hostInfo.cpu_type == CPU_TYPE_ARM64) {
+switch (hostInfo.cpu_subtype) {
+case CPU_SUBTYPE_ARM64E:
+  return "apple-m1";
+default:
+  return "cyclone";
+}
+  } else {
+assert(false && "CPUType not equal to ARM/ARM64 should not be possible");
+  }
 
   return "generic";
 }
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -151,6 +151,8 @@
   std::pair Split = StringRef(MarchLowerCase).split("+");
 
   llvm::AArch64::ArchKind ArchKind = llvm::AArch64::parseArch(Split.first);
+  if (Split.first == "native")
+ArchKind = 
llvm::AArch64::getCPUArchKind(llvm::sys::getHostCPUName().str());
   if (ArchKind == llvm::AArch64::ArchKind::INVALID ||
   !llvm::AArch64::getArchFeatures(ArchKind, Features))
 return false;


Index: llvm/lib/Support/Host.cpp
===
--- llvm/lib/Support/Host.cpp
+++ llvm/lib/Support/Host.cpp
@@ -1299,11 +1299,7 @@
   bool HaveVectorSupport = CVT[244] & 0x80;
   return getCPUNameFromS390Model(Id, HaveVectorSupport);
 }
-#elif defined(__APPLE__) && defined(__aarch64__)
-StringRef sys::getHostCPUName() {
-  return "cyclone";
-}
-#elif defined(__APPLE__) && defined(__arm__)
+#elif defined(__APPLE__) && (defined(__arm__) || defined(__aarch64__))
 StringRef sys::getHostCPUName() {
   host_basic_info_data_t hostInfo;
   mach_msg_type_number_t infoCount;
@@ -1314,15 +1310,23 @@
 );
   mach_port_deallocate(mach_task_self(), hostPort);
 
-  if (hostInfo.cpu_type != CPU_TYPE_ARM) {
-assert(false && "CPUType not equal to ARM should not be possible on ARM");
-return "generic";
-  }
-  switch (hostInfo.cpu_subtype) {
+  if (hostInfo.cpu_type == CPU_TYPE_ARM) {
+switch (hostInfo.cpu_subtype) {
 case CPU_SUBTYPE_ARM_V7S:
   return "swift";
-default:;
+default:
+  break;
 }
+  } else if (hostInfo.cpu_type == CPU_TYPE_ARM64) {
+switch (hostInfo.cpu_subtype) {
+case CPU_SUBTYPE_ARM64E:
+  return "apple-m1";
+default:
+  return "cyclone";
+}
+  } else {
+assert(false && "CPUType not equal to ARM/ARM64 should not be possible");
+  }
 
   return "generic";
 }
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -151,6 +151,8 @@
   std::pair Split = StringRef(MarchLowerCase).split("+");
 
   llvm::AArch64::ArchKind ArchKind = llvm::AArch64::parseArch(Split.first);
+  if (Split.first == "native")
+ArchKind = llvm::AArch64::getCPUArchKind(llvm::sys::getHostCPUName().str());
   if (ArchKind == llvm::AArch64::ArchKind::INVALID ||
   !llvm::AArch64::getArchFeatures(ArchKind, Features))
 

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua added a comment.

In D119745#3320154 , @ymandel wrote:

> Looks good. But, please add tests. Thanks!

Done; added a test for a multi-AtomicChange edit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119745

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

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 408636.
li.zhe.hua marked 2 inline comments as done.
li.zhe.hua added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119745

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -26,6 +26,7 @@
 using ::clang::transformer::before;
 using ::clang::transformer::cat;
 using ::clang::transformer::changeTo;
+using ::clang::transformer::editList;
 using ::clang::transformer::makeRule;
 using ::clang::transformer::member;
 using ::clang::transformer::name;
@@ -36,6 +37,8 @@
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::UnorderedElementsAre;
 
 constexpr char KHeaderContents[] = R"cc(
   struct string {
@@ -120,10 +123,11 @@
 return *ChangedCode;
   }
 
-  Transformer::ChangeConsumer consumer() {
-return [this](Expected C) {
+  Transformer::ChangeSetConsumer consumer() {
+return [this](Expected> C) {
   if (C) {
-Changes.push_back(std::move(*C));
+Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
+   std::make_move_iterator(C->end()));
   } else {
 // FIXME: stash this error rather then printing.
 llvm::errs() << "Error generating changes: "
@@ -799,7 +803,6 @@
 }
 
 TEST_F(TransformerTest, EditList) {
-  using clang::transformer::editList;
   std::string Input = R"cc(
 void foo() {
   if (10 > 1.0)
@@ -827,7 +830,6 @@
 }
 
 TEST_F(TransformerTest, Flatten) {
-  using clang::transformer::editList;
   std::string Input = R"cc(
 void foo() {
   if (10 > 1.0)
@@ -1638,4 +1640,46 @@
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), format(Header));
 }
+
+// A single change set can span multiple files.
+TEST_F(TransformerTest, MultiFileEdit) {
+  // NB: The fixture is unused for this test, but kept for the test suite name.
+  std::string Header = R"cc(void Func(int id);)cc";
+  std::string Source = R"cc(#include "input.h"
+void Caller() {
+  int id = 0;
+  Func(id);
+})cc";
+  int ErrorCount = 0;
+  std::vector ChangeSets;
+  clang::ast_matchers::MatchFinder MatchFinder;
+  Transformer T(
+  makeRule(callExpr(callee(functionDecl(hasName("Func"))),
+forEachArgumentWithParam(expr().bind("arg"),
+ parmVarDecl().bind("param"))),
+   editList({changeTo(node("arg"), cat("ARG")),
+ changeTo(node("param"), cat("PARAM"))})),
+  [&](Expected> Changes) {
+if (Changes)
+  ChangeSets.push_back(AtomicChanges(Changes->begin(), Changes->end()));
+else
+  ++ErrorCount;
+  });
+  T.registerMatchers();
+  auto Factory = newFrontendActionFactory();
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  Factory->create(), Source, std::vector(), "input.cc",
+  "clang-tool", std::make_shared(),
+  {{"input.h", Header}}));
+
+  EXPECT_EQ(ErrorCount, 0);
+  EXPECT_THAT(
+  ChangeSets,
+  UnorderedElementsAre(UnorderedElementsAre(
+  ResultOf([](const AtomicChange ) { return C.getFilePath(); },
+   "input.cc"),
+  ResultOf([](const AtomicChange ) { return C.getFilePath(); },
+   "./input.h";
+}
+
 } // namespace
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
 }
   }
 
+  llvm::SmallVector Changes;
+  Changes.reserve(ChangesByFileID.size());
   for (auto  : ChangesByFileID)
-Consumer(std::move(IDChangePair.second));
+Changes.push_back(std::move(IDChangePair.second));
+  Consumer(llvm::MutableArrayRef(Changes));
 }
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,44 @@
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  using ChangeConsumer =
-  std::function Change)>;
+  using ChangeConsumer = std::function Change)>;
 
+  /// Provides the set of changes to the consumer.  The callback is free to move
+  /// or 

[PATCH] D119785: [clang-format] Fix formatting of struct-like records followed by variable declaration.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 408629.
curdeius added a comment.

Remove unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119785

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -78,25 +78,38 @@
 TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
   auto Tokens = annotate("class C {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("const class C {} c;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("const class {} c;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
   auto Tokens = annotate("union U {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+
+  Tokens = annotate("union U { void f() { return; } };");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsEnums) {
   auto Tokens = annotate("enum E {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_EnumLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsLBracesInMacroDefinition) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3665,6 +3665,21 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "class {\n"
+   "} anonymous;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "const class {\n"
+   "} anonymous;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "constexpr class C {\n"
+   "} c;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2992,7 +2992,7 @@
   // Just a declaration or something is wrong.
   if (FormatTok->isNot(tok::l_brace))
 return true;
-  FormatTok->setType(TT_RecordLBrace);
+  FormatTok->setType(TT_EnumLBrace);
   FormatTok->setBlockKind(BK_Block);
 
   if (Style.Language == FormatStyle::LK_Java) {
@@ -3229,8 +3229,22 @@
   nextToken();
 }
   }
+
+  auto GetBraceType = [](const FormatToken ) {
+switch (RecordTok.Tok.getKind()) {
+case tok::kw_class:
+  return TT_ClassLBrace;
+case tok::kw_struct:
+  return TT_StructLBrace;
+case tok::kw_union:
+  return TT_UnionLBrace;
+default:
+  // Useful for e.g. interface.
+  return TT_RecordLBrace;
+}
+  };
   if (FormatTok->Tok.is(tok::l_brace)) {
-FormatTok->setType(TT_RecordLBrace);
+FormatTok->setType(GetBraceType(InitialToken));
 if (ParseAsExpr) {
   parseChildBlock();
 } else {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -26,6 +26,11 @@
  NextNext && NextNext->is(tok::l_brace);
 }
 
+bool isRecordLBrace(const FormatToken ) {
+  return Tok.isOneOf(TT_ClassLBrace, TT_EnumLBrace, TT_RecordLBrace,
+ TT_StructLBrace, TT_UnionLBrace);
+}
+
 /// Tracks the indent level of \c AnnotatedLines across levels.
 ///
 /// \c nextLine must be called for each \c AnnotatedLine, after 

[PATCH] D119785: [clang-format] Fix formatting of struct-like records followed by variable declaration.

[Marek Kurdej via Phabricator via cfe-commits]

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

Fixes https://github.com/llvm/llvm-project/issues/24781.
Fixes https://github.com/llvm/llvm-project/issues/38160.

This patch splits TT_RecordLBrace for classes/enums/structs/unions (and other 
records, e.g. interfaces) and uses the brace type to avoid the error-prone 
scanning for record token.

The mentioned bugs were provoked by the scanning being too limited (and so not 
considering `const` or `constexpr`, or other qualifiers, on an anonymous struct 
variable declaration).

Moreover, the proposed solution is more efficient as we parse tokens once only 
(scanning being parsing too).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119785

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -78,25 +78,38 @@
 TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
   auto Tokens = annotate("class C {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("const class C {} c;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("const class {} c;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
   auto Tokens = annotate("union U {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+
+  Tokens = annotate("union U { void f() { return; } };");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsEnums) {
   auto Tokens = annotate("enum E {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_EnumLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsLBracesInMacroDefinition) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3665,6 +3665,21 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "class {\n"
+   "} anonymous;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "const class {\n"
+   "} anonymous;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace some_namespace {\n"
+   "constexpr class C {\n"
+   "} c;\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2992,7 +2992,7 @@
   // Just a declaration or something is wrong.
   if (FormatTok->isNot(tok::l_brace))
 return true;
-  FormatTok->setType(TT_RecordLBrace);
+  FormatTok->setType(TT_EnumLBrace);
   FormatTok->setBlockKind(BK_Block);
 
   if (Style.Language == FormatStyle::LK_Java) {
@@ -3229,8 +3229,22 @@
   nextToken();
 }
   }
+
+  auto GetBraceType = [](const FormatToken ) {
+switch (RecordTok.Tok.getKind()) {
+case tok::kw_class:
+  return TT_ClassLBrace;
+case tok::kw_struct:
+  return TT_StructLBrace;
+case tok::kw_union:
+  return TT_UnionLBrace;
+default:
+  // Useful for e.g. interface.
+  return TT_RecordLBrace;
+}
+  };
   if (FormatTok->Tok.is(tok::l_brace)) {
-FormatTok->setType(TT_RecordLBrace);
+FormatTok->setType(GetBraceType(InitialToken));
 

[PATCH] D118875: [compiler-rt][builtins] build the macOS compiler-rt built-ins with Mac Catalyst support

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 408621.
arphaman added a comment.

Add checking for -darwin-target-variant support


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

https://reviews.llvm.org/D118875

Files:
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake


Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -3,6 +3,7 @@
 # .o files. This is particularly useful in producing larger, more complex
 # runtime libraries.
 
+include(BuiltinTests)
 include(CheckIncludeFile)
 include(CheckCXXSourceCompiles)
 include(GNUInstallDirs)
@@ -138,6 +139,12 @@
 set(OSX_SYSROOT_FLAG "")
   endif()
 
+  try_compile_only(COMPILER_RT_HAS_DARWIN_TARGET_VARIANT_FLAG
+   FLAGS
+   "-target" "x86_64-apple-macos10.15"
+   "-darwin-target-variant" "x86_64-apple-ios13.1-macabi"
+   "-Werror")
+  option(COMPILER_RT_ENABLE_MACCATALYST "Enable building for Mac Catalyst" 
${COMPILER_RT_HAS_DARWIN_TARGET_VARIANT_FLAG})
   option(COMPILER_RT_ENABLE_IOS "Enable building for iOS" On)
   option(COMPILER_RT_ENABLE_WATCHOS "Enable building for watchOS - 
Experimental" Off)
   option(COMPILER_RT_ENABLE_TVOS "Enable building for tvOS - Experimental" Off)
Index: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
@@ -298,6 +298,14 @@
  -target 
"${LIB_ARCH}-apple-${base_os}${DARWIN_${LIBOS}_BUILTIN_MIN_VER}-simulator")
   endif()
 
+  if ("${COMPILER_RT_ENABLE_MACCATALYST}" AND
+  "${LIB_OS}" MATCHES "^osx$")
+# Build the macOS builtins with Mac Catalyst support.
+list(APPEND builtin_cflags
+  -target ${LIB_ARCH}-apple-macos${DARWIN_osx_BUILTIN_MIN_VER}
+  -darwin-target-variant ${LIB_ARCH}-apple-ios13.0-macabi)
+  endif()
+
   set_target_compile_flags(${libname}
 ${sysroot_flag}
 ${DARWIN_${LIB_OS}_BUILTIN_MIN_VER_FLAG}


Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -3,6 +3,7 @@
 # .o files. This is particularly useful in producing larger, more complex
 # runtime libraries.
 
+include(BuiltinTests)
 include(CheckIncludeFile)
 include(CheckCXXSourceCompiles)
 include(GNUInstallDirs)
@@ -138,6 +139,12 @@
 set(OSX_SYSROOT_FLAG "")
   endif()
 
+  try_compile_only(COMPILER_RT_HAS_DARWIN_TARGET_VARIANT_FLAG
+   FLAGS
+   "-target" "x86_64-apple-macos10.15"
+   "-darwin-target-variant" "x86_64-apple-ios13.1-macabi"
+   "-Werror")
+  option(COMPILER_RT_ENABLE_MACCATALYST "Enable building for Mac Catalyst" ${COMPILER_RT_HAS_DARWIN_TARGET_VARIANT_FLAG})
   option(COMPILER_RT_ENABLE_IOS "Enable building for iOS" On)
   option(COMPILER_RT_ENABLE_WATCHOS "Enable building for watchOS - Experimental" Off)
   option(COMPILER_RT_ENABLE_TVOS "Enable building for tvOS - Experimental" Off)
Index: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
@@ -298,6 +298,14 @@
  -target "${LIB_ARCH}-apple-${base_os}${DARWIN_${LIBOS}_BUILTIN_MIN_VER}-simulator")
   endif()
 
+  if ("${COMPILER_RT_ENABLE_MACCATALYST}" AND
+  "${LIB_OS}" MATCHES "^osx$")
+# Build the macOS builtins with Mac Catalyst support.
+list(APPEND builtin_cflags
+  -target ${LIB_ARCH}-apple-macos${DARWIN_osx_BUILTIN_MIN_VER}
+  -darwin-target-variant ${LIB_ARCH}-apple-ios13.0-macabi)
+  endif()
+
   set_target_compile_flags(${libname}
 ${sysroot_flag}
 ${DARWIN_${LIB_OS}_BUILTIN_MIN_VER_FLAG}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: sammccall, rsmith, dblaikie, majnemer, mizvekov, 
ChuanqiXu.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

Emit the note when we fail to deduce a return type, or deduce conflicting 
return types.

It's emitted a //little// too eagerly, e.g. I wish we could avoid it in cases 
like

  $ bin/clang++ x.cpp
  x.cpp:1:19: error: use of undeclared identifier 'x'
  auto f() { return x; }
^
  x.cpp:1:1: note: deducing return type for 'f'
  auto f() { return x; }
  ^~~~
  1 error generated.

But Clang doesn't seem to be able to distinguish "parsing and type-checking the 
return //expression//" from "deducing the function's actual return //type//," 
and I'm not inclined to dig this rabbit hole deeper than I already have. I 
think this is still a reasonably clean place to stop digging.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119778

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Sema/invalid-bitwidth-expr.mm
  clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
  clang/test/SemaCXX/cxx2b-consteval-if.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/std-compare-cxx2a.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp
  clang/test/SemaOpenCLCXX/template-astype.cl
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -114,6 +114,7 @@
   }
   template void g5() {
 ([]() -> C auto{ // expected-error-re {{deduced type {{.*}} does not satisfy}}
+// expected-note@-1 {{deducing return type for 'operator()'}}
  return T();
  }(), ...);
   }
@@ -174,11 +175,17 @@
   template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
 
   C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f1'}}
   C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f2'}}
   C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f3'}}
   C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f4'}}
   C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f5'}}
   C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f6'}}
 
   void g() {
 f1();
Index: clang/test/SemaOpenCLCXX/template-astype.cl
===
--- clang/test/SemaOpenCLCXX/template-astype.cl
+++ clang/test/SemaOpenCLCXX/template-astype.cl
@@ -11,6 +11,7 @@
 
 auto neg_test_int(int x) { return templated_astype(x); }
 // expected-note@-1{{in instantiation of function template specialization 'templated_astype' requested here}}
+// expected-note@-2{{deducing return type for 'neg_test_int'}}
 
 auto test_short4(short4 x) { return templated_astype(x); }
 
Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -2,6 +2,7 @@
 auto check1() {
   return 1;
   return s; // expected-error {{use of undeclared identifier 's'}}
+// expected-note@-3 {{deducing return type for 'check1'}}
 }
 
 int test = 11; // expected-note 2 {{'test' declared here}}
@@ -9,6 +10,7 @@
   return "s";
   return tes; // expected-error {{use of undeclared identifier 'tes'; did you mean 'test'?}}
   // expected-error@-1 {{deduced as 'int' here but deduced as 'const char *' in earlier}}
+  // expected-note@-4 {{deducing return type for 'check2'}}
 }
 
 template  struct is_same { static constexpr bool value = false; };
Index: clang/test/SemaCXX/std-compare-cxx2a.cpp
===
--- clang/test/SemaCXX/std-compare-cxx2a.cpp
+++ clang/test/SemaCXX/std-compare-cxx2a.cpp
@@ -32,6 +32,7 @@
 auto compare_incomplete_test() {
   // 

[PATCH] D118875: [compiler-rt][builtins] build the macOS compiler-rt built-ins with Mac Catalyst support

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D118875#3296030 , @bc-lee wrote:

> Hi, I built and tested clang using this patch and 
> https://reviews.llvm.org/D118862. It works great.
> The current patch adds the -darwin-target-variant flag to all object files 
> for MacOS targets in the compiler_rt's builtin library. However, users 
> building compiler_rt may have compilers that don't support such flag, so it 
> would be nice to have a CMake option to enable compiler flags like 
> `COMPILER_RT_ENABLE_CATALYST`.

Good point. I will update the patch and add detection for the Mac catalyst 
support that detects if clang supports it with a flag .


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

https://reviews.llvm.org/D118875

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

[PATCH] D119184: [clang] [concepts] Check constrained-auto return types for void-returning functions

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 408594.
Quuxplusone marked an inline comment as done and an inline comment as not done.
Quuxplusone added a comment.

Address review comments; add exhaustive tests as a parent revision.


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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,23 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+
+  void g() {
+f1();
+f2();
+f3();
+f4();
+f5();
+f6();
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,8 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT,
+ /*HasReturnStmt=*/true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3761,9 +3762,9 @@
 /// C++1y [dcl.spec.auto]p6.
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
-Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3809,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
+if (!FD->getReturnType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
+  << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+<< OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
@@ -3989,7 +3997,8 @@
   // trying to deduce its return type.
   // (Some return values may be needlessly wrapped in RecoveryExpr).
   if (FD->isInvalidDecl() ||
-  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT,
+   /*HasReturnStmt=*/true)) 

[PATCH] D119726: [asan] Add support for disable_sanitizer_instrumentation attribute

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119726

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

[clang] 688622f - [clang][test] Add -fuse-ld= to test cases added in d238acd1131ec2670acf5cf47b89069ca6c2e86c to resolve test failure with CLANG_DEFAULT_LINKER=lld

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2022-02-14T13:21:39-08:00
New Revision: 688622ff607ca1b15e76f9b4f6216f78dd22fab2

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

LOG: [clang][test] Add -fuse-ld= to test cases added in 
d238acd1131ec2670acf5cf47b89069ca6c2e86c to resolve test failure with 
CLANG_DEFAULT_LINKER=lld

Added: 


Modified: 
clang/test/Driver/darwin-ld-platform-version-target-version.c
clang/test/Driver/darwin-zippered-target-version.c

Removed: 




diff  --git a/clang/test/Driver/darwin-ld-platform-version-target-version.c 
b/clang/test/Driver/darwin-ld-platform-version-target-version.c
index ec9825d21bde0..cc88e015f82c5 100644
--- a/clang/test/Driver/darwin-ld-platform-version-target-version.c
+++ b/clang/test/Driver/darwin-ld-platform-version-target-version.c
@@ -9,9 +9,9 @@
 // RUN:   | FileCheck --check-prefix=ARM64_NEW %s
 // RUN: %clang -target arm64-apple-macos10.15 -darwin-target-variant 
arm64-apple-ios13.1-macabi  -isysroot %S/Inputs/MacOSX10.15.versioned.sdk 
-mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=ARM64_NEW-INV %s
-// RUN: %clang -target arm64-apple-ios13.1-macabi -darwin-target-variant 
arm64-apple-macos10.15 -isysroot %S/Inputs/MacOSX10.15.versioned.sdk 
-mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-ios13.1-macabi -darwin-target-variant 
arm64-apple-macos10.15 -isysroot %S/Inputs/MacOSX10.15.versioned.sdk -fuse-ld= 
-mlinker-version=400 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=ARM64_OLD %s
-// RUN: %clang -target arm64-apple-macos10.15 -darwin-target-variant 
arm64-apple-ios13.1-macabi -isysroot %S/Inputs/MacOSX10.15.versioned.sdk 
-mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-macos10.15 -darwin-target-variant 
arm64-apple-ios13.1-macabi -isysroot %S/Inputs/MacOSX10.15.versioned.sdk 
-fuse-ld= -mlinker-version=400 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=ARM64_OLD-INV %s
 
 // CHECK: "-platform_version" "mac catalyst" "13.1.0" "13.1"

diff  --git a/clang/test/Driver/darwin-zippered-target-version.c 
b/clang/test/Driver/darwin-zippered-target-version.c
index 91449fa2331ee..0f739a44f4d4a 100644
--- a/clang/test/Driver/darwin-zippered-target-version.c
+++ b/clang/test/Driver/darwin-zippered-target-version.c
@@ -1,10 +1,10 @@
 // RUN: %clang -target unknown-apple-macos10.15 -arch x86_64 -arch x86_64h 
-arch i386 \
 // RUN:   -darwin-target-variant x86_64-apple-ios13.1-macabi 
-darwin-target-variant x86_64h-apple-ios13.1-macabi \
-// RUN:   %s -mlinker-version=400 -### 2>&1 | FileCheck %s
+// RUN:   %s -fuse-ld= -mlinker-version=400 -### 2>&1 | FileCheck %s
 
 // RUN: %clang -target unknown-apple-ios13.1-macabi -arch x86_64 -arch x86_64h 
\
 // RUN:   -darwin-target-variant x86_64-apple-macos10.15 \
-// RUN:   %s -mlinker-version=400 -### 2>&1 | FileCheck 
--check-prefix=INVERTED %s
+// RUN:   %s -fuse-ld= -mlinker-version=400 -### 2>&1 | FileCheck 
--check-prefix=INVERTED %s
 
 // CHECK: "-arch" "x86_64" "-macosx_version_min" "10.15.0" 
"-maccatalyst_version_min" "13.1"
 // CHECK: "-arch" "x86_64h" "-macosx_version_min" "10.15.0" 
"-maccatalyst_version_min" "13.1"



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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1764
+def NoInline : DeclOrStmtAttr {
+  let Spellings = [Clang<"noinline">, Declspec<"noinline">];
+  let Documentation = [NoInlineDocs];

What I need to mention here. With Clang<"noinline">, we lost support for 
[[gnu::noinline]] syntax, now unrecognized. Maybe it is ok, as we have now more 
powerful attribute?




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

https://reviews.llvm.org/D119061

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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/test/CodeGen/attr-noinline.cpp:11
+
+void foo(int i) {
+  [[clang::noinline]] bar();

kuhar wrote:
> Could you also add a test function that is already marked as noinline at the 
> declaration?
Yes, added


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

https://reviews.llvm.org/D119061

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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 408588.
xbolva00 added a comment.

More tests


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

https://reviews.llvm.org/D119061

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-noinline.cpp
  clang/test/Sema/attr-noinline.cpp

Index: clang/test/Sema/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-noinline.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int bar();
+
+[[gnu::always_inline]] void always_inline_fn(void) { }
+[[gnu::flatten]] void flatten_fn(void) { }
+
+[[clang::noinline]] void noinline_fn(void) { }
+
+void foo() {
+  [[clang::noinline]] bar();
+  [[clang::noinline(0)]] bar(); // expected-error {{'noinline' attribute takes no arguments}}
+  int x;
+  [[clang::noinline]] x = 0; // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] { asm("nop"); } // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] label: x = 1; // expected-error {{'noinline' attribute only applies to functions and statements}}
+
+
+  [[clang::noinline]] always_inline_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
+  [[clang::noinline]] flatten_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'flatten'}}
+  [[clang::noinline]] noinline_fn();
+
+}
+
+[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}}
Index: clang/test/CodeGen/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noinline.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+void g(bool);
+
+static int baz(int x) {
+return x * 10;
+}
+
+[[clang::noinline]] bool noi() { }
+
+void foo(int i) {
+  [[clang::noinline]] bar();
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR:[0-9]+]]
+  [[clang::noinline]] i = baz(i);
+// CHECK: call noundef i32 @_ZL3bazi({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] (i = 4, bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] (void)(bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] f(bar(), bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) #[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  bar();
+// CHECK: call noundef zeroext i1 @_Z3barv()
+  [[clang::noinline]] noi();
+// CHECK: call noundef zeroext i1 @_Z3noiv()
+  noi();
+// CHECK: call noundef zeroext i1 @_Z3noiv()
+}
+
+struct S {
+  friend bool operator==(const S , const S );
+};
+
+void func(const S , const S ) {
+  [[clang::noinline]]g(s1 == s2);
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+// CHECK: call void @_Z1gb({{.*}}) #[[NOINLINEATTR]]
+  bool b;
+  [[clang::noinline]] b = s1 == s2;
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+}
+
+// CHECK: attributes #[[NOINLINEATTR]] = { noinline }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -175,17 +175,21 @@
 
 namespace {
 class CallExprFinder : public ConstEvaluatedExprVisitor {
-  bool FoundCallExpr = false;
-
+  bool FoundAsmStmt = false;
+  std::vector CallExprs;
 public:
   typedef ConstEvaluatedExprVisitor Inherited;
 
   CallExprFinder(Sema , const Stmt *St) : Inherited(S.Context) { Visit(St); }
 
-  bool foundCallExpr() { return FoundCallExpr; }
+  bool foundCallExpr() { return !CallExprs.empty(); }
+  const std::vector () { return CallExprs; }
+
+  bool foundAsmStmt() { return FoundAsmStmt; }
 
-  void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
-  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
+  void VisitCallExpr(const CallExpr 

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 408587.
li.zhe.hua added a comment.

Update based on comments.

Update test to switch off depreated constructor.
Fix assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119745

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -120,10 +120,11 @@
 return *ChangedCode;
   }
 
-  Transformer::ChangeConsumer consumer() {
-return [this](Expected C) {
+  Transformer::ChangeSetConsumer consumer() {
+return [this](Expected> C) {
   if (C) {
-Changes.push_back(std::move(*C));
+Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
+   std::make_move_iterator(C->end()));
   } else {
 // FIXME: stash this error rather then printing.
 llvm::errs() << "Error generating changes: "
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
 }
   }
 
+  llvm::SmallVector Changes;
+  Changes.reserve(ChangesByFileID.size());
   for (auto  : ChangesByFileID)
-Consumer(std::move(IDChangePair.second));
+Changes.push_back(std::move(IDChangePair.second));
+  Consumer(llvm::MutableArrayRef(Changes));
 }
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,44 @@
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  using ChangeConsumer =
-  std::function Change)>;
+  using ChangeConsumer = std::function Change)>;
 
+  /// Provides the set of changes to the consumer.  The callback is free to 
move
+  /// or destructively consume the changes as needed.
+  ///
+  /// We use \c MutableArrayRef as an abstraction to provide decoupling, and we
+  /// expect the majority of consumers to copy or move the individual values
+  /// into a separate data structure.
+  using ChangeSetConsumer = std::function> Changes)>;
+
+  /// Deprecated. Use the constructor accepting \c ChangesConsumer.
+  ///
   /// \param Consumer Receives each rewrite or error.  Will not necessarily be
   /// called for each match; for example, if the rewrite is not applicable
   /// because of macros, but doesn't fail.  Note that clients are responsible
   /// for handling the case that independent \c AtomicChanges conflict with 
each
   /// other.
   Transformer(transformer::RewriteRule Rule, ChangeConsumer Consumer)
-  : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {}
+  : Transformer(std::move(Rule),
+[Consumer = std::move(Consumer)](
+Expected> Changes) 
{
+  if (Changes)
+for (auto  : *Changes)
+  Consumer(std::move(Change));
+  else
+Consumer(Changes.takeError());
+}) {}
+
+  /// \param Consumer Receives all rewrites for a single match, or an error.
+  /// Will not necessarily be called for each match; for example, if the rule
+  /// generates no edits but does not fail.  Note that clients are responsible
+  /// for handling the case that independent \c AtomicChanges conflict with 
each
+  /// other.
+  Transformer(transformer::RewriteRule Rule, ChangeSetConsumer Consumer)
+  : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {
+assert(this->Consumer && "Consumer is empty");
+  }
 
   /// N.B. Passes `this` pointer to `MatchFinder`.  So, this object should not
   /// be moved after this call.
@@ -43,8 +71,9 @@
 
 private:
   transformer::RewriteRule Rule;
-  /// Receives each successful rewrites as an \c AtomicChange.
-  ChangeConsumer Consumer;
+  /// Receives sets of successful rewrites as an
+  /// \c llvm::ArrayRef.
+  ChangeSetConsumer Consumer;
 };
 } // namespace tooling
 } // namespace clang


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -120,10 +120,11 @@
 return *ChangedCode;
   }
 
-  Transformer::ChangeConsumer consumer() {
-return [this](Expected C) {
+  Transformer::ChangeSetConsumer consumer() {
+return [this](Expected> C) {
   if (C) {
-

[PATCH] D119772: [clang] [NFC] More exhaustive tests for deducing void return types

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: sammccall, rsmith, dblaikie, mizvekov.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

This is a preliminary ahead of D119184  (I'll 
rebase that one on top of this). This shows what Clang's //current// behavior 
is for various deduced-return-type situations. Then, in D119184 
, I'll be able to demonstrate exactly how the 
diagnostics on each of these situations change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119772

Files:
  clang/test/SemaCXX/deduced-return-void.cpp

Index: clang/test/SemaCXX/deduced-return-void.cpp
===
--- clang/test/SemaCXX/deduced-return-void.cpp
+++ clang/test/SemaCXX/deduced-return-void.cpp
@@ -4,20 +4,137 @@
 // Check that we don't get any extra warning for "return" without an
 // expression, in a function that might have been intended to return
 // void all along.
-auto f1() {
-  return 1;
-  return; // expected-error {{deduced as 'void' here but deduced as 'int' in earlier return statement}}
+decltype(h1) h1() { // expected-error {{use of undeclared identifier 'h1'}}
+  return;
+}
+
+namespace JustAuto {
+int i;
+auto f1() { }
+auto f2() { return; }
+auto f3() { return void(); }
+auto f4() {
+  return i;
+  return; // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+}
+auto f5() {
+  return i;
+  return void(); // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
 }
 
-decltype(auto) f2() {
-  return 1;
-  return; // expected-error {{deduced as 'void' here but deduced as 'int' in earlier return statement}}
+auto l1 = []() { };
+auto l2 = []() { return; };
+auto l3 = []() { return void(); };
+auto l4 = []() {
+  return i;
+  return; // expected-error {{return type 'void' must match previous return type 'int' when lambda expression has unspecified explicit return type}}
+};
+auto l5 = []() {
+  return i;
+  return void(); // expected-error {{return type 'void' must match previous return type 'int' when lambda expression has unspecified explicit return type}}
+};
+
+} // namespace JustAuto
+
+namespace DecltypeAuto {
+int i;
+decltype(auto) f1() { }
+decltype(auto) f2() { return; }
+decltype(auto) f3() { return void(); }
+decltype(auto) f4() {
+  return i;
+  return; // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+}
+decltype(auto) f5() {
+  return i;
+  return void(); // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
 }
 
-auto *g() {
+auto l1 = []() -> decltype(auto) { };
+auto l2 = []() -> decltype(auto) { return; };
+auto l3 = []() -> decltype(auto) { return void(); };
+auto l4 = []() -> decltype(auto) {
+  return i;
+  return; // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+};
+auto l5 = []() -> decltype(auto) {
+  return i;
+  return void(); // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+};
+
+} // namespace DecltypeAuto
+
+namespace AutoPtr {
+int i;
+auto *f1() { } // expected-error {{cannot deduce return type 'auto *' for function with no return statements}}
+auto *f2() {
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted return expression}}
+}
+auto *f3() {
+  return void(); // expected-error {{cannot deduce return type 'auto *' from returned value of type 'void'}}
+}
+auto *f4() {
+  return 
   return; // expected-error {{cannot deduce return type 'auto *' from omitted return expression}}
 }
+auto *f5() {
+  return 
+  return void(); // expected-error {{cannot deduce return type 'auto *' from returned value of type 'void'}}
+}
 
-decltype(h1) h1() { // expected-error {{use of undeclared identifier 'h1'}}
-  return;
+auto l1 = []() -> auto* { }; // expected-error {{cannot deduce return type 'auto *' for function with no return statements}}
+auto l2 = []() -> auto* {
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted return expression}}
+};
+auto l3 = []() -> auto* {
+  return void(); // expected-error {{cannot deduce return type 'auto *' from returned value of type 'void'}}
+};
+auto l4 = []() -> auto* {
+  return 
+  return; // expected-error {{cannot deduce return type 'auto *' from omitted return expression}}
+};
+auto l5 = []() -> auto* {
+  return 
+  return void(); // expected-error {{cannot deduce return type 'auto *' from returned value of type 'void'}}
+};
+} // namespace AutoPtr
+
+namespace AutoRef {
+int i;
+auto& f1() { // expected-error {{cannot deduce return type 'auto &' for 

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 408580.

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

https://reviews.llvm.org/D119061

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-noinline.cpp
  clang/test/Sema/attr-noinline.cpp

Index: clang/test/Sema/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-noinline.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int bar();
+
+[[gnu::always_inline]] void always_inline_fn(void) { }
+[[gnu::flatten]] void flatten_fn(void) { }
+
+void foo() {
+  [[clang::noinline]] bar();
+  [[clang::noinline(0)]] bar(); // expected-error {{'noinline' attribute takes no arguments}}
+  int x;
+  [[clang::noinline]] x = 0; // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] { asm("nop"); } // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] label: x = 1; // expected-error {{'noinline' attribute only applies to functions and statements}}
+
+
+  [[clang::noinline]] always_inline_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
+  [[clang::noinline]] flatten_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'flatten'}}
+
+}
+
+[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}}
Index: clang/test/CodeGen/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noinline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+void g(bool);
+
+static int baz(int x) {
+return x * 10;
+}
+
+void foo(int i) {
+  [[clang::noinline]] bar();
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR:[0-9]+]]
+  [[clang::noinline]] i = baz(i);
+// CHECK: call noundef i32 @_ZL3bazi({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] (i = 4, bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] (void)(bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] f(bar(), bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) #[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  bar();
+// CHECK: call noundef zeroext i1 @_Z3barv()
+}
+
+struct S {
+  friend bool operator==(const S , const S );
+};
+
+void func(const S , const S ) {
+  [[clang::noinline]]g(s1 == s2);
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+// CHECK: call void @_Z1gb({{.*}}) #[[NOINLINEATTR]]
+  bool b;
+  [[clang::noinline]] b = s1 == s2;
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+}
+
+// CHECK: attributes #[[NOINLINEATTR]] = { noinline }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -175,17 +175,21 @@
 
 namespace {
 class CallExprFinder : public ConstEvaluatedExprVisitor {
-  bool FoundCallExpr = false;
-
+  bool FoundAsmStmt = false;
+  std::vector CallExprs;
 public:
   typedef ConstEvaluatedExprVisitor Inherited;
 
   CallExprFinder(Sema , const Stmt *St) : Inherited(S.Context) { Visit(St); }
 
-  bool foundCallExpr() { return FoundCallExpr; }
+  bool foundCallExpr() { return !CallExprs.empty(); }
+  const std::vector () { return CallExprs; }
+
+  bool foundAsmStmt() { return FoundAsmStmt; }
 
-  void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
-  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
+  void VisitCallExpr(const CallExpr *E) { CallExprs.push_back(E); }
+
+  void VisitAsmStmt(const AsmStmt *S) { FoundAsmStmt = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
@@ -200,8 +204,8 @@
   NoMergeAttr NMA(S.Context, A);
   CallExprFinder CEF(S, St);
 
-  if (!CEF.foundCallExpr()) {
-S.Diag(St->getBeginLoc(), 

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 408578.

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

https://reviews.llvm.org/D119061

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-noinline.cpp
  clang/test/Sema/attr-noinline.cpp

Index: clang/test/Sema/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-noinline.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int bar();
+
+[[gnu::always_inline]] void always_inline_fn(void) { }
+[[gnu::flatten]] void flatten_fn(void) { }
+
+void foo() {
+  [[clang::noinline]] bar();
+  [[clang::noinline(0)]] bar(); // expected-error {{'noinline' attribute takes no arguments}}
+  int x;
+  [[clang::noinline]] x = 0; // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] { asm("nop"); } // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] label: x = 1; // expected-error {{'noinline' attribute only applies to functions and statements}}
+
+
+  [[clang::noinline]] always_inline_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
+  [[clang::noinline]] flatten_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'flatten'}}
+
+}
+
+[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}}
Index: clang/test/CodeGen/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noinline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+void g(bool);
+
+static int baz(int x) {
+return x * 10;
+}
+
+void foo(int i) {
+  [[clang::noinline]] bar();
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR:[0-9]+]]
+  [[clang::noinline]] i = baz(i);
+// CHECK: call noundef i32 @_ZL3bazi({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] (i = 4, bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] (void)(bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] f(bar(), bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) #[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  bar();
+// CHECK: call noundef zeroext i1 @_Z3barv()
+}
+
+struct S {
+  friend bool operator==(const S , const S );
+};
+
+void func(const S , const S ) {
+  [[clang::noinline]]g(s1 == s2);
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+// CHECK: call void @_Z1gb({{.*}}) #[[NOINLINEATTR]]
+  bool b;
+  [[clang::noinline]] b = s1 == s2;
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+}
+
+// CHECK: attributes #[[NOINLINEATTR]] = { noinline }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -175,17 +175,21 @@
 
 namespace {
 class CallExprFinder : public ConstEvaluatedExprVisitor {
-  bool FoundCallExpr = false;
-
+  bool FoundAsmStmt = false;
+  std::vector CallExprs;
 public:
   typedef ConstEvaluatedExprVisitor Inherited;
 
   CallExprFinder(Sema , const Stmt *St) : Inherited(S.Context) { Visit(St); }
 
-  bool foundCallExpr() { return FoundCallExpr; }
+  bool foundCallExpr() { return !CallExprs.empty(); }
+  const std::vector () { return CallExprs; }
+
+  bool foundAsmStmt() { return FoundAsmStmt; }
 
-  void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
-  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
+  void VisitCallExpr(const CallExpr *E) { CallExprs.push_back(E); }
+
+  void VisitAsmStmt(const AsmStmt *S) { FoundAsmStmt = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
@@ -200,8 +204,8 @@
   NoMergeAttr NMA(S.Context, A);
   CallExprFinder CEF(S, St);
 
-  if (!CEF.foundCallExpr()) {
-S.Diag(St->getBeginLoc(), 

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:522
+  let Content = [{
+This function attribute suppresses the inlining of a function at the call sites
+of the function.

Feel free to suggest better wording :)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4044
+def warn_function_stmt_attribute_precedence : Warning<
+  "statement attribute %0 has higher precedence than function attribute "
+  "'%select{always_inline|flatten}1'">,

Feel free to suggest better wording :)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4045
+  "statement attribute %0 has higher precedence than function attribute "
+  "'%select{always_inline|flatten}1'">,
+  InGroup;

Ideally this would be something like

"statement attribute %0 has higher precedence than function attribute %1"

but...



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:231
+S.Diag(St->getBeginLoc(), 
diag::warn_function_stmt_attribute_precedence)
+  << A << (Decl->hasAttr() ? 0 : 1);
+  }

.. not sure how to get AttributeCommonInfo here so I could use "<< X".



Comment at: clang/test/CodeGen/attr-noinline.cpp:15-16
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the 
anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) 
#[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}

aaron.ballman wrote:
> I'd also like to see the behavior tested with blocks and statement 
> expressions.
> 
> Also, I'd like to see test cases where the call is "hidden" from the user's 
> sight. e.g.,
> ```
> struct S {
>   friend bool operator==(const S , const S );
> };
> 
> void f(bool);
> 
> void func(const S , const S ) {
>   [[clang::noinline]]f(s1 == s2); // Is operator==() then not inlined?
> }
> ```
Added as tests. Yes, not inlined.


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

https://reviews.llvm.org/D119061

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

[PATCH] D119061: [Clang] noinline call site attribute

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added inline comments.



Comment at: clang/test/CodeGen/attr-noinline.cpp:11
+
+void foo(int i) {
+  [[clang::noinline]] bar();

Could you also add a test function that is already marked as noinline at the 
declaration?


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

https://reviews.llvm.org/D119061

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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 408573.
xbolva00 added a comment.

Added check to warn for conflicting attributes.
More docs, tests.


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

https://reviews.llvm.org/D119061

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-noinline.cpp
  clang/test/Sema/attr-noinline.cpp

Index: clang/test/Sema/attr-noinline.cpp
===
--- clang/test/Sema/attr-noinline.cpp
+++ clang/test/Sema/attr-noinline.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int bar();
+
+[[gnu::always_inline]] void always_inline_fn(void) { }
+[[gnu::flatten]] void flatten_fn(void) { }
+
+void foo() {
+  [[clang::noinline]] bar();
+  [[clang::noinline(0)]] bar(); // expected-error {{'noinline' attribute takes no arguments}}
+  int x;
+  [[clang::noinline]] x = 0; // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] { asm("nop"); } // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] label: x = 1; // expected-error {{'noinline' attribute only applies to functions and statements}}
+
+
+  [[clang::noinline]] always_inline_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
+  [[clang::noinline]] flatten_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'flatten'}}
+
+}
+
+[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}}
Index: clang/test/CodeGen/attr-noinline.cpp
===
--- clang/test/CodeGen/attr-noinline.cpp
+++ clang/test/CodeGen/attr-noinline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+void g(bool);
+
+static int baz(int x) {
+return x * 10;
+}
+
+void foo(int i) {
+  [[clang::noinline]] bar();
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR:[0-9]+]]
+  [[clang::noinline]] i = baz(i);
+// CHECK: call noundef i32 @_ZL3bazi({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] (i = 4, bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] (void)(bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] f(bar(), bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) #[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  bar();
+// CHECK: call noundef zeroext i1 @_Z3barv()
+}
+
+struct S {
+  friend bool operator==(const S , const S );
+};
+
+void func(const S , const S ) {
+  [[clang::noinline]]g(s1 == s2);
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+// CHECK: call void @_Z1gb({{.*}}) #[[NOINLINEATTR]]
+  bool b;
+  [[clang::noinline]] b = s1 == s2;
+// CHECK: call noundef zeroext i1 @_ZeqRK1SS1_({{.*}}) #[[NOINLINEATTR]]
+}
+
+// CHECK: attributes #[[NOINLINEATTR]] = { noinline }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -175,17 +175,21 @@
 
 namespace {
 class CallExprFinder : public ConstEvaluatedExprVisitor {
-  bool FoundCallExpr = false;
-
+  bool FoundAsmStmt = false;
+  std::vector CallExprs;
 public:
   typedef ConstEvaluatedExprVisitor Inherited;
 
   CallExprFinder(Sema , const Stmt *St) : Inherited(S.Context) { Visit(St); }
 
-  bool foundCallExpr() { return FoundCallExpr; }
+  bool foundCallExpr() { return !CallExprs.empty(); }
+  const std::vector () { return CallExprs; }
+
+  bool foundAsmStmt() { return FoundAsmStmt; }
 
-  void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
-  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
+  void VisitCallExpr(const CallExpr *E) { CallExprs.push_back(E); }
+
+  void VisitAsmStmt(const AsmStmt *S) { FoundAsmStmt = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
@@ 

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Fixes for inliners landed, working on this now.


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

https://reviews.llvm.org/D119061

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

[PATCH] D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands

[Alex Lorenz 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 rGd238acd1131e: [clang][driver] add clang driver support for 
emitting macho files with two… (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D118862?vs=405495=408563#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118862

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-ld-platform-version-target-version.c
  clang/test/Driver/darwin-objc-runtime-maccatalyst-target-variant.m
  clang/test/Driver/darwin-target-variant-sdk-version.c
  clang/test/Driver/darwin-target-variant.c
  clang/test/Driver/darwin-zippered-target-version.c

Index: clang/test/Driver/darwin-zippered-target-version.c
===
--- /dev/null
+++ clang/test/Driver/darwin-zippered-target-version.c
@@ -0,0 +1,16 @@
+// RUN: %clang -target unknown-apple-macos10.15 -arch x86_64 -arch x86_64h -arch i386 \
+// RUN:   -darwin-target-variant x86_64-apple-ios13.1-macabi -darwin-target-variant x86_64h-apple-ios13.1-macabi \
+// RUN:   %s -mlinker-version=400 -### 2>&1 | FileCheck %s
+
+// RUN: %clang -target unknown-apple-ios13.1-macabi -arch x86_64 -arch x86_64h \
+// RUN:   -darwin-target-variant x86_64-apple-macos10.15 \
+// RUN:   %s -mlinker-version=400 -### 2>&1 | FileCheck --check-prefix=INVERTED %s
+
+// CHECK: "-arch" "x86_64" "-macosx_version_min" "10.15.0" "-maccatalyst_version_min" "13.1"
+// CHECK: "-arch" "x86_64h" "-macosx_version_min" "10.15.0" "-maccatalyst_version_min" "13.1"
+// CHECK: "-arch" "i386" "-macosx_version_min" "10.15.0"
+// CHECK-NOT: maccatalyst_version_min
+
+// INVERTED: "-arch" "x86_64" "-maccatalyst_version_min" "13.1.0" "-macosx_version_min" "10.15"
+// INVERTED: "-arch" "x86_64h" "-maccatalyst_version_min" "13.1.0"
+// INVERTED-NOT: macosx_version_min
Index: clang/test/Driver/darwin-target-variant.c
===
--- /dev/null
+++ clang/test/Driver/darwin-target-variant.c
@@ -0,0 +1,32 @@
+// RUN: %clang -target unknown-apple-macos10.15 -arch x86_64 -arch x86_64h -arch i386 \
+// RUN:   -darwin-target-variant x86_64-apple-ios13.1-macabi -darwin-target-variant x86_64h-apple-ios13.1-macabi \
+// RUN:   -c %s -### 2>&1 | FileCheck %s
+
+// RUN: %clang -target x86_64-apple-macos10.15 -darwin-target-variant i386-apple-ios13.1-macabi \
+// RUN:   -c %s -### 2>&1 | FileCheck --check-prefix=UNUSED-TV %s
+
+// RUN: %clang -target x86_64-apple-macos10.15 -darwin-target-variant x86_64-apple-ios13.1-macabi \
+// RUN:   -darwin-target-variant x86_64-apple-ios13.1-macabi -c %s -### 2>&1 | FileCheck --check-prefix=REDUNDANT-TV %s
+
+// RUN: %clang -target x86_64-apple-macos10.15 -darwin-target-variant x86_64-apple-ios13.1 \
+// RUN:   -c %s -### 2>&1 | FileCheck --check-prefix=INCORRECT-TV %s
+
+// RUN: %clang -target unknown-apple-ios13.1-macabi -arch x86_64 -arch x86_64h \
+// RUN:   -darwin-target-variant x86_64-apple-macos10.15 \
+// RUN:   -c %s -### 2>&1 | FileCheck --check-prefix=INVERTED %s
+
+// CHECK: "-triple" "x86_64-apple-macosx10.15.0"
+// CHECK-SAME: "-darwin-target-variant-triple" "x86_64-apple-ios13.1-macabi"
+// CHECK: "-triple" "x86_64h-apple-macosx10.15.0"
+// CHECK-SAME: "-darwin-target-variant-triple" "x86_64h-apple-ios13.1-macabi"
+// CHECK: "-triple" "i386-apple-macosx10.15.0"
+// CHECK-NOT: target-variant-triple
+
+// INVERTED: "-triple" "x86_64-apple-ios13.1.0-macabi"
+// INVERTED-SAME: "-darwin-target-variant-triple" "x86_64-apple-macos10.15"
+// INVERTED: "-triple" "x86_64h-apple-ios13.1.0-macabi"
+// INVERTED-NOT: target-variant-triple
+
+// UNUSED-TV: argument unused during compilation: '-darwin-target-variant i386-apple-ios13.1-macabi'
+// REDUNDANT-TV: argument unused during compilation: '-darwin-target-variant x86_64-apple-ios13.1-macabi'
+// INCORRECT-TV: unsupported '-darwin-target-variant' value 'x86_64-apple-ios13.1'; use 'ios-macabi' instead
Index: clang/test/Driver/darwin-target-variant-sdk-version.c
===
--- /dev/null
+++ clang/test/Driver/darwin-target-variant-sdk-version.c
@@ -0,0 +1,12 @@
+// RUN: %clang -target x86_64-apple-macosx10.15 -darwin-target-variant x86_64-apple-ios13.1-macabi -isysroot %S/Inputs/MacOSX10.15.versioned.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+// RUN: env SDKROOT=%S/Inputs/MacOSX10.15.versioned.sdk %clang -target x86_64-apple-macosx10.15 -darwin-target-variant x86_64-apple-ios13.1-macabi -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+// RUN: %clang -target x86_64-apple-ios13.1-macabi -darwin-target-variant x86_64-apple-macosx10.15 -isysroot 

[clang] d238acd - [clang][driver] add clang driver support for emitting macho files with two build version load commands

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2022-02-14T12:27:14-08:00
New Revision: d238acd1131ec2670acf5cf47b89069ca6c2e86c

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

LOG: [clang][driver] add clang driver support for emitting macho files with two 
build version load commands

This patch extends clang driver to pass the right flags to the clang frontend, 
and ld64,
so that they can emit macho files with two build version load commands. It adds 
a new
0darwin-target-variant option which complements -target and also can be used to 
specify different
target variants when multi-arch compilations are invoked with multiple -arch 
commands.

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

Added: 
clang/test/Driver/darwin-ld-platform-version-target-version.c
clang/test/Driver/darwin-objc-runtime-maccatalyst-target-variant.m
clang/test/Driver/darwin-target-variant-sdk-version.c
clang/test/Driver/darwin-target-variant.c
clang/test/Driver/darwin-zippered-target-version.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Driver/ToolChains/Darwin.h

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 427cb788c3a4c..b688c121b1c07 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -637,4 +637,8 @@ def err_drv_cuda_offload_only_emit_bc : Error<
 def warn_drv_jmc_requires_debuginfo : Warning<
   "/JMC requires debug info. Use '/Zi', '/Z7' or other debug options; option 
ignored">,
   InGroup;
+
+def err_drv_target_variant_invalid : Error<
+  "unsupported '%0' value '%1'; use 'ios-macabi' instead">;
+
 }

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index cd0d56cecaca1..b81973155cae6 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3994,6 +3994,9 @@ def : Separate<["--"], "no-system-header-prefix">, 
Alias, Group;
 def target : Joined<["--"], "target=">, Flags<[NoXarchOption, CoreOption]>,
   HelpText<"Generate code for the given target">;
+def darwin_target_variant : Separate<["-"], "darwin-target-variant">,
+  Flags<[NoXarchOption, CoreOption]>,
+  HelpText<"Generate code for an additional runtime variant of the deployment 
target">;
 def print_supported_cpus : Flag<["-", "--"], "print-supported-cpus">,
   Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Print supported cpu models for the given target (if target is not 
specified,"

diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index df860ccc6be4f..3c408010b5b19 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1496,6 +1496,10 @@ struct DarwinPlatform {
   /// Returns true if the simulator environment can be inferred from the arch.
   bool canInferSimulatorFromArch() const { return InferSimulatorFromArch; }
 
+  const Optional () const {
+return TargetVariantTriple;
+  }
+
   /// Adds the -m-version-min argument to the compiler invocation.
   void addOSVersionMinArgument(DerivedArgList , const OptTable ) {
 if (Argument)
@@ -1558,6 +1562,16 @@ struct DarwinPlatform {
   }
 }
   }
+  // In a zippered build, we could be building for a macOS target that's
+  // lower than the version that's implied by the OS version. In that case
+  // we need to use the minimum version as the native target version.
+  if (TargetVariantTriple) {
+auto TargetVariantVersion = TargetVariantTriple->getOSVersion();
+if (TargetVariantVersion.getMajor()) {
+  if (TargetVariantVersion < NativeTargetVersion)
+NativeTargetVersion = TargetVariantVersion;
+}
+  }
   break;
 }
 default:
@@ -1567,12 +1581,14 @@ struct DarwinPlatform {
 
   static DarwinPlatform
   createFromTarget(const llvm::Triple , StringRef OSVersion, Arg *A,
+   Optional TargetVariantTriple,
const Optional ) {
 DarwinPlatform Result(TargetArg, getPlatformFromOS(TT.getOS()), OSVersion,
   A);
 VersionTuple OsVersion = TT.getOSVersion();
 if (OsVersion.getMajor() == 0)
   Result.HasOSVersion = false;
+Result.TargetVariantTriple = TargetVariantTriple;
 Result.setEnvironment(TT.getEnvironment(), OsVersion, SDKInfo);
 return Result;
   }
@@ -1656,6 +1672,7 @@ struct DarwinPlatform {
   bool HasOSVersion = true, InferSimulatorFromArch = true;
   Arg *Argument;
   StringRef EnvVarName;
+  Optional TargetVariantTriple;
 };
 
 /// Returns the 

[PATCH] D119711: Add asan support for MSVC debug runtimes

[Curtis J Bezault via Phabricator via cfe-commits]

cbezault added a comment.

Imo I agree that this shouldn’t be merged until the debug variants of the asan 
runtime are getting built publicly. @stevewishnousky


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119711

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

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a subscriber: aaron.ballman.
xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:186
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 

pengfei wrote:
> xbolva00 wrote:
> > This is totally wrong, just big hack to smuggle it here.
> Could you explain more? Is there any unexpect sideeffect by this?
It looks unfortunate to have something like AsmStmt in "CallExprFinder" with 
CallExpr as reference to clang's CallExpr.

Kinda surprised that your list of reviewers missed ALL known clang 
developers/code owner, in this case especially @aaron.ballman .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84225

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

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D118855#3320064 , @Bigcheese wrote:

> lgtm.

Thanks for the review, Michael! I'll give some time to the pre-merge checks to 
run, then will land the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

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

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 408536.
vsapsai added a comment.

Update assertion message wording to be more actionable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/Decl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -773,7 +773,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
-  ED->setCompleteDefinition(false);
+  ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
   if (OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
@@ -828,7 +828,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
-  RD->setCompleteDefinition(false);
+  RD->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, RD);
 } else {
   OldDef = RD;
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4301,6 +4301,7 @@
   setEmbeddedInDeclarator(false);
   setFreeStanding(false);
   setCompleteDefinitionRequired(false);
+  TagDeclBits.IsThisDeclarationADemotedDefinition = false;
 }
 
 SourceLocation TagDecl::getOuterLocStart() const {
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1443,10 +1443,14 @@
 /// Has the full definition of this type been required by a use somewhere 
in
 /// the TU.
 uint64_t IsCompleteDefinitionRequired : 1;
+
+/// Whether this tag is a definition which was demoted due to
+/// a module merge.
+uint64_t IsThisDeclarationADemotedDefinition : 1;
   };
 
   /// Number of non-inherited bits in TagDeclBitfields.
-  enum { NumTagDeclBits = 9 };
+  enum { NumTagDeclBits = 10 };
 
   /// Stores the bits used by EnumDecl.
   /// If modified NumEnumDeclBit and the accessor
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3486,6 +3486,24 @@
   /// parameters.
   bool isDependentType() const { return isDependentContext(); }
 
+  /// Whether this declaration was a definition in some module but was forced
+  /// to be a declaration.
+  ///
+  /// Useful for clients checking if a module has a definition of a specific
+  /// symbol and not interested in the final AST with deduplicated definitions.
+  bool isThisDeclarationADemotedDefinition() const {
+return TagDeclBits.IsThisDeclarationADemotedDefinition;
+  }
+
+  /// Mark a definition as a declaration and maintain information it _was_
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() &&
+   "Should demote definitions only, not forward declarations");
+setCompleteDefinition(false);
+TagDeclBits.IsThisDeclarationADemotedDefinition = true;
+  }
+
   /// Starts the definition of this tag declaration.
   ///
   /// This method should be invoked at the beginning of the definition


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -773,7 +773,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
-  ED->setCompleteDefinition(false);
+  ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
   if (OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
@@ -828,7 +828,7 @@
 }
 if (OldDef) {
   Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
-  RD->setCompleteDefinition(false);
+  RD->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, RD);
 } else {
   OldDef = RD;
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4301,6 +4301,7 @@
   setEmbeddedInDeclarator(false);
   setFreeStanding(false);
   setCompleteDefinitionRequired(false);
+  TagDeclBits.IsThisDeclarationADemotedDefinition = false;
 }
 
 SourceLocation TagDecl::getOuterLocStart() const {
Index: clang/include/clang/AST/DeclBase.h
===
--- 

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review, Dan!




Comment at: clang/include/clang/AST/Decl.h:1383-1398
   /// If this definition should pretend to be a declaration.
   bool isThisDeclarationADemotedDefinition() const {
 return isa(this) ? false :
   NonParmVarDeclBits.IsThisDeclarationADemotedDefinition;
   }
 
   /// This is a definition which should be demoted to a declaration.

Naming for my change is mimicking this API.



Comment at: clang/include/clang/AST/Decl.h:3494
+  /// symbol and not interested in the final AST with deduplicated definitions.
+  bool isThisDeclarationADemotedDefinition() const {
+return TagDeclBits.IsThisDeclarationADemotedDefinition;

delcypher wrote:
> This name feels a little clunky. How about `isDemotedDefinition()`?
I agree the name is awkward but I am using the same name as `NonParmVarDecl` 
does. Based on similarities in implementation I think the similarities in the 
name are useful. Also we have `isThisDeclarationADefinition` in multiple 
places, so it is good to follow the same naming pattern, even if it is clunky.



Comment at: clang/include/clang/AST/Decl.h:3500
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() && "Not a definition!");

delcypher wrote:
> delcypher wrote:
> > Given the name of this function (suggesting it always `demotes`) it 
> > probably should 
> > 
> > ```
> > assert(!isThisDeclarationADemotedDefinition() && "decl already demoted");
> > ```
> > 
> > alternatively comments saying the operation is idempotent is probably fine 
> > too.
> The `demoteThisDefinitionToDeclaration()` name feels a little bit clunky. How 
> about `demoteDefinitionToDecl()`?
Interesting suggestion, made me think about what exactly I'm trying to do here. 
The strict requirement is not to set the flag on forward declarations (aka 
non-definitions). To achieve that the alternative would be
```lang=c++
if (!isCompleteDefinition())
  return;
setCompleteDefinition(false);
TagDeclBits.IsThisDeclarationADemotedDefinition = true;
```

Which is similar to the current code with the assertion. Demoting already 
demoted definition again is acceptable but there is really no need for that.

I agree the assertion makes the code less robust but its purpose is to enforce 
expected API usage, so being picky about the way it is called is intended. 
Though let me change the assertion message to something more actionable rather 
than state-of-the-world-observation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

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

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Looks good. But, please add tests. Thanks!




Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:27
 
+  using ChangesConsumer = std::function> Changes)>;

Maybe `ChangeSetConsumer` to be more clearly a different word than 
`ChangeConsumer`?



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:28
+  using ChangesConsumer = std::function> Changes)>;
+

Maybe document the choice of MutableArrayRef vs passing by value? I think this 
is the right choice, but it's not totally obvious. My justification is that we 
assume most consumers are reading the data and not storing the whole array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119745

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

[PATCH] D119612: [clang] Pass more flags to ld64.lld

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/darwin-ld-dedup.c:4
+// -no_deduplicate is only present from ld64 version 262 and later, or lld.
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=261 -O0 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s

If you are going to change the command line, prefer the canonical `--target=` 
to the legacy `-target `.


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

https://reviews.llvm.org/D119612

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

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I've made the requested changes from @jyknight in 
a766545402d8569532294d097d026cf2e837b5c2 
 -- please 
let me know if there are more issues you'd like me to address. Thank you for 
the post-commit feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Dan Liew via Phabricator via cfe-commits]

delcypher added a comment.

Change seems reasonable but I don't have expertise on this code. I've left a 
few minor nits.




Comment at: clang/include/clang/AST/Decl.h:3494
+  /// symbol and not interested in the final AST with deduplicated definitions.
+  bool isThisDeclarationADemotedDefinition() const {
+return TagDeclBits.IsThisDeclarationADemotedDefinition;

This name feels a little clunky. How about `isDemotedDefinition()`?



Comment at: clang/include/clang/AST/Decl.h:3500
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() && "Not a definition!");

Given the name of this function (suggesting it always `demotes`) it probably 
should 

```
assert(!isThisDeclarationADemotedDefinition() && "decl already demoted");
```

alternatively comments saying the operation is idempotent is probably fine too.



Comment at: clang/include/clang/AST/Decl.h:3500
+  /// a definition.
+  void demoteThisDefinitionToDeclaration() {
+assert(isCompleteDefinition() && "Not a definition!");

delcypher wrote:
> Given the name of this function (suggesting it always `demotes`) it probably 
> should 
> 
> ```
> assert(!isThisDeclarationADemotedDefinition() && "decl already demoted");
> ```
> 
> alternatively comments saying the operation is idempotent is probably fine 
> too.
The `demoteThisDefinitionToDeclaration()` name feels a little bit clunky. How 
about `demoteDefinitionToDecl()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

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

[PATCH] D119612: [clang] Pass more flags to ld64.lld

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:272
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above and lld run the deduplicate pass by default.
+  if ((Version >= VersionTuple(262) || LinkerIsLLD) &&

lld runs `--icf=none` (i.e. `-no_deduplicate`) by default.


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

https://reviews.llvm.org/D119612

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

[clang] a766545 - Update the diagnostic behavior of [[noreturn]] in C2x

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-02-14T14:04:32-05:00
New Revision: a766545402d8569532294d097d026cf2e837b5c2

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

LOG: Update the diagnostic behavior of [[noreturn]] in C2x

Post-commit review feedback suggested dropping the deprecated
diagnostic for the 'noreturn' macro (the diagnostic from the header
file suffices and the macro diagnostic could be confusing) and to only
issue the deprecated diagnostic for [[_Noreturn]] when the attribute
identifier is either directly written or not from a system macro.

Amends the commit made in 5029dce492b3cf3ac191eda0b5bf268c3acac2e0.

Added: 


Modified: 
clang/lib/Headers/stdnoreturn.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/c2x-noreturn.c

Removed: 




diff  --git a/clang/lib/Headers/stdnoreturn.h b/clang/lib/Headers/stdnoreturn.h
index 92fd4a98a87bf..944e6904c7df6 100644
--- a/clang/lib/Headers/stdnoreturn.h
+++ b/clang/lib/Headers/stdnoreturn.h
@@ -15,11 +15,13 @@
 
 #if __STDC_VERSION__ > 201710L &&  
\
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
-/* The noreturn macro is deprecated in C2x. */
-#pragma clang deprecated(noreturn)
-
-/* Including the header file in C2x is also deprecated. */
-#warning "the '' header is deprecated in C2x"
+/* The noreturn macro is deprecated in C2x. We do not mark it as such because
+   including the header file in C2x is also deprecated and we do not want to
+   issue a confusing diagnostic for code which includes 
+   followed by code that writes [[noreturn]]. The issue with such code is not
+   with the attribute, or the use of 'noreturn', but the inclusion of the
+   header. */
+#warning "the '' header is deprecated in C2x; either use the 
'_Noreturn' keyword or the '[[noreturn]]' attribute"
 #endif
 
 #endif /* __STDNORETURN_H */

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 6d0c8f0974767..3034abdf04028 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2177,9 +2177,14 @@ static void handleNoReturnAttr(Sema , Decl *D, const 
ParsedAttr ) {
 
 static void handleStandardNoReturnAttr(Sema , Decl *D, const ParsedAttr ) {
   // The [[_Noreturn]] spelling is deprecated in C2x, so if that was used,
-  // issue an appropriate diagnostic.
+  // issue an appropriate diagnostic. However, don't issue a diagnostic if the
+  // attribute name comes from a macro expansion. We don't want to punish users
+  // who write [[noreturn]] after including  (where 'noreturn'
+  // is defined as a macro which expands to '_Noreturn').
   if (!S.getLangOpts().CPlusPlus &&
-  A.getSemanticSpelling() == CXX11NoReturnAttr::C2x_Noreturn)
+  A.getSemanticSpelling() == CXX11NoReturnAttr::C2x_Noreturn &&
+  !(A.getLoc().isMacroID() &&
+S.getSourceManager().isInSystemMacro(A.getLoc(
 S.Diag(A.getLoc(), diag::warn_deprecated_noreturn_spelling) << 
A.getRange();
 
   D->addAttr(::new (S.Context) CXX11NoReturnAttr(S.Context, A));

diff  --git a/clang/test/Sema/c2x-noreturn.c b/clang/test/Sema/c2x-noreturn.c
index e522a43cf6eba..6c119736f6454 100644
--- a/clang/test/Sema/c2x-noreturn.c
+++ b/clang/test/Sema/c2x-noreturn.c
@@ -36,29 +36,30 @@ _Noreturn void func1(void); // ok, using the function 
specifier
 [[_Noreturn]] void func3(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}}
 
 // Test the behavior of including 
-#include  // c2x-warning@stdnoreturn.h:* {{the 
'' header is deprecated in C2x}}
+#include  // c2x-warning@stdnoreturn.h:* {{the 
'' header is deprecated in C2x; either use the '_Noreturn' 
keyword or the '[[noreturn]]' attribute}}
 
-[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
-   // c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
-   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}
+[[noreturn]] void func6(void);
 
-void func7 [[noreturn]] (void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
-// c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
-// c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}
+void func7 [[noreturn]] (void);
 
-noreturn void func8(void); // c2x-warning {{macro 'noreturn' has been marked 
as deprecated}} \
-   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}
+noreturn void func8(void);
 
-// Ensure the function specifier form still works
-void 

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118855

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

[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D119525#3317623 , @tbaeder wrote:

> In D119525#3314383 , @aaron.ballman 
> wrote:
>
>> Can you also add a release note for the change?
>
> Would this go in the "libclang" section in `clang/docs/ReleaseNotes.rst`? Or 
> somewhere else?

Nope, I'd add a new section titled "Bug Fixes" after "Major New Features" and 
then add it there.


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

https://reviews.llvm.org/D119525

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

[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
li.zhe.hua requested review of this revision.
Herald added a project: clang.

Previously, Transformer would invoke the consumer once per file modified per
match, in addition to any errors encountered. The consumer is not aware of which
AtomicChanges come from any particular match. It is unclear which sets of edits
may be related or whether an error invalidates any previously emitted changes.

Modify the signature of the consumer to accept a set of changes. This keeps
related changes (i.e. all edits from a single match) together, and clarifies
that errors don't produce partial changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119745

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/Transformer.cpp


Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
 }
   }
 
+  llvm::SmallVector Changes;
+  Changes.reserve(ChangesByFileID.size());
   for (auto  : ChangesByFileID)
-Consumer(std::move(IDChangePair.second));
+Changes.push_back(std::move(IDChangePair.second));
+  Consumer(llvm::MutableArrayRef(Changes));
 }
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,38 @@
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  using ChangeConsumer =
-  std::function Change)>;
+  using ChangeConsumer = std::function Change)>;
 
+  using ChangesConsumer = std::function> Changes)>;
+
+  /// Deprecated. Use the constructor accepting \c ChangesConsumer.
+  ///
   /// \param Consumer Receives each rewrite or error.  Will not necessarily be
   /// called for each match; for example, if the rewrite is not applicable
   /// because of macros, but doesn't fail.  Note that clients are responsible
   /// for handling the case that independent \c AtomicChanges conflict with 
each
   /// other.
   Transformer(transformer::RewriteRule Rule, ChangeConsumer Consumer)
-  : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {}
+  : Transformer(std::move(Rule),
+[Consumer = std::move(Consumer)](
+Expected> Changes) 
{
+  if (Changes)
+for (auto  : *Changes)
+  Consumer(std::move(Change));
+  else
+Consumer(Changes.takeError());
+}) {}
+
+  /// \param Consumer Receives all rewrites for a single match, or an error.
+  /// Will not necessarily be called for each match; for example, if the rule
+  /// generates no edits but does not fail.  Note that clients are responsible
+  /// for handling the case that independent \c AtomicChanges conflict with 
each
+  /// other.
+  Transformer(transformer::RewriteRule Rule, ChangesConsumer Consumer)
+  : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {
+assert(Consumer && "Consumer is empty");
+  }
 
   /// N.B. Passes `this` pointer to `MatchFinder`.  So, this object should not
   /// be moved after this call.
@@ -43,8 +65,9 @@
 
 private:
   transformer::RewriteRule Rule;
-  /// Receives each successful rewrites as an \c AtomicChange.
-  ChangeConsumer Consumer;
+  /// Receives sets of successful rewrites as an
+  /// \c llvm::ArrayRef.
+  ChangesConsumer Consumer;
 };
 } // namespace tooling
 } // namespace clang


Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
 }
   }
 
+  llvm::SmallVector Changes;
+  Changes.reserve(ChangesByFileID.size());
   for (auto  : ChangesByFileID)
-Consumer(std::move(IDChangePair.second));
+Changes.push_back(std::move(IDChangePair.second));
+  Consumer(llvm::MutableArrayRef(Changes));
 }
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,38 @@
 /// defined by the arguments of the constructor.
 class Transformer : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  using ChangeConsumer =
-  std::function Change)>;
+  using ChangeConsumer = std::function Change)>;
 
+  using ChangesConsumer = std::function> Changes)>;
+
+  /// Deprecated. Use the constructor accepting \c ChangesConsumer.
+  ///
   /// \param Consumer Receives each rewrite 

[PATCH] D119528: [Clang][Sema] Add a missing regression test about Wliteral-range

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for the additional test coverage! Do you need me to commit on 
your behalf? If so, what name and email address would you like me to use for 
patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119528

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

[PATCH] D119095: [clang] Fix redundant functional cast in ConstantExpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I haven't had the chance to review this yet (in C standards meetings again this 
week), but I did notice that https://reviews.llvm.org/D119477 seems to be 
touching on a related (or possibly the same) issue. Can you coordinate with the 
other patch author to make sure nobody is writing patches (or reviewing them) 
that conflict? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119095

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

[PATCH] D119477: Ignore FullExpr when traversing cast sub-expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I haven't had the chance to review this yet (in C standards meetings again this 
week), but I did notice that https://reviews.llvm.org/D119095 seems to be 
touching on a related (or possibly the same) issue. Can you coordinate with the 
other patch author to make sure nobody is writing patches (or reviewing them) 
that conflict? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119477

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

[PATCH] D78762: build: use `find_package(Python3)` if available

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.
Herald added a project: clang-tools-extra.

What's the new cmake flag to set the python executable, and should we update 
llvm/docs/GettingStarted.rst to not refer to PYTHON_EXECUTABLE anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78762

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

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 408498.
vtjnash added a comment.

fixup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119591

Files:
  clang/CMakeLists.txt
  clang/examples/AnnotateFunctions/CMakeLists.txt
  clang/examples/Attribute/CMakeLists.txt
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/PluginsOrder/CMakeLists.txt
  clang/examples/PrintFunctionNames/CMakeLists.txt
  clang/lib/Analysis/plugins/CMakeLists.txt
  clang/test/CMakeLists.txt

Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -145,8 +145,8 @@
   endif()
 endif()
 
-if (CLANG_ENABLE_STATIC_ANALYZER)
-  if (CLANG_PLUGIN_SUPPORT)
+if(CLANG_ENABLE_STATIC_ANALYZER)
+  if(CLANG_PLUGIN_SUPPORT AND LLVM_ENABLE_PLUGINS) # Determine if we built them
 list(APPEND CLANG_TEST_DEPS
   SampleAnalyzerPlugin
   CheckerDependencyHandlingAnalyzerPlugin
Index: clang/lib/Analysis/plugins/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CMakeLists.txt
@@ -1,4 +1,7 @@
-if(CLANG_ENABLE_STATIC_ANALYZER AND CLANG_PLUGIN_SUPPORT)
+# Since these do not specify a specific PLUGIN_TOOL (which could be clang or
+# clang-tidy), we cannot compile this unless the platform supports plugins with
+# undefined symbols, and cannot use it unless the user has opted for clang plugins).
+if(CLANG_ENABLE_STATIC_ANALYZER AND CLANG_PLUGIN_SUPPORT AND LLVM_ENABLE_PLUGINS)
   add_subdirectory(SampleAnalyzer)
   add_subdirectory(CheckerDependencyHandling)
   add_subdirectory(CheckerOptionHandling)
Index: clang/examples/PrintFunctionNames/CMakeLists.txt
===
--- clang/examples/PrintFunctionNames/CMakeLists.txt
+++ clang/examples/PrintFunctionNames/CMakeLists.txt
@@ -11,7 +11,7 @@
 
 add_llvm_library(PrintFunctionNames MODULE PrintFunctionNames.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/PluginsOrder/CMakeLists.txt
===
--- clang/examples/PluginsOrder/CMakeLists.txt
+++ clang/examples/PluginsOrder/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(PluginsOrder MODULE PluginsOrder.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/CallSuperAttribute/CMakeLists.txt
===
--- clang/examples/CallSuperAttribute/CMakeLists.txt
+++ clang/examples/CallSuperAttribute/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(CallSuperAttr MODULE CallSuperAttrInfo.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/CMakeLists.txt
===
--- clang/examples/CMakeLists.txt
+++ clang/examples/CMakeLists.txt
@@ -3,8 +3,10 @@
   set(EXCLUDE_FROM_ALL ON)
 endif()
 
-add_subdirectory(PrintFunctionNames)
-add_subdirectory(AnnotateFunctions)
-add_subdirectory(Attribute)
-add_subdirectory(CallSuperAttribute)
-add_subdirectory(PluginsOrder)
+if(CLANG_PLUGIN_SUPPORT)
+  add_subdirectory(PrintFunctionNames)
+  add_subdirectory(AnnotateFunctions)
+  add_subdirectory(Attribute)
+  add_subdirectory(CallSuperAttribute)
+  add_subdirectory(PluginsOrder)
+endif()
Index: clang/examples/Attribute/CMakeLists.txt
===
--- clang/examples/Attribute/CMakeLists.txt
+++ clang/examples/Attribute/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(Attribute MODULE Attribute.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   target_link_libraries(Attribute PRIVATE
 clangAST
 clangBasic
Index: clang/examples/AnnotateFunctions/CMakeLists.txt
===
--- clang/examples/AnnotateFunctions/CMakeLists.txt
+++ clang/examples/AnnotateFunctions/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(AnnotateFunctions MODULE AnnotateFunctions.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -476,9 +476,14 @@
 option(CLANG_BUILD_TOOLS
   "Build the Clang tools. If OFF, just generate build targets." ON)
 
+if(LLVM_ENABLE_PLUGINS OR LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)
+  set(HAVE_CLANG_PLUGIN_SUPPORT ON)

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Headers/stdnoreturn.h:19
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+

aaron.ballman wrote:
> jyknight wrote:
> > Is this actually useful? Isn't it sufficient to have the include-time 
> > warning for this header?
> I think it gives a more precise diagnostic than the header inclusion one, and 
> it will diagnose closer to where the use happens instead of at the include. 
> Might not be the most useful, but it I think there's some utility.
Okay, I've convinced myself to remove this a well for the same reason we don't 
want to warn on `[[noreturn]]` which expands to `[[_Noreturn]]` -- the issue 
isn't with use of the attribute, it's with use of the header file.  I don't 
want to risk a user writing `[[noreturn]]` and hearing that the `noreturn` 
macro is deprecated and they think that means that `[[noreturn]]` is deprecated.



Comment at: clang/lib/Headers/stdnoreturn.h:22
+/* Including the header file in C2x is also deprecated. */
+#warning "the '' header is deprecated in C2x"
+#endif

jyknight wrote:
> aaron.ballman wrote:
> > jyknight wrote:
> > > Would be nice to mention the solution, as well. E.g.
> > > 
> > > "The '' header has been deprecated in C2x: including it no 
> > > longer necessary in order to use 'noreturn'. "
> > I can add that, but it is necessary in order to use 'noreturn' as a 
> > function specifier, so that wording isn't quite right. e.g., `noreturn void 
> > func(void);` is valid C today if you `#include `
> Ah, I missed that subtlety. Maybe say:
> 
> "The '' header has been deprecated in C2x; either use the 
> _Noreturn keyword or the [[noreturn]] attribute."
> 
Sure, I can go with that.



Comment at: clang/test/Sema/c2x-noreturn.c:41-43
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+   // c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}

jyknight wrote:
> aaron.ballman wrote:
> > jyknight wrote:
> > > If you've written [[noreturn]] in your code, you're doing the right thing 
> > > already. Why bother emitting a warning? The problem that should be 
> > > corrected is at the point of inclusion of stdnoreturn.h, not the spelling 
> > > here.
> > > 
> > > I'd suggest only warning about "[[_Noreturn]]" if the user actually 
> > > //wrote// it like that, and not when it's expanded from the "noreturn" 
> > > macro.
> > > I'd suggest only warning about "[[_Noreturn]]" if the user actually wrote 
> > > it like that, and not when it's expanded from the "noreturn" macro.
> > 
> > I'm not keen on that design. The goal of this diagnostic is to let people 
> > know that they have a surprise in their code (that this macro is expanding 
> > to something they may not expect) at the location the surprise happens. 
> > Consider:
> > 
> > ```
> > // TU.c
> > #include 
> > #include "my_header.h"
> > 
> > // my_header.h
> > [[noreturn]] void func(void);
> > ```
> > my_header.h doesn't see the inclusion of stdnoreturn.h, so finding out 
> > about the macro may inform the developer to be more protective of using the 
> > attribute.
> But that's exactly my point. There _is_ no surprise in this code. It works 
> exactly as expected. 
> The my_header.h code is using the new non-deprecated spelling, and is getting 
> the correct and desired behavior -- even though someone else has included 
> stdnoreturn.h. That it's working properly due to there existing a 
> compatibility attribute "_Noreturn" is basically an irrelevant detail to any 
> user.
> 
> Emitting any warnings for this line of code seems potentially even harmful. 
> Consider: what should my_header.h be doing differently due to the warning? 
> Nothing. Ideally, they should change nothing. Yet, if we do emit warnings for 
> this usage, it'll likely cause some people to try to add push/undef/pop gunk 
> to their headers to avoid the warning, which makes the code worse than if 
> they did nothing.
> 
Okay, thank you for sticking with me, that logic makes sense to me. I'll remove 
the diagnostic in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

We are not going to start exporting plugin support if the user explicitly 
disabled it with CLANG_PLUGIN_SUPPORT=OFF, but why is it trying to read that 
directory at all, when it should be disallowed by this PR unless 
CLANG_PLUGIN_SUPPORT=ON


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119591

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

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 408491.
vtjnash added a comment.

cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119591

Files:
  clang/CMakeLists.txt
  clang/examples/AnnotateFunctions/CMakeLists.txt
  clang/examples/Attribute/CMakeLists.txt
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/PluginsOrder/CMakeLists.txt
  clang/examples/PrintFunctionNames/CMakeLists.txt
  clang/lib/Analysis/plugins/CMakeLists.txt
  clang/test/CMakeLists.txt

Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -145,8 +145,8 @@
   endif()
 endif()
 
-if (CLANG_ENABLE_STATIC_ANALYZER)
-  if (CLANG_PLUGIN_SUPPORT)
+if(CLANG_ENABLE_STATIC_ANALYZER)
+  if(CLANG_PLUGIN_SUPPORT AND LLVM_ENABLE_PLUGINS) # Determine if we built them
 list(APPEND CLANG_TEST_DEPS
   SampleAnalyzerPlugin
   CheckerDependencyHandlingAnalyzerPlugin
Index: clang/lib/Analysis/plugins/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CMakeLists.txt
@@ -1,4 +1,7 @@
-if(CLANG_ENABLE_STATIC_ANALYZER AND CLANG_PLUGIN_SUPPORT)
+# Since these do not specify a specific PLUGIN_TOOL (which could be clang or
+# clang-tidy), we cannot compile this unless the platform supports plugins with
+# undefined symbols, and cannot use it unless the user has opted for clang plugins).
+if(CLANG_ENABLE_STATIC_ANALYZER AND CLANG_PLUGIN_SUPPORT AND LLVM_ENABLE_PLUGINS)
   add_subdirectory(SampleAnalyzer)
   add_subdirectory(CheckerDependencyHandling)
   add_subdirectory(CheckerOptionHandling)
Index: clang/examples/PrintFunctionNames/CMakeLists.txt
===
--- clang/examples/PrintFunctionNames/CMakeLists.txt
+++ clang/examples/PrintFunctionNames/CMakeLists.txt
@@ -11,7 +11,7 @@
 
 add_llvm_library(PrintFunctionNames MODULE PrintFunctionNames.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/PluginsOrder/CMakeLists.txt
===
--- clang/examples/PluginsOrder/CMakeLists.txt
+++ clang/examples/PluginsOrder/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(PluginsOrder MODULE PluginsOrder.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/CallSuperAttribute/CMakeLists.txt
===
--- clang/examples/CallSuperAttribute/CMakeLists.txt
+++ clang/examples/CallSuperAttribute/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(CallSuperAttr MODULE CallSuperAttrInfo.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/examples/CMakeLists.txt
===
--- clang/examples/CMakeLists.txt
+++ clang/examples/CMakeLists.txt
@@ -3,8 +3,10 @@
   set(EXCLUDE_FROM_ALL ON)
 endif()
 
-add_subdirectory(PrintFunctionNames)
-add_subdirectory(AnnotateFunctions)
-add_subdirectory(Attribute)
-add_subdirectory(CallSuperAttribute)
-add_subdirectory(PluginsOrder)
+if(CLANG_PLUGIN_SUPPORT)
+  add_subdirectory(PrintFunctionNames)
+  add_subdirectory(AnnotateFunctions)
+  add_subdirectory(Attribute)
+  add_subdirectory(CallSuperAttribute)
+  add_subdirectory(PluginsOrder)
+endif()
Index: clang/examples/Attribute/CMakeLists.txt
===
--- clang/examples/Attribute/CMakeLists.txt
+++ clang/examples/Attribute/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(Attribute MODULE Attribute.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   target_link_libraries(Attribute PRIVATE
 clangAST
 clangBasic
Index: clang/examples/AnnotateFunctions/CMakeLists.txt
===
--- clang/examples/AnnotateFunctions/CMakeLists.txt
+++ clang/examples/AnnotateFunctions/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_library(AnnotateFunctions MODULE AnnotateFunctions.cpp PLUGIN_TOOL clang)
 
-if(CLANG_PLUGIN_SUPPORT AND (WIN32 OR CYGWIN))
+if(WIN32 OR CYGWIN)
   set(LLVM_LINK_COMPONENTS
 Support
   )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -476,9 +476,10 @@
 option(CLANG_BUILD_TOOLS
   "Build the Clang tools. If OFF, just generate build targets." ON)
 
+set(HAVE_CLANG_PLUGIN_SUPPORT LLVM_ENABLE_PLUGINS OR LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)
 

[PATCH] D119711: Add asan support for MSVC debug runtimes

[Reid Kleckner via Phabricator via cfe-commits]

rnk added subscribers: cbezault, vitalybuka.
rnk added a comment.

I'm hesitant to do this. The main reason we disabled the use of the debug 
runtimes was that the debug runtimes interfered with malloc interception, which 
leads to crashes and a generally poor user experience. I'd like some 
confirmation beyond just your word that this is addressed.

After that, this is kind of scary because we don't build and ship these "dbg" 
variants of the ASan runtime (so far as I know). They are only provided as part 
of the MSVC CRT. Is LLVM ASan actually compatible with the ASan runtimes 
provided by MSVC? I'm guessing the answer that things appear to work, but so 
far as I know, there is no usptream support for this. If things break, there's 
nobody who will fix them. I have reached out to Microsoft to ask them to help 
contribute upstream, but so far I haven't seen major contributions.

This change is one small step towards encouraging users to combine clang-cl 
with the Microsoft provided ASan runtimes, and I'm not sure we want to guide 
users in that direction. We only promise interop with compiler runtime 
libraries that we vend.

Other stakeholders: +@cbezault @vitalybuka




Comment at: clang/lib/Driver/SanitizerArgs.cpp:829
-  << lastArgumentForMask(D, Args, SanitizerKind::Address);
-  D.Diag(clang::diag::note_drv_address_sanitizer_debug_runtime);
-}

This is the only usage of this note, and if you remove it, the note should be 
removed as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119711

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

[PATCH] D117989: [RISCV] Add the passthru operand for RVV nomask binary intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:181
+  // The nomask intrinsic IR have the passthru operand.
+  bit HasNoMaskPolicy = false;
+

Should this be `HasNoMaskPassThru` rather than `Policy`?



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4733
 } else {
-  Vec = DAG.getNode(RISCVISD::VSLIDE1DOWN_VL, DL, I32VT, Vec, ScalarLo,
-I32Mask, I32VL);
-  Vec = DAG.getNode(RISCVISD::VSLIDE1DOWN_VL, DL, I32VT, Vec, ScalarHi,
-I32Mask, I32VL);
+  // TODO Those VSLDIE1 could be TAMA because we use vmerge to select 
maskedoff
+  SDValue Undef = DAG.getUNDEF(I32VT);

VSLDIE1 -> VSLIDE1

Fix the clang-format warning



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4760
+if (MaskedOff.isUndef()) {
+  assert(Policy == (RISCVII::TAIL_AGNOSTIC | RISCVII::MASK_AGNOSTIC) &&
+ "Invalid policy, undef maskedoff need to have tail and mask "

I don't think you can assert the policy here. The mask can be folded to undef 
without the user knowing.



Comment at: llvm/test/CodeGen/RISCV/rvv/masked-vslide1down-rv32.ll:56
+
+; Fallback vslide1 to mask undisturbed untill InsertVSETVLI supports mask 
agnostic.
+define  
@intrinsic_vslide1down_mask_tuma_vx_nxv1i64_nxv1i64_i64( %0, 
 %1, i64 %2,  %3, i32 %4) nounwind {

untill -> until



Comment at: llvm/test/CodeGen/RISCV/rvv/masked-vslide1down-rv32.ll:78
+
+; Fallback vslide1 to mask undisturbed untill InsertVSETVLI supports mask 
agnostic.
+define  
@intrinsic_vslide1down_mask_tama_vx_nxv1i64_nxv1i64_i64( %0, 
i64 %1,  %2, i32 %3) nounwind {

untill -> until


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117989

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

[PATCH] D119095: [clang] Fix redundant functional cast in ConstantExpr

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think this makes sense to me, I'd like to see it captured in an AST test 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119095

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

[PATCH] D110641: Implement P0857R0 -Part B: requires clause for template-template params

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D110641#3319787 , @cor3ntin wrote:

> In D110641#3064200 , @erichkeane 
> wrote:
>
>> Ping!  Does anyone have any feedback on this?  I'd really like to start 
>> getting us caught up on standards implementation, and this seems like it 
>> might be an easy win (unless someone on the list knows why it is just plain 
>> wrong/more complicated than I think!).
>
> Have we figured out what the intended semantic purposes of that requires 
> clause is supposed to be yet? Because I'm still very confused

Nope, I have yet to be able to come up with a reproducer that would actually 
care about this in any of the compilers :/


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

https://reviews.llvm.org/D110641

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

[PATCH] D110641: Implement P0857R0 -Part B: requires clause for template-template params

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D110641#3064200 , @erichkeane 
wrote:

> Ping!  Does anyone have any feedback on this?  I'd really like to start 
> getting us caught up on standards implementation, and this seems like it 
> might be an easy win (unless someone on the list knows why it is just plain 
> wrong/more complicated than I think!).

Have we figured out what the intended semantic purposes of that requires clause 
is supposed to be yet? Because I'm still very confused


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

https://reviews.llvm.org/D110641

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D105169#3315009 , @MaskRay wrote:

> It may not be worth changing now, but I want to mention: it's more 
> conventional to have a `BoolOption` which adds `-[no-]noundef-analysis`. 
> Since both positive and negative forms exist. When we make the default 
> switch, existing users don't need to change the option. After the option 
> becomes quite stable and the workaround is deemed not useful, we can remove 
> the CC1 option.

+1 to this (changing the name and the default at the same time makes migrations 
a bit more difficult - if the default is changed without renaming (by having 
both positive and negative flag names) then users can adopt their current 
default explicitly with no change ahead of picking up the patch that changes 
the default) & also this flag seems to have no tests? Could you (@hyeongyukim ) 
add some frontend test coverage for the flag - and yeah, maybe consider giving 
it a name that has both explicit on/off names, as @maskray suggested? (I think 
that's useful even after the default switch - since a user might want to 
override a previous argument on the command line, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

> I like the idea of the assert, can we do it as a part of this?  That way when 
> we run into it it becomes more obvious/linked to this discussion.

I think that's a good idea too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119651

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

[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/Headers/stdnoreturn.h:22
+/* Including the header file in C2x is also deprecated. */
+#warning "the '' header is deprecated in C2x"
+#endif

aaron.ballman wrote:
> jyknight wrote:
> > Would be nice to mention the solution, as well. E.g.
> > 
> > "The '' header has been deprecated in C2x: including it no 
> > longer necessary in order to use 'noreturn'. "
> I can add that, but it is necessary in order to use 'noreturn' as a function 
> specifier, so that wording isn't quite right. e.g., `noreturn void 
> func(void);` is valid C today if you `#include `
Ah, I missed that subtlety. Maybe say:

"The '' header has been deprecated in C2x; either use the 
_Noreturn keyword or the [[noreturn]] attribute."




Comment at: clang/test/Sema/c2x-noreturn.c:41-43
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute 
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+   // c2x-warning {{macro 'noreturn' has been 
marked as deprecated}} \
+   // c2x-note@stdnoreturn.h:* {{macro marked 
'deprecated' here}}

aaron.ballman wrote:
> jyknight wrote:
> > If you've written [[noreturn]] in your code, you're doing the right thing 
> > already. Why bother emitting a warning? The problem that should be 
> > corrected is at the point of inclusion of stdnoreturn.h, not the spelling 
> > here.
> > 
> > I'd suggest only warning about "[[_Noreturn]]" if the user actually 
> > //wrote// it like that, and not when it's expanded from the "noreturn" 
> > macro.
> > I'd suggest only warning about "[[_Noreturn]]" if the user actually wrote 
> > it like that, and not when it's expanded from the "noreturn" macro.
> 
> I'm not keen on that design. The goal of this diagnostic is to let people 
> know that they have a surprise in their code (that this macro is expanding to 
> something they may not expect) at the location the surprise happens. Consider:
> 
> ```
> // TU.c
> #include 
> #include "my_header.h"
> 
> // my_header.h
> [[noreturn]] void func(void);
> ```
> my_header.h doesn't see the inclusion of stdnoreturn.h, so finding out about 
> the macro may inform the developer to be more protective of using the 
> attribute.
But that's exactly my point. There _is_ no surprise in this code. It works 
exactly as expected. 
The my_header.h code is using the new non-deprecated spelling, and is getting 
the correct and desired behavior -- even though someone else has included 
stdnoreturn.h. That it's working properly due to there existing a compatibility 
attribute "_Noreturn" is basically an irrelevant detail to any user.

Emitting any warnings for this line of code seems potentially even harmful. 
Consider: what should my_header.h be doing differently due to the warning? 
Nothing. Ideally, they should change nothing. Yet, if we do emit warnings for 
this usage, it'll likely cause some people to try to add push/undef/pop gunk to 
their headers to avoid the warning, which makes the code worse than if they did 
nothing.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707

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

[PATCH] D113738: [LTO] Allow passing -Os/-Oz as the optimization level

[Fangrui Song via Phabricator via cfe-commits]

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

D72404  has some comments why the -Os/-Oz 
levels are discouraged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113738

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

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

[Balázs Benics via Phabricator via cfe-commits]

steakhal reopened this revision.
steakhal added a subscriber: simoll.
steakhal added a comment.
This revision is now accepted and ready to land.

It seems like the `clang-ve-ninja` doesn't really want to accept any patches 
from me :D
I hope it's not personal. Let's be friends bot, please.

I'm inviting @simoll for resolving this, and the underlying issue to prevent 
future breakages and reverts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118987

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

[clang] b8ae323 - Revert "[analyzer] Add failing test case demonstrating buggy taint propagation"

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-02-14T18:45:46+01:00
New Revision: b8ae323cca61dc1edcd36e9ae18c7e4c3d76d52e

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

LOG: Revert "[analyzer] Add failing test case demonstrating buggy taint 
propagation"

This reverts commit 744745ae195f0997e5bfd5aa2de47b9ea156b6a6.

I'm reverting this since this patch caused a build breakage.

https://lab.llvm.org/buildbot/#/builders/91/builds/3818

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Removed: 
clang/test/Analysis/taint-checker-callback-order-has-definition.c
clang/test/Analysis/taint-checker-callback-order-without-definition.c



diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 428778e6cfaa6..e2209e3debfde 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -32,8 +32,6 @@
 #include 
 #include 
 
-#define DEBUG_TYPE "taint-checker"
-
 using namespace clang;
 using namespace ento;
 using namespace taint;
@@ -693,13 +691,6 @@ void GenericTaintChecker::checkPostCall(const CallEvent 
,
   if (TaintArgs.isEmpty())
 return;
 
-  LLVM_DEBUG(for (ArgIdxTy I
-  : TaintArgs) {
-llvm::dbgs() << "PostCall<";
-Call.dump(llvm::dbgs());
-llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
-  });
-
   for (ArgIdxTy ArgNum : TaintArgs) {
 // Special handling for the tainted return value.
 if (ArgNum == ReturnValueIndex) {
@@ -777,25 +768,15 @@ void GenericTaintRule::process(const GenericTaintChecker 
,
 
   /// Propagate taint where it is necessary.
   ForEachCallArg(
-  [this, , WouldEscape, ](ArgIdxTy I, const Expr *E, SVal V) {
-if (PropDstArgs.contains(I)) {
-  LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs());
- llvm::dbgs()
- << "> prepares tainting arg index: " << I << '\n';);
+  [this, , WouldEscape](ArgIdxTy I, const Expr *E, SVal V) {
+if (PropDstArgs.contains(I))
   State = State->add(I);
-}
 
 // TODO: We should traverse all reachable memory regions via the
 // escaping parameter. Instead of doing that we simply mark only the
 // referred memory region as tainted.
-if (WouldEscape(V, E->getType())) {
-  LLVM_DEBUG(if (!State->contains(I)) {
-llvm::dbgs() << "PreCall<";
-Call.dump(llvm::dbgs());
-llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';
-  });
+if (WouldEscape(V, E->getType()))
   State = State->add(I);
-}
   });
 
   C.addTransition(State);

diff  --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c 
b/clang/test/Analysis/taint-checker-callback-order-has-definition.c
deleted file mode 100644
index 295f95c2bf452..0
--- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c
+++ /dev/null
@@ -1,42 +0,0 @@
-// RUN: %clang_analyze_cc1 %s \
-// RUN:   -analyzer-checker=core,alpha.security.taint \
-// RUN:   -mllvm -debug-only=taint-checker \
-// RUN:   2>&1 | FileCheck %s
-
-// FIXME: We should not crash.
-// XFAIL: *
-
-struct _IO_FILE;
-typedef struct _IO_FILE FILE;
-FILE *fopen(const char *fname, const char *mode);
-
-void nested_call(void) {}
-
-char *fgets(char *s, int n, FILE *fp) {
-  nested_call();   // no-crash: we should not try adding taint to a 
non-existent argument.
-  return (char *)0;
-}
-
-void top(const char *fname, char *buf) {
-  FILE *fp = fopen(fname, "r");
-  // CHECK:  PreCall prepares tainting arg index: -1
-  // CHECK-NEXT: PostCall actually wants to taint arg 
index: -1
-
-  if (!fp)
-return;
-
-  (void)fgets(buf, 42, fp); // Trigger taint propagation.
-  // CHECK-NEXT: PreCall prepares tainting arg index: -1
-  // CHECK-NEXT: PreCall prepares tainting arg index: 0
-  // CHECK-NEXT: PreCall prepares tainting arg index: 1
-  // CHECK-NEXT: PreCall prepares tainting arg index: 2
-
-  // FIXME: We should propagate taint from PreCall -> PostCall.
-  // CHECK-NEXT: PostCall actually wants to taint arg index: -1
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 0
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 1
-  // CHECK-NEXT: PostCall actually wants to taint arg index: 2
-
-  // FIXME: We should not crash.
-  // CHECK: PLEASE submit a bug report
-}

diff  --git 
a/clang/test/Analysis/taint-checker-callback-order-without-definition.c 
b/clang/test/Analysis/taint-checker-callback-order-without-definition.c
deleted file mode 100644
index 962e8b259298e..0
--- 

[clang] d16c5f4 - Revert "[analyzer] Fix taint propagation by remembering to the location context"

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-02-14T18:45:46+01:00
New Revision: d16c5f4192c30d53468a472c6820163a81192825

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

LOG: Revert "[analyzer] Fix taint propagation by remembering to the location 
context"

This reverts commit b099e1e562555fbc67e2e0abbc15074e14a85ff1.

I'm reverting this since the head of the patch stack caused a build
breakage.

https://lab.llvm.org/buildbot/#/builders/91/builds/3818

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/taint-checker-callback-order-has-definition.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 66143f78932c..428778e6cfaa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -38,8 +38,6 @@ using namespace clang;
 using namespace ento;
 using namespace taint;
 
-using llvm::ImmutableSet;
-
 namespace {
 
 class GenericTaintChecker;
@@ -436,9 +434,7 @@ template <> struct 
ScalarEnumerationTraits {
 /// to the call post-visit. The values are signed integers, which are either
 /// ReturnValueIndex, or indexes of the pointer/reference argument, which
 /// points to data, which should be tainted on return.
-REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *,
-   ImmutableSet)
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy)
+REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, ArgIdxTy)
 
 void GenericTaintRuleParser::validateArgVector(const std::string ,
const ArgVecTy ) const {
@@ -689,26 +685,22 @@ void GenericTaintChecker::checkPostCall(const CallEvent 
,
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
   ProgramStateRef State = C.getState();
-  const StackFrameContext *CurrentFrame = C.getStackFrame();
 
   // Depending on what was tainted at pre-visit, we determined a set of
   // arguments which should be tainted after the function returns. These are
   // stored in the state as TaintArgsOnPostVisit set.
-  TaintArgsOnPostVisitTy TaintArgsMap = State->get();
-
-  const ImmutableSet *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
-  if (!TaintArgs)
+  TaintArgsOnPostVisitTy TaintArgs = State->get();
+  if (TaintArgs.isEmpty())
 return;
-  assert(!TaintArgs->isEmpty());
 
   LLVM_DEBUG(for (ArgIdxTy I
-  : *TaintArgs) {
+  : TaintArgs) {
 llvm::dbgs() << "PostCall<";
 Call.dump(llvm::dbgs());
 llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
   });
 
-  for (ArgIdxTy ArgNum : *TaintArgs) {
+  for (ArgIdxTy ArgNum : TaintArgs) {
 // Special handling for the tainted return value.
 if (ArgNum == ReturnValueIndex) {
   State = addTaint(State, Call.getReturnValue());
@@ -722,7 +714,7 @@ void GenericTaintChecker::checkPostCall(const CallEvent 
,
   }
 
   // Clear up the taint info from the state.
-  State = State->remove(CurrentFrame);
+  State = State->remove();
   C.addTransition(State);
 }
 
@@ -784,33 +776,28 @@ void GenericTaintRule::process(const GenericTaintChecker 
,
   };
 
   /// Propagate taint where it is necessary.
-  auto  = State->getStateManager().get_context();
-  ImmutableSet Result = F.getEmptySet();
   ForEachCallArg(
-  [this, WouldEscape, , , ](ArgIdxTy I, const Expr *E,
-  SVal V) {
+  [this, , WouldEscape, ](ArgIdxTy I, const Expr *E, SVal V) {
 if (PropDstArgs.contains(I)) {
   LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs());
  llvm::dbgs()
  << "> prepares tainting arg index: " << I << '\n';);
-  Result = F.add(Result, I);
+  State = State->add(I);
 }
 
 // TODO: We should traverse all reachable memory regions via the
 // escaping parameter. Instead of doing that we simply mark only the
 // referred memory region as tainted.
 if (WouldEscape(V, E->getType())) {
-  LLVM_DEBUG(if (!Result.contains(I)) {
+  LLVM_DEBUG(if (!State->contains(I)) {
 llvm::dbgs() << "PreCall<";
 Call.dump(llvm::dbgs());
 llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';
   });
-  Result = F.add(Result, I);
+  State = State->add(I);
 }
   });
 
-  if (!Result.isEmpty())
-State = State->set(C.getStackFrame(), Result);
   C.addTransition(State);
 }
 
@@ -901,11 +888,7 @@ void GenericTaintChecker::taintUnsafeSocketProtocol(const