[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

[Nikita Popov via Phabricator via cfe-commits]

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

LGTM. You probably want to add these intrinsics to the auto-upgrade code in 
`BitcodeReader::propagateAttributeTypes()` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120527

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

[PATCH] D121269: [clang-format] Fix namespace format when the name is followed by a macro

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM % nits. Thanks for working on this!




Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:54-55
+Tok = processTokens(Tok, tok::l_paren, tok::r_paren, nullptr);
+  } else if (Tok->is(tok::l_square))
+Tok = processTokens(Tok, tok::l_square, tok::r_square, nullptr);
+  return Tok;

Nit: add braces around `else if` block to match the `if`.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:77
   } else {
 // Skip attributes.
+Tok = skipAttribute(Tok);

Comment not necessary anymore. Please remove.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:130
   Tok = Tok->getNextNonComment();
+  // Skip attribute
+  const FormatToken *TokAfterAttr = skipAttribute(Tok);

Comment not necessary anymore. Please remove.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:138
+if (!FirstNSName.empty() && !HasColoncolon)
+  name = FirstNSName + (!name.empty() ? " " + name : "");
   }




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121269

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

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM. Thanks a lot!




Comment at: clang/lib/Format/Format.cpp:1181-1184
+  LLVMStyle.AlignConsecutiveAssignments = {
+  /*Enabled=*/false, /*AcrossEmptyLines=*/false,
+  /*AcrossComments=*/false, /*AlignCompound=*/false,
+  /*PadOperators=*/true};

That should be more future-proof.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

Hmmm... if this should be done within compiler side, I am wondering whether 
this should be resolved in AsmPrinter/DwarfDebug instead.
@aprantl please let me know wyt?

However, it looks like that after this hack the `name` here isn't the 
appropriate one... and it looks like something that debuggers should know how 
to workout ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121100

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

[PATCH] D121286: [clangd] Test against path insensitivity

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 414301.
kadircet added a comment.

Add tests specifically and postpone change in testRoot


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121286

Files:
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp


Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -12,7 +12,9 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "support/Path.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -271,6 +273,29 @@
 *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
 }
 
+#ifdef _WIN32
+// Outside of windows systems, we usually have case sensitive file systems.
+TEST(HeaderSourceSwitchTest, CaseSensitivity) {
+  TestTU TU = TestTU::withCode("void foo() {}");
+  TU.HeaderCode = R"cpp(
+  inline void bar1() {}
+  inline void bar2() {})cpp";
+  // Give main file and header different base names to make sure file system
+  // heuristics don't work.
+  TU.Filename = "Source.cpp";
+  TU.HeaderFilename = "Header.h";
+
+  auto Index = TU.index();
+  auto AST = TU.build();
+
+  auto HeaderAbsPath = testPath(TU.Filename);
+  HeaderAbsPath[0] = llvm::toLower(HeaderAbsPath[0]);
+  EXPECT_EQ(testPath(TU.Filename),
+getCorrespondingHeaderOrSource(testPath(TU.HeaderFilename), AST,
+   Index.get()));
+}
+#endif
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
@@ -82,7 +83,7 @@
   llvm::StringMap Candidates; // Target path => score.
   auto AwardTarget = [&](const char *TargetURI) {
 if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
-  if (*TargetPath != OriginalFile) // exclude the original file.
+  if (!pathEqual(*TargetPath, OriginalFile)) // exclude the original file.
 ++Candidates[*TargetPath];
 } else {
   elog("Failed to resolve URI {0}: {1}", TargetURI, 
TargetPath.takeError());


Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -12,7 +12,9 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "support/Path.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -271,6 +273,29 @@
 *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
 }
 
+#ifdef _WIN32
+// Outside of windows systems, we usually have case sensitive file systems.
+TEST(HeaderSourceSwitchTest, CaseSensitivity) {
+  TestTU TU = TestTU::withCode("void foo() {}");
+  TU.HeaderCode = R"cpp(
+  inline void bar1() {}
+  inline void bar2() {})cpp";
+  // Give main file and header different base names to make sure file system
+  // heuristics don't work.
+  TU.Filename = "Source.cpp";
+  TU.HeaderFilename = "Header.h";
+
+  auto Index = TU.index();
+  auto AST = TU.build();
+
+  auto HeaderAbsPath = testPath(TU.Filename);
+  HeaderAbsPath[0] = llvm::toLower(HeaderAbsPath[0]);
+  EXPECT_EQ(testPath(TU.Filename),
+getCorrespondingHeaderOrSource(testPath(TU.HeaderFilename), AST,
+   Index.get()));
+}
+#endif
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
@@ -82,7 +83,7 @@
   llvm::StringMap Candidates; // Target path => score.
   auto AwardTarget = [&](const char *TargetURI) {
 if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
-  if (*TargetPath != OriginalFile) // exclude the original file.
+  if (!pathEqual(*TargetPath, OriginalFile)) // exclude the original file.
 ++Candidates[*TargetPath];
 } else

[PATCH] D121175: [clang] Add -Wstart-no-unknown-warning-option/-Wend-no-unknown-warning-option.

[Zhiwei Chen via Phabricator via cfe-commits]

condy added a comment.

IMHO, it's the duty of build systems. CMake provides `check_cxx_compiler_flag` 
to report unknown options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121175

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

[PATCH] D121359: [AVR] Synchronize device list with gcc-avr 5.4.0 and avr-libc 2.0.0

[Ben Shi via Phabricator via cfe-commits]

benshi001 created this revision.
benshi001 added reviewers: dylanmckay, aykevl.
Herald added subscribers: Jim, hiraditya.
Herald added a project: All.
benshi001 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jacquesguan.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121359

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/lib/Target/AVR/AVRDevices.td

Index: llvm/lib/Target/AVR/AVRDevices.td
===
--- llvm/lib/Target/AVR/AVRDevices.td
+++ llvm/lib/Target/AVR/AVRDevices.td
@@ -245,6 +245,7 @@
 def : Device<"avrtiny", FamilyTiny, ELFArchTiny>;
 
 // Specific MCUs
+// NOTE: This list has been synchronized with gcc-avr 5.4.0 and avr-libc 2.0.0.
 def : Device<"at90s1200", FamilyAVR0, ELFArchAVR1>;
 def : Device<"attiny11", FamilyAVR1, ELFArchAVR1>;
 def : Device<"attiny12", FamilyAVR1, ELFArchAVR1>;
@@ -299,6 +300,8 @@
 def : Device<"at90usb82", FamilyAVR35, ELFArchAVR35>;
 def : Device<"at90usb162", FamilyAVR35, ELFArchAVR35>;
 def : Device<"ata5505", FamilyAVR35, ELFArchAVR35>;
+def : Device<"ata6617c", FamilyAVR35, ELFArchAVR35>;
+def : Device<"ata664251", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega8u2", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega16u2", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega32u2", FamilyAVR35, ELFArchAVR35>;
@@ -310,6 +313,7 @@
  [FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
 def : Device<"ata6285", FamilyAVR4, ELFArchAVR4>;
 def : Device<"ata6286", FamilyAVR4, ELFArchAVR4>;
+def : Device<"ata6612c", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48a", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48pa", FamilyAVR4, ELFArchAVR4>;
@@ -331,8 +335,17 @@
 def : Device<"at90pwm3", FamilyAVR4, ELFArchAVR4>;
 def : Device<"at90pwm3b", FamilyAVR4, ELFArchAVR4>;
 def : Device<"at90pwm81", FamilyAVR4, ELFArchAVR4>;
+def : Device<"ata5702m322", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5782", FamilyAVR5, ELFArchAVR5>;
 def : Device<"ata5790", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5790n", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5791", FamilyAVR5, ELFArchAVR5>;
 def : Device<"ata5795", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5831", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata6613c", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata6614q", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata8210", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata8510", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega16", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega16a", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega161", FamilyAVR3, ELFArchAVR5,
@@ -411,6 +424,7 @@
 def : Device<"atmega32hvb", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega32hvbrevb", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega64hve", FamilyAVR5, ELFArchAVR5>;
+def : Device<"atmega64hve2", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90can32", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90can64", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90pwm161", FamilyAVR5, ELFArchAVR5>;
@@ -452,7 +466,9 @@
 def : Device<"atxmega16d4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32a4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32a4u", FamilyXMEGAU, ELFArchXMEGA2>;
+def : Device<"atxmega32c3", FamilyXMEGAU, ELFArchXMEGA2>;
 def : Device<"atxmega32c4", FamilyXMEGAU, ELFArchXMEGA2>;
+def : Device<"atxmega32d3", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32d4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32e5", FamilyXMEGAU, ELFArchXMEGA2>;
 def : Device<"atxmega16e5", FamilyXMEGAU, ELFArchXMEGA2>;
Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -77,7 +77,7 @@
 
 // RUN: not %clang_cc1 -triple avr--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix AVR
 // AVR: error: unknown target CPU 'not-a-cpu'
-// AVR-NEXT: note: valid target CPU values are: avr1, avr2, avr25, avr3, avr31, avr35, avr4, avr5, avr51, avr6, avrxmega1, avrxmega2, avrxmega3, avrxmega4, avrxmega5, avrxmega6, avrxmega7, avrtiny, at90s1200, attiny11, attiny12, attiny15, attiny28, at90s2313, at90s2323, at90s2333, at90s2343, attiny22, attiny26, at86rf401, at90s4414, at90s4433, at90s4434, at90s8515, at90c8534, at90s8535, ata5272, attiny13, attiny13a, attiny2313, attiny2313a, attiny24, attiny24a, attiny4313, attiny44, attiny44a, attiny84, attiny84a, attiny25, attiny45, attiny85, attiny261, attiny261a, attiny441, attiny461, attiny461a, attiny841, attiny861, attiny861a, attiny87, attiny43u, attiny48, attiny88, attiny828, at43usb355, at76c711, atmega103, at43usb320, attiny167, at90usb82, at90usb162, ata5505, atmega8u2, atmega16u2, atmega32u2, attiny1634, atm

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D121214#3369871 , @steakhal wrote:

> Drop the alias-related changes and preserve the note in the 
> `bugprone-shared-ptr-array-mismatch.rst` stating this relationship with the 
> cert rule?
> If we do it like that, then this will be again NFC.

I would suggest definitely doing that. Perhaps with a bit more emphasis on this 
one catching the `shared_ptr` case only.
If you could create the check for the `unique_ptr` case, that would also be 
great.
I think some of MEM51-CPP is covered by the Static Analyser, right? (Things 
like `free()`ing a `new`-created pointer.)

In D121214#3369871 , @steakhal wrote:

> How shall I proceed?

Let's ask @aaron.ballman or @njames93 with regards to that. The long-term 
solution would be implementing the `1-to-N` check aliasing in the 
infrastructure, but I understand it might be an out-of-scope work right now.



However, I am also in favour of creating a mapping of "partially implemented 
guidelines" in the documentation somewhere... maybe in the `list.rst`, maybe on 
a separate page. There, we could start documenting cases like this one... It's 
less direct than an alias, but more accessible to users who wish to cover as 
many rules of thumb as possible than having to iterate through hundreds of 
distinct documentation pages for the checks and trying to deduce //"Hey, so 
this one makes me catch some of that guideline!"//.

Consider (in a Markdown example, because I can never remember at the top of my 
head how the heading level magic characters of RST are in sequence...)

  # Guideline coverage
  
  ## CppCoreGuidelines
  
  {first the fully covered, I believe the cross-reference links here are enough}
   * X.00 Blah blah rule | cppcoreguidelines-foo-bar
   * Y.01 Blah blah rule | cppcoreguidelines-baz-qux
  
  {then the partially covered}
   * Z.99 Blah blah rule | misc-whatever-something, bugprone-fancy-diagnostics
  
  ## CERT
  <... same format ...>
  
  <... etc. ...>

Note: I'm not saying you should go ahead and create the ENTIRE collection of 
these data right now, just to start pinning down a format. A //"work in 
progress"// marker could also be added to the top of this new page.


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

https://reviews.llvm.org/D121214

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

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Seems OK. Thanks for the patch.

Do opaque pointer variants (like `i32 @llvm.aarch64.stxr.p0(i64 1, ptr 
elementtype(i64) %ptr.0)`) get tested automatically from the existing tests 
once -opaque-pointers is the default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120527

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

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added reviewers: gribozavr, alexfh.
Herald added a project: All.
courbet requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121365

Files:
  clang/lib/Analysis/CFG.cpp
  clang/unittests/Analysis/CFGBuildResult.h
  clang/unittests/Analysis/CFGTest.cpp


Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -70,6 +70,27 @@
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+template 
+struct Base {
+  virtual ~Base() {}
+};
+
+template 
+struct Derived : public Base {
+  virtual ~Derived() {}
+};
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+BuildCFG(Code, Options, ast_matchers::hasName("~Derived"))
+.getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
 BuildResult B = BuildCFG(Code);
Index: clang/unittests/Analysis/CFGBuildResult.h
===
--- clang/unittests/Analysis/CFGBuildResult.h
+++ clang/unittests/Analysis/CFGBuildResult.h
@@ -56,13 +56,15 @@
 TheBuildResult = BuildResult::SawFunctionBody;
 Options.AddImplicitDtors = true;
 if (std::unique_ptr Cfg =
-CFG::buildCFG(nullptr, Body, Result.Context, Options))
+CFG::buildCFG(Func, Body, Result.Context, Options))
   TheBuildResult = {BuildResult::BuiltCFG, Func, std::move(Cfg),
 std::move(AST)};
   }
 };
 
-inline BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {}) {
+template 
+BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {},
+ FuncMatcherT FuncMatcher = ast_matchers::anything()) {
   std::vector Args = {"-std=c++11",
"-fno-delayed-template-parsing"};
   std::unique_ptr AST = tooling::buildASTFromCodeWithArgs(Code, Args);
@@ -72,7 +74,8 @@
   CFGCallback Callback(std::move(AST));
   Callback.Options = Options;
   ast_matchers::MatchFinder Finder;
-  Finder.addMatcher(ast_matchers::functionDecl().bind("func"), &Callback);
+  Finder.addMatcher(ast_matchers::functionDecl(FuncMatcher).bind("func"),
+&Callback);
 
   Finder.matchAST(Callback.AST->getASTContext());
   return std::move(Callback.TheBuildResult);
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -1884,7 +1884,7 @@
 // (which is different from the current class) is responsible for
 // destroying them.
 const CXXRecordDecl *CD = VI.getType()->getAsCXXRecordDecl();
-if (!CD->hasTrivialDestructor()) {
+if (CD && !CD->hasTrivialDestructor()) {
   autoCreateBlock();
   appendBaseDtor(Block, &VI);
 }
@@ -1894,7 +1894,7 @@
   for (const auto &BI : RD->bases()) {
 if (!BI.isVirtual()) {
   const CXXRecordDecl *CD = BI.getType()->getAsCXXRecordDecl();
-  if (!CD->hasTrivialDestructor()) {
+  if (CD && !CD->hasTrivialDestructor()) {
 autoCreateBlock();
 appendBaseDtor(Block, &BI);
   }


Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -70,6 +70,27 @@
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+template 
+struct Base {
+  virtual ~Base() {}
+};
+
+template 
+struct Derived : public Base {
+  virtual ~Derived() {}
+};
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+BuildCFG(Code, Options, ast_matchers::hasName("~Derived"))
+.getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
 BuildResult B = BuildCFG(Code);
Index: clang/unittests/Analysis/CFGBuildResult.h
===
--- clang/unittests/Analysis/CFGBuildResult.h
+++ clang/unittests/Analysis/CFGBuildResult.h
@@ -56,13 +56,15 @@
 TheBuildResult = BuildResult::SawFunctionBody;
 Options.AddImplicitDtors = true;
 if (std::unique_ptr Cfg =
-CFG::buildCFG(nullptr, Body, Result.Context, Options))
+CFG::buildCFG(Func, Body, Result.Context, Opt

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Basically, this looks good to me. But my confidence with ObjC is low, so, I'd 
like @shafik as well to approve.
Besides, seems like the pre-merge check fails with the new tests on Debian, 
could you please address that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121176

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

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

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

steakhal planned changes to this revision.
steakhal added a comment.

In D121214#3372091 , @whisperity 
wrote:

> In D121214#3369871 , @steakhal 
> wrote:
>
>> Drop the alias-related changes and preserve the note in the 
>> `bugprone-shared-ptr-array-mismatch.rst` stating this relationship with the 
>> cert rule?
>> If we do it like that, then this will be again NFC.
>
> I would suggest definitely doing that. Perhaps with a bit more emphasis on 
> this one catching the `shared_ptr` case only.

Okay, I'll pick this direction.

> If you could create the check for the `unique_ptr` case, that would also be 
> great.

We will put it on the roadmap, but no promises :D

> I think some of MEM51-CPP is covered by the Static Analyser, right? (Things 
> like `free()`ing a `new`-created pointer.)

Exactly. CSA checkers 
`unix.MismatchedDeallocator`,`cplusplus.NewDelete`,`cplusplus.NewDeleteLeaks` 
together can catch all but the last two cases.

> In D121214#3369871 , @steakhal 
> wrote:
>
>> How shall I proceed?
>
> Let's ask @aaron.ballman or @njames93 with regards to that. The long-term 
> solution would be implementing the `1-to-N` check aliasing in the 
> infrastructure, but I understand it might be an out-of-scope work right now.

This is definitely out of scope for me ATM.

> However, I am also in favour of creating a mapping of "partially implemented 
> guidelines" in the documentation somewhere... maybe in the `list.rst`, maybe 
> on a separate page. There, we could start documenting cases like this one... 
> It's less direct than an alias, but more accessible to users who wish to 
> cover as many rules of thumb as possible than having to iterate through 
> hundreds of distinct documentation pages for the checks and trying to deduce 
> //"Hey, so this one makes me catch some of that guideline!"//.

We should also consider the documentation hell, and carefully design a solution 
that mitigates the already existing burden.
It's already hard to keep everything in sync.


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

https://reviews.llvm.org/D121214

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

[PATCH] D121359: [AVR] Add more devices

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 414319.
benshi001 edited the summary of this revision.

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

https://reviews.llvm.org/D121359

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/lib/Target/AVR/AVRDevices.td

Index: llvm/lib/Target/AVR/AVRDevices.td
===
--- llvm/lib/Target/AVR/AVRDevices.td
+++ llvm/lib/Target/AVR/AVRDevices.td
@@ -245,6 +245,7 @@
 def : Device<"avrtiny", FamilyTiny, ELFArchTiny>;
 
 // Specific MCUs
+// NOTE: This list has been synchronized with gcc-avr 5.4.0 and avr-libc 2.0.0.
 def : Device<"at90s1200", FamilyAVR0, ELFArchAVR1>;
 def : Device<"attiny11", FamilyAVR1, ELFArchAVR1>;
 def : Device<"attiny12", FamilyAVR1, ELFArchAVR1>;
@@ -299,6 +300,8 @@
 def : Device<"at90usb82", FamilyAVR35, ELFArchAVR35>;
 def : Device<"at90usb162", FamilyAVR35, ELFArchAVR35>;
 def : Device<"ata5505", FamilyAVR35, ELFArchAVR35>;
+def : Device<"ata6617c", FamilyAVR35, ELFArchAVR35>;
+def : Device<"ata664251", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega8u2", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega16u2", FamilyAVR35, ELFArchAVR35>;
 def : Device<"atmega32u2", FamilyAVR35, ELFArchAVR35>;
@@ -310,6 +313,7 @@
  [FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
 def : Device<"ata6285", FamilyAVR4, ELFArchAVR4>;
 def : Device<"ata6286", FamilyAVR4, ELFArchAVR4>;
+def : Device<"ata6612c", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48a", FamilyAVR4, ELFArchAVR4>;
 def : Device<"atmega48pa", FamilyAVR4, ELFArchAVR4>;
@@ -331,8 +335,17 @@
 def : Device<"at90pwm3", FamilyAVR4, ELFArchAVR4>;
 def : Device<"at90pwm3b", FamilyAVR4, ELFArchAVR4>;
 def : Device<"at90pwm81", FamilyAVR4, ELFArchAVR4>;
+def : Device<"ata5702m322", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5782", FamilyAVR5, ELFArchAVR5>;
 def : Device<"ata5790", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5790n", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5791", FamilyAVR5, ELFArchAVR5>;
 def : Device<"ata5795", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata5831", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata6613c", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata6614q", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata8210", FamilyAVR5, ELFArchAVR5>;
+def : Device<"ata8510", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega16", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega16a", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega161", FamilyAVR3, ELFArchAVR5,
@@ -411,6 +424,7 @@
 def : Device<"atmega32hvb", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega32hvbrevb", FamilyAVR5, ELFArchAVR5>;
 def : Device<"atmega64hve", FamilyAVR5, ELFArchAVR5>;
+def : Device<"atmega64hve2", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90can32", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90can64", FamilyAVR5, ELFArchAVR5>;
 def : Device<"at90pwm161", FamilyAVR5, ELFArchAVR5>;
@@ -452,7 +466,9 @@
 def : Device<"atxmega16d4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32a4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32a4u", FamilyXMEGAU, ELFArchXMEGA2>;
+def : Device<"atxmega32c3", FamilyXMEGAU, ELFArchXMEGA2>;
 def : Device<"atxmega32c4", FamilyXMEGAU, ELFArchXMEGA2>;
+def : Device<"atxmega32d3", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32d4", FamilyXMEGA, ELFArchXMEGA2>;
 def : Device<"atxmega32e5", FamilyXMEGAU, ELFArchXMEGA2>;
 def : Device<"atxmega16e5", FamilyXMEGAU, ELFArchXMEGA2>;
Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -77,7 +77,7 @@
 
 // RUN: not %clang_cc1 -triple avr--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix AVR
 // AVR: error: unknown target CPU 'not-a-cpu'
-// AVR-NEXT: note: valid target CPU values are: avr1, avr2, avr25, avr3, avr31, avr35, avr4, avr5, avr51, avr6, avrxmega1, avrxmega2, avrxmega3, avrxmega4, avrxmega5, avrxmega6, avrxmega7, avrtiny, at90s1200, attiny11, attiny12, attiny15, attiny28, at90s2313, at90s2323, at90s2333, at90s2343, attiny22, attiny26, at86rf401, at90s4414, at90s4433, at90s4434, at90s8515, at90c8534, at90s8535, ata5272, attiny13, attiny13a, attiny2313, attiny2313a, attiny24, attiny24a, attiny4313, attiny44, attiny44a, attiny84, attiny84a, attiny25, attiny45, attiny85, attiny261, attiny261a, attiny441, attiny461, attiny461a, attiny841, attiny861, attiny861a, attiny87, attiny43u, attiny48, attiny88, attiny828, at43usb355, at76c711, atmega103, at43usb320, attiny167, at90usb82, at90usb162, ata5505, atmega8u2, atmega16u2, atmega32u2, attiny1634, atmega8, ata6289, atmega8a, ata6285, ata6286, atmega48, atmega48a, atmega48pa, atmega48pb, atmega48p, atmega88, atmega88a, atmega88p, atmega88pa, atmega88pb, atmega8515, atmega85

[PATCH] D121286: [clangd] Test against path insensitivity

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 414324.
kadircet added a comment.

- Adjust after tests on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121286

Files:
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp

Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -99,7 +99,7 @@
   llvm::Expected
   getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
   llvm::StringRef HintPath) const override {
-if (!HintPath.empty() && !HintPath.startswith(testRoot()))
+if (!HintPath.empty() && !pathStartsWith(testRoot(), HintPath))
   return error("Hint path is not empty and doesn't start with {0}: {1}",
testRoot(), HintPath);
 if (!Body.consume_front("/"))
@@ -111,12 +111,11 @@
 
   llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
-llvm::StringRef Body = AbsolutePath;
-if (!Body.consume_front(testRoot()))
+if (!pathConsumeFront(AbsolutePath, testRoot()))
   return error("{0} does not start with {1}", AbsolutePath, testRoot());
 
 return URI(Scheme, /*Authority=*/"",
-   llvm::sys::path::convert_to_slash(Body));
+   llvm::sys::path::convert_to_slash(AbsolutePath));
   }
 };
 
Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -12,7 +12,9 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "support/Path.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -271,6 +273,31 @@
 *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
 }
 
+#ifdef _WIN32
+// Outside of windows systems, we usually have case sensitive file systems.
+TEST(HeaderSourceSwitchTest, CaseSensitivity) {
+  TestTU TU = TestTU::withCode("void foo() {}");
+  TU.HeaderCode = R"cpp(
+  inline void bar1() {}
+  inline void bar2() {}
+  void foo();)cpp";
+  // Give main file and header different base names to make sure file system
+  // heuristics don't work.
+  TU.Filename = "Source.cpp";
+  TU.HeaderFilename = "Header.h";
+
+  auto Index = TU.index();
+  TU.Code = std::move(TU.HeaderCode);
+  TU.HeaderCode.clear();
+  auto AST = TU.build();
+
+  auto HeaderAbsPath = testPath(TU.HeaderFilename);
+  HeaderAbsPath[0] = llvm::toLower(HeaderAbsPath[0]);
+  EXPECT_EQ(testPath(TU.Filename),
+getCorrespondingHeaderOrSource(HeaderAbsPath, AST, Index.get()));
+}
+#endif
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -44,6 +44,11 @@
 /// Variant of parent_path that operates only on absolute paths.
 /// Unlike parent_path doesn't consider C: a parent of C:\.
 PathRef absoluteParent(PathRef Path);
+
+/// Tries to strip \p Prefix from beginning of \p Path. Returns true on success.
+/// If \p Prefix doesn't match, leaves \p Path untouched and returns false.
+bool pathConsumeFront(PathRef &Path, PathRef Prefix);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -49,5 +49,12 @@
   // afterwards.
   return Path.empty() || llvm::sys::path::is_separator(Path.front(), Style);
 }
+
+bool pathConsumeFront(PathRef &Path, PathRef Prefix) {
+  if (!pathStartsWith(Prefix, Path))
+return false;
+  Path = Path.drop_front(Prefix.size());
+  return true;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
@@ -82,7 +83,7 @@
   llvm::StringMap Candidates; // Target path => score.
   auto AwardTarget = [&](const char *TargetURI) {
 if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
-  if (*Targe

[PATCH] D121286: [clangd] Handle case insensitive file systems in header/source switch

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 414325.
kadircet retitled this revision from "[clangd] Test against path insensitivity" 
to "[clangd] Handle case insensitive file systems in header/source switch".
kadircet added a comment.

- rename patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121286

Files:
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp

Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -99,7 +99,7 @@
   llvm::Expected
   getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
   llvm::StringRef HintPath) const override {
-if (!HintPath.empty() && !HintPath.startswith(testRoot()))
+if (!HintPath.empty() && !pathStartsWith(testRoot(), HintPath))
   return error("Hint path is not empty and doesn't start with {0}: {1}",
testRoot(), HintPath);
 if (!Body.consume_front("/"))
@@ -111,12 +111,11 @@
 
   llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
-llvm::StringRef Body = AbsolutePath;
-if (!Body.consume_front(testRoot()))
+if (!pathConsumeFront(AbsolutePath, testRoot()))
   return error("{0} does not start with {1}", AbsolutePath, testRoot());
 
 return URI(Scheme, /*Authority=*/"",
-   llvm::sys::path::convert_to_slash(Body));
+   llvm::sys::path::convert_to_slash(AbsolutePath));
   }
 };
 
Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -12,7 +12,9 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "support/Path.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -271,6 +273,31 @@
 *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
 }
 
+#ifdef _WIN32
+// Outside of windows systems, we usually have case sensitive file systems.
+TEST(HeaderSourceSwitchTest, CaseSensitivity) {
+  TestTU TU = TestTU::withCode("void foo() {}");
+  TU.HeaderCode = R"cpp(
+  inline void bar1() {}
+  inline void bar2() {}
+  void foo();)cpp";
+  // Give main file and header different base names to make sure file system
+  // heuristics don't work.
+  TU.Filename = "Source.cpp";
+  TU.HeaderFilename = "Header.h";
+
+  auto Index = TU.index();
+  TU.Code = std::move(TU.HeaderCode);
+  TU.HeaderCode.clear();
+  auto AST = TU.build();
+
+  auto HeaderAbsPath = testPath(TU.HeaderFilename);
+  HeaderAbsPath[0] = llvm::toLower(HeaderAbsPath[0]);
+  EXPECT_EQ(testPath(TU.Filename),
+getCorrespondingHeaderOrSource(HeaderAbsPath, AST, Index.get()));
+}
+#endif
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -44,6 +44,11 @@
 /// Variant of parent_path that operates only on absolute paths.
 /// Unlike parent_path doesn't consider C: a parent of C:\.
 PathRef absoluteParent(PathRef Path);
+
+/// Tries to strip \p Prefix from beginning of \p Path. Returns true on success.
+/// If \p Prefix doesn't match, leaves \p Path untouched and returns false.
+bool pathConsumeFront(PathRef &Path, PathRef Prefix);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -49,5 +49,12 @@
   // afterwards.
   return Path.empty() || llvm::sys::path::is_separator(Path.front(), Style);
 }
+
+bool pathConsumeFront(PathRef &Path, PathRef Prefix) {
+  if (!pathStartsWith(Prefix, Path))
+return false;
+  Path = Path.drop_front(Prefix.size());
+  return true;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
@@ -82,7 +83,7 @@
   llvm::StringMap Candidates; // Target path => sco

[PATCH] D121286: [clangd] Handle case insensitive file systems in header/source switch

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 414326.
kadircet added a comment.

- Match path ignoring case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121286

Files:
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp

Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -99,7 +99,7 @@
   llvm::Expected
   getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
   llvm::StringRef HintPath) const override {
-if (!HintPath.empty() && !HintPath.startswith(testRoot()))
+if (!HintPath.empty() && !pathStartsWith(testRoot(), HintPath))
   return error("Hint path is not empty and doesn't start with {0}: {1}",
testRoot(), HintPath);
 if (!Body.consume_front("/"))
@@ -111,12 +111,11 @@
 
   llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
-llvm::StringRef Body = AbsolutePath;
-if (!Body.consume_front(testRoot()))
+if (!pathConsumeFront(AbsolutePath, testRoot()))
   return error("{0} does not start with {1}", AbsolutePath, testRoot());
 
 return URI(Scheme, /*Authority=*/"",
-   llvm::sys::path::convert_to_slash(Body));
+   llvm::sys::path::convert_to_slash(AbsolutePath));
   }
 };
 
Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -12,7 +12,9 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "support/Path.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -271,6 +273,31 @@
 *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
 }
 
+#ifdef _WIN32
+// Outside of windows systems, we usually have case sensitive file systems.
+TEST(HeaderSourceSwitchTest, CaseSensitivity) {
+  TestTU TU = TestTU::withCode("void foo() {}");
+  TU.HeaderCode = R"cpp(
+  inline void bar1() {}
+  inline void bar2() {}
+  void foo();)cpp";
+  // Give main file and header different base names to make sure file system
+  // heuristics don't work.
+  TU.Filename = "Source.cpp";
+  TU.HeaderFilename = "Header.h";
+
+  auto Index = TU.index();
+  TU.Code = std::move(TU.HeaderCode);
+  TU.HeaderCode.clear();
+  auto AST = TU.build();
+
+  auto HeaderAbsPath = testPath(TU.HeaderFilename);
+  HeaderAbsPath[0] = llvm::toLower(HeaderAbsPath[0]);
+  EXPECT_THAT(testing::StrCaseEq(testPath(TU.Filename)),
+  getCorrespondingHeaderOrSource(HeaderAbsPath, AST, Index.get()));
+}
+#endif
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -44,6 +44,11 @@
 /// Variant of parent_path that operates only on absolute paths.
 /// Unlike parent_path doesn't consider C: a parent of C:\.
 PathRef absoluteParent(PathRef Path);
+
+/// Tries to strip \p Prefix from beginning of \p Path. Returns true on success.
+/// If \p Prefix doesn't match, leaves \p Path untouched and returns false.
+bool pathConsumeFront(PathRef &Path, PathRef Prefix);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -49,5 +49,12 @@
   // afterwards.
   return Path.empty() || llvm::sys::path::is_separator(Path.front(), Style);
 }
+
+bool pathConsumeFront(PathRef &Path, PathRef Prefix) {
+  if (!pathStartsWith(Prefix, Path))
+return false;
+  Path = Path.drop_front(Prefix.size());
+  return true;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/clangd/HeaderSourceSwitch.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
@@ -82,7 +83,7 @@
   llvm::StringMap Candidates; // Target path => score.
   auto AwardTarget = [&](const char *TargetURI) {
 if (auto TargetPath = URI::resolve(TargetURI, OriginalFile))

[PATCH] D121368: [pseudo][WIP] Build Ambiguous forest node in the GLR Parser.

[Haojian Wu via Phabricator via cfe-commits]

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

Forest node by design is unmutable. To create an ambiguous node, we have
to know all alternatives in advance.

In order to achieve that, we must perform all reductions in a careful
order (see code comments for details), so that we can gather completed
alternatives as a batch, and process them in a single pass.

E.g. considering the following grammar:

  bnf
  TU := stmt
  TU := expr
  stmt := expr
  expr := ID
  stmt := ID
  
  // Ambiguous stmt forest node:
  // stmt (ambiguous)
  /// \
  //   /  stmt
  //   |   |
  //  stmt   expr
  //\ /
  //   ID

The ambiguous Stmt node is built in a single section where we perform three 
reductions:

1. expr := ID
2. stmt := ID
3. stmt := expr (enabled after 1 is performed)

We expect to perform them in a batch way with the order {1}, {2, 3}.
When processing the batch {2, 3} where ambiguity happens, we build an
ambiguous node for stmt.

Based on https://reviews.llvm.org/D121150


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121368

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Forest.h
  clang/include/clang/Tooling/Syntax/Pseudo/GLRParser.h
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/tools/clang-pseudo/ClangPseudo.cpp

Index: clang/tools/clang-pseudo/ClangPseudo.cpp
===
--- clang/tools/clang-pseudo/ClangPseudo.cpp
+++ clang/tools/clang-pseudo/ClangPseudo.cpp
@@ -89,7 +89,7 @@
  << " nodes: " << Arena.nodeNum() << "\n";
 llvm::outs() << "GSS bytes: " << Parser.getGSS().bytes()
  << " nodes: " << Parser.getGSS().nodeCount() << "\n";
-// llvm::outs() << Root->DumpRecursive(*G, true);
+llvm::outs() << Root->DumpRecursive(*G, false);
   }
 }
 return 0;
Index: clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
@@ -100,11 +100,52 @@
 ++RulePos;
   T->Nonterminals[SID].RuleRange = {Start, RulePos};
 }
+calculateDependencyOrder(T.get());
 auto G = std::make_unique(std::move(T));
 diagnoseGrammar(*G);
 return G;
   }
 
+  void calculateDependencyOrder(GrammarTable *T) const {
+llvm::DenseMap> DependencyGraph;
+for (const auto &Rule : T->Rules) {
+  // A := B, A depends on B.
+  if (Rule.Size == 1 && pseudo::isNonterminal(Rule.Sequence[0]))
+DependencyGraph[Rule.Target].insert(Rule.Sequence[0]);
+}
+std::vector Order;
+// Each nonterminal state flows: NotVisited -> Visiting -> Visited.
+enum State {
+  NotVisited,
+  Visiting,
+  Visited,
+};
+std::vector VisitStates(T->Nonterminals.size(), NotVisited);
+std::function DFS = [&](SymbolID SID) -> void {
+  if (VisitStates[SID] == Visited)
+return;
+  if (VisitStates[SID] == Visiting) {
+Diagnostics.push_back(
+llvm::formatv("The grammar is cyclic, see symbol {0}\n",
+  T->Nonterminals[SID].Name));
+return;
+  }
+  VisitStates[SID] = Visiting;
+  auto It = DependencyGraph.find(SID);
+  if (It != DependencyGraph.end()) {
+for (SymbolID Dep : (It->getSecond()))
+  DFS(Dep);
+  }
+  VisitStates[SID] = Visited;
+  Order.push_back(SID);
+};
+for (SymbolID ID = 0; ID != T->Nonterminals.size(); ++ID)
+  DFS(ID);
+for (size_t I = 0; I < Order.size(); ++I) {
+  T->Nonterminals[Order[I]].TopologicalOrder = I;
+}
+  }
+
 private:
   // Text representation of a BNF grammar rule.
   struct RuleSpec {
Index: clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp
===
--- clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp
+++ clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp
@@ -18,6 +18,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 #include 
+#include 
 #include 
 
 #define DEBUG_TYPE "GLRParser.cpp"
@@ -156,72 +157,135 @@
   enumPath(Head, PathLength);
 }
 
-// Perform reduction recursively until we don't have reduce actions with
-// heads.
+// Perform reductions recursively until there is no available reductions.
+//
+// Reductions can manipulate the GSS in following way:
+//
+//  1) Split --
+// 1.1 when a stack head has mutiple reduce actions, the head is
+// made to split to accommodate the various possiblities.
+// E.g.
+//   0 -> 1 (ID)
+// After performing reduce of production rules (class-name := ID,
+// enum-name := ID), t

[clang] af98b0a - [clang][dataflow] Add analysis that detects unsafe accesses to optionals

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-03-10T11:05:31Z
New Revision: af98b0af6705df3859ee3c98525b57025d40d9e8

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

LOG: [clang][dataflow] Add analysis that detects unsafe accesses to optionals

This commit reverts e0cc28dfdc67105974924cce42bb8c85bd44925a and moves
UncheckedOptionalAccessModelTest.cpp into 
clang/unittests/Analysis/FlowSensitive,
to avoid build failures. The test will be moved back into a Models subdir
in a follow up patch that will address the build configuration issues.

Original description:

Adds a dataflow analysis that detects unsafe accesses to values of type
`std::optional`, `absl::optional`, or `base::Optional`.

Reviewed-by: ymandel, xazax.hun

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

Added: 

clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Modified: 
clang/lib/Analysis/FlowSensitive/CMakeLists.txt
clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

Removed: 




diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
 
b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
new file mode 100644
index 0..e5ad0cadd7bd9
--- /dev/null
+++ 
b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -0,0 +1,40 @@
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+
+namespace clang {
+namespace dataflow {
+
+/// Dataflow analysis that discovers unsafe accesses of optional values and
+/// adds the respective source locations to the lattice.
+///
+/// Models the `std::optional`, `absl::optional`, and `base::Optional` types.
+///
+/// FIXME: Consider separating the models from the unchecked access analysis.
+class UncheckedOptionalAccessModel
+: public DataflowAnalysis {
+public:
+  explicit UncheckedOptionalAccessModel(ASTContext &AstContext);
+
+  static SourceLocationsLattice initialElement() {
+return SourceLocationsLattice();
+  }
+
+  void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
+Environment &Env);
+
+private:
+  MatchSwitch> TransferMatchSwitch;
+};
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H

diff  --git a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt 
b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
index cfe3c8e77b4fd..4a8c63b3db21e 100644
--- a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
@@ -12,3 +12,5 @@ add_clang_library(clangAnalysisFlowSensitive
   clangAST
   clangBasic
   )
+
+add_subdirectory(Models)

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt 
b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
new file mode 100644
index 0..5bed00c4bfdc6
--- /dev/null
+++ b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_clang_library(clangAnalysisFlowSensitiveModels
+  UncheckedOptionalAccessModel.cpp
+
+  LINK_LIBS
+  clangAnalysis
+  clangAnalysisFlowSensitive
+  clangAST
+  clangASTMatchers
+  clangBasic
+  )

diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
new file mode 100644
index 0..bb6aa5ff74e2c
--- /dev/null
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -0,0 +1,136 @@
+#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include 
+
+namespace clang {
+namespace dataflow {
+namespace {
+
+using namespace ::clang::ast_matchers;
+
+u

[PATCH] D121269: [clang-format] Fix namespace format when the name is followed by a macro

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:33
+  llvm::function_ref Fn) {
+  if (!Tok || !Tok->is(StartTok))
+return Tok;

And other places below where applicable.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:76
 }
   } else {
 // Skip attributes.

Early return.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:122
+IsPrevColoncolon = FirstNSTok->is(tok::coloncolon);
+HasColoncolon |= IsPrevColoncolon;
+if (FirstNSTok->is(tok::kw_inline)) {





Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:137-140
+if (!FirstNSName.empty() && !HasColoncolon)
+  name = FirstNSName + (!name.empty() ? " " + name : "");
   }
   return name;

Something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121269

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

[PATCH] D121368: [pseudo][WIP] Build Ambiguous forest node in the GLR Parser.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The implementation (`performReduction`) is awkward at the moment, but it is 
completed, should give a global view of the algorithm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121368

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

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

Fixes #38995

This is an attempt to modify the regular expression to identify
`@import` and `import` alongside the regular `#include`. The challenging
part was not to support `@` in addition to `#` but how to handle
everything that comes after the `include|import` keywords. Previously
everything that wasn't `"` or `<` was consumed. But as you can see in
this example from the issue #38995, there is no `"` or `<` following the
keyword:

  @import Foundation;

I experimented with a lot of fancy and useful expressions in this
online regex tool  only to find out that some
things are simply not supported by the regex implementation in LLVM.

- For example the beginning `[\t\ ]*` should be replacable by the horizontal 
whitespace character `\h*` but this will break the 
`SortIncludesTest.LeadingWhitespace` test.

That's why I've chosen to come back to the basic building blocks.

The essential change in this patch is the change from this regular
expression:

  ^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">])
  ~  ~~
  ^  ^
  |  |
  only support # prefix not @|
 only support "" and <> as delimiters
 no support for C++ modules and ;
 ending. Also this allows for ">
 or <" or "" or <> which all seems
 either off or wrong.

to this:

  ^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;)
  ~ ~~~ ~~~ ~~~ 
~
  ^ ^   ^   ^   ^
  | |   |   |   |
  | |   Clearly support "" and 
<>
  | |   as well as an include 
name
  | |   without enclosing 
characters.
  | |   Allows for no mixture 
of ">
  | |or <" or empty include 
names.
  | |
  Now optionally support @ and #.   These are needed to support: 
@import Foundation;

Here is how I've tested this patch:

  ninja clang-Format
  ninja FormatTests
  ./tools/clang/unittests/Format/FormatTests --gtest_filter=SortIncludesTest*

And if that worked I doubled checked that nothing else broke by running
all format checks:

  ./tools/clang/unittests/Format/FormatTests

One side effect of this change is it should partially support
C++20 Module 
`import` lines without the optional `export` in front. Adding
this can be a change on its own that shouldn't be too hard.

I see an opportunity to optimized the matching to exclude `@include` for
example. But eventually these should be caught by the compiler, so...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121370

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -458,6 +458,19 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SupportAtImportLines) {
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "#import \n"
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   EXPECT_EQ("#include \"llvm/a.h\"\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -170,11 +170,11 @@
 }
 
 inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
+  return IncludeName.trim("\"<>;");
 }
 
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("

[PATCH] D120631: [clang-format][docs] Fix incorrect 'clang-format 12' option markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.
Herald added a project: All.

Kind reminder, please deliver this one for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120631

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

[PATCH] D121214: [clang-tidy][docs][NFC] Refer to the CERT rule in bugprone-shared-ptr-array-mismatch docs

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

steakhal updated this revision to Diff 414337.
steakhal retitled this revision from "[clang-tidy][docs][NFC] Add alias 
cert-mem51-cpp to bugprone-shared-ptr-array-mismatch" to 
"[clang-tidy][docs][NFC] Refer to the CERT rule in 
bugprone-shared-ptr-array-mismatch docs".
steakhal added a comment.

Removed all the check alias stuff; preserved only the docs part.
Reworded the docs to put emphasis on the implementation limitations.


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

https://reviews.llvm.org/D121214

Files:
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
@@ -29,3 +29,8 @@
 std::shared_ptr x(new Foo[10]); // no replacement in this case
 // ^ warning: shared pointer to non-array is 
initialized with array [bugprone-shared-ptr-array-mismatch]
   };
+
+This check partially covers the CERT C++ Coding Standard rule
+`MEM51-CPP. Properly deallocate dynamically allocated resources
+`_
+However, only the ``std::shared_ptr`` case is detected by this check.


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
@@ -29,3 +29,8 @@
 std::shared_ptr x(new Foo[10]); // no replacement in this case
 // ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
   };
+
+This check partially covers the CERT C++ Coding Standard rule
+`MEM51-CPP. Properly deallocate dynamically allocated resources
+`_
+However, only the ``std::shared_ptr`` case is detected by this check.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121372: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck

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

steakhal created this revision.
steakhal added reviewers: whisperity, aaron.ballman, njames93, 
LegalizeAdulthood.
steakhal added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, martong, rnkovacs, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a subscriber: cfe-commits.

The docs URL was dangling, and the docs suggested that it has no fixits, but it 
actually had.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121372

Files:
  clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
  clang-tools-extra/docs/clang-tidy/checks/list.rst


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -113,7 +113,7 @@
`bugprone-unused-return-value `_,
`bugprone-use-after-move `_,
`bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_,
+   `cert-dcl21-cpp `_, "Yes"
`cert-dcl50-cpp `_,
`cert-dcl58-cpp `_,
`cert-env33-c `_,
Index: clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
===
--- clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
+++ clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
@@ -19,7 +19,7 @@
 /// object.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cert-postfix-operator.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html
 class PostfixOperatorCheck : public ClangTidyCheck {
 public:
   PostfixOperatorCheck(StringRef Name, ClangTidyContext *Context)


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -113,7 +113,7 @@
`bugprone-unused-return-value `_,
`bugprone-use-after-move `_,
`bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_,
+   `cert-dcl21-cpp `_, "Yes"
`cert-dcl50-cpp `_,
`cert-dcl58-cpp `_,
`cert-env33-c `_,
Index: clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
===
--- clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
+++ clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h
@@ -19,7 +19,7 @@
 /// object.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cert-postfix-operator.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html
 class PostfixOperatorCheck : public ClangTidyCheck {
 public:
   PostfixOperatorCheck(StringRef Name, ClangTidyContext *Context)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121373: [clang-tidy][docs] Fix wrong url in DontModifyStdNamespaceCheck

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

steakhal created this revision.
steakhal added reviewers: whisperity, aaron.ballman, njames93, 
LegalizeAdulthood.
steakhal added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, martong, rnkovacs, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a subscriber: cfe-commits.

It was probably a copy-paste mistake.
The check was added as `cert-dcl58-cpp`, so the doc should point there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121373

Files:
  clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h


Index: clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
===
--- clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
+++ clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
@@ -19,7 +19,7 @@
 /// This check warns for such modifications.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cert-msc53-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html
 class DontModifyStdNamespaceCheck : public ClangTidyCheck {
 public:
   DontModifyStdNamespaceCheck(StringRef Name, ClangTidyContext *Context)


Index: clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
===
--- clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
+++ clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.h
@@ -19,7 +19,7 @@
 /// This check warns for such modifications.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cert-msc53-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html
 class DontModifyStdNamespaceCheck : public ClangTidyCheck {
 public:
   DontModifyStdNamespaceCheck(StringRef Name, ClangTidyContext *Context)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski created this revision.
awarzynski added reviewers: rovka, schweitz, Leporacanthicus, 
kiranchandramohan, shraiysh.
Herald added a reviewer: sscalpone.
Herald added a subscriber: dang.
Herald added projects: Flang, All.
awarzynski requested review of this revision.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

This option is added in both `flang-new` (the compiler driver) and
`flang-new -fc1` (the frontend driver). The semantics are consistent
with `clang` and `clang -cc1`.

As Flang does not run any LLVM passes when invoked with `-emit-llvm`
(i.e. `flang-new -S -emit-llvm `), the tests use
`-S`/`-c`/`-emit-obj` instead. These options require an LLVM backend to
be run by the driver to generate the output (this makese `-mllvm`
relevant here).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121374

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/mllvm.f90

Index: flang/test/Driver/mllvm.f90
===
--- /dev/null
+++ flang/test/Driver/mllvm.f90
@@ -0,0 +1,32 @@
+! Test the `-mlvm` option
+
+!
+! RUN COMMAND
+!
+! 1. Test typical usage.
+! RUN: %flang -S -mllvm -print-before-all %s -o - 2>&1 | FileCheck %s --check-prefix=OUTPUT
+! RUN: %flang_fc1 -S -mllvm -print-before-all %s -o - 2>&1 | FileCheck %s --check-prefix=OUTPUT
+
+! 2. Does the option forwarding from `flang-new` to `flang-new -fc1` work?
+! RUN: %flang -### -S -mllvm -print-before-all %s -o - 2>&1 | FileCheck %s --check-prefix=OPTION_FORWARDING
+
+! 3. Test invalid usage (`-print-before` requires an argument)
+! RUN: not %flang -S -mllvm -print-before %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID_USAGE
+
+!
+! EXPECTED OUTPUT
+!
+! OUTPUT: *** IR Dump Before Pre-ISel Intrinsic Lowering (pre-isel-intrinsic-lowering) ***
+! OUTPUT-NEXT: ; ModuleID = 'FIRModule'
+! OUTPUT-NEXT: source_filename = "FIRModule"
+
+! Verify that `-mllvm ` is forwarded to flang -fc1
+! OPTION_FORWARDING: flang-new" "-fc1"
+! OPTION_FORWARDING-SAME: "-mllvm" "-print-before-all"
+
+! INVALID_USAGE: flang (LLVM option parsing): for the --print-before option: requires a value!
+
+!--
+! INPUT
+!--
+end program
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -47,6 +47,7 @@
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: -help  Display available options
 ! HELP-NEXT: -IAdd directory to the end of the list of include search paths
+! HELP-NEXT: -mllvm  Additional arguments to forward to LLVM's option processing
 ! HELP-NEXT: -module-dir   Put MODULE files in 
 ! HELP-NEXT: -nocpp Disable predefined and command line preprocessor macros
 ! HELP-NEXT: -o   Write output to 
@@ -121,6 +122,7 @@
 ! HELP-FC1-NEXT: -init-only Only execute frontend initialization
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of include search paths
 ! HELP-FC1-NEXT: -load Load the named plugin (dynamic shared object)
+! HELP-FC1-NEXT: -mllvm  Additional arguments to forward to LLVM's option processing
 ! HELP-FC1-NEXT: -module-dir   Put MODULE files in 
 ! HELP-FC1-NEXT: -module-suffix  Use  as the suffix for module files (the default value is `.mod`)
 ! HELP-FC1-NEXT: -nocpp Disable predefined and command line preprocessor macros
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -47,6 +47,7 @@
 ! CHECK-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
 ! CHECK-NEXT: -help Display available options
 ! CHECK-NEXT: -IAdd directory to the end of the list of include search paths
+! CHECK-NEXT: -mllvm  Additional arguments to forward to LLVM's option processing
 ! CHECK-NEXT: -module-dir   Put MODULE files in 
 ! CHECK-NEXT: -nocpp Disable predefined and command line preprocessor macros
 ! CHECK-NEXT: -o  Write output to 
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -135,6 +135,19 @@
 }
   }
 
+  // Honor -mllvm. This should happen AFTER plugins have been loaded!
+  if (!flang->frontendOpts().llvmArgs.em

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:2759
+  IncludeName =
+  StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+}

How does that not go out of scope and I have a dangling reference?



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

I don't see why this is needed. It should match with the previous stuff.



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

HazardyKnusperkeks wrote:
> I don't see why this is needed. It should match with the previous stuff.
Do we want that optional?



Comment at: clang/unittests/Format/SortIncludesTest.cpp:471
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}

I've no idea about obj-c, is it feasible to mix `#` and `@`? If so please add a 
test where the `@` is between `#`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

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

Modify initialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2699,7 +2699,7 @@
 
 TEST_F(FormatTestJS, AlignConsecutiveDeclarations) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("letletVariable = 5;\n"
"double constVariable = 10;",
Style);
@@ -2736,7 +2736,7 @@
 TEST_F(FormatTestJS, AlignConsecutiveAssignments) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
 
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("let letVariable  = 5;\n"
"double constVariable = 10;",
Style);
@@ -2772,8 +2772,8 @@
 
 TEST_F(FormatTestJS, AlignConsecutiveAssignmentsAndDeclarations) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+  Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("letletVariable   = 5;\n"
"double constVariable = 10;",
Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2076,7 +2076,7 @@
   " res2 = [](int &a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int *c;\n"
"const unsigned int *d;\n"
"Const unsigned int &e;\n"
@@ -2117,7 +2117,7 @@
   "res2 = [](int& a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int* c;\n"
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
@@ -2138,7 +2138,7 @@
   verifyFormat("for (int a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo *c : {1, 2, 3})", Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int *c;\n"
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
@@ -2174,7 +2174,7 @@
   " res2 = [](int & a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int*  c;\n"
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
@@ -14462,8 +14462,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveAssignments.Enabled = true;
+  Tab.AlignConsecutiveDeclarations.Enabled = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -14701,8 +14701,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveAssignments.Enabled = true;
+  Tab.AlignConsecutiveDeclarations.Enabled = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -15872,9 +15872,8 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
 
   verifyFormat("#define a 3\n"
  

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D121233#3371422 , @sammccall wrote:

> Thanks Aaron, and my apologies for being easily frustrated.

No worries at all! I always appreciate our discussions, even when we don't 
initially agree.

> In D121233#3370992 , @aaron.ballman 
> wrote:
>
>> 2. The layout of clang-tools-extra in terms of test directories is 
>> problematic and it'd sure be nice if someday someone moved the test 
>> directories under the individual tools instead of using a top-level test 
>> directory.
>
> I'd really like to enable this.
> I think the blocker is mostly the amount of boilerplate needed to set up test 
> directories. 
> These required files are mostly boilerplate:
>
> - test/CMakeLists.txt (deps are meaningful, rest is boilerplate)
> - test/lit.site.cfg.py.in
> - test/lit.cfg.py
> - test/Unit/lit.site.cfg.py.in (this patch puts these under unittest/ 
> instead, but either way)
> - test/Unit/lit.cfg.py
>
> Creating 10 more copies of each of those seems a little irresponsible, 
> especially for tools that are "lightly maintained". So it's hard to say with 
> a straight face that restructuring them all is a good idea, and maybe this 
> motivated the original clang-tools-extra structure.
> But if those files were generated/eliminated somehow, restructuring would be 
> very simple patches that didn't add more cruft to maintain.
>
> It's not obvious to me exactly how to do this (lit.* files affect both 
> discovery and configuration) but I'll dig into this more tomorrow.

That's fair. It could be that we decide that clang-tools-extra is the dumping 
ground for lightly maintained one-off tools where it makes sense to share docs 
and tests, and other things that are expected to be actively maintained 
long-term (clang-tidy, clandg) should live at the top level of the monorepo as 
stand-alone tools. That may add more tension for where to add new projects 
though, so it may not be ideal either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233

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

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

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

xbolva00 added inline comments.



Comment at: flang/test/Driver/mllvm.f90:1
+! Test the `-mlvm` option
+

mllvm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121374

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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a subscriber: dexonsmith.
Herald added a project: All.
hokein requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

The function will be moved from clangFrontend to clangBasic, which
allows tools (clang pseudoparser)which don't depend on clangFrontend to use
this function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121375

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3111,149 +3111,7 @@
  const llvm::Triple &T,
  std::vector &Includes,
  LangStandard::Kind LangStd) {
-  // Set some properties which depend solely on the input kind; it would be nice
-  // to move these to the language standard, and have the driver resolve the
-  // input kind + language standard.
-  //
-  // FIXME: Perhaps a better model would be for a single source file to have
-  // multiple language standards (C / C++ std, ObjC std, OpenCL std, OpenMP std)
-  // simultaneously active?
-  if (IK.getLanguage() == Language::Asm) {
-Opts.AsmPreprocessor = 1;
-  } else if (IK.isObjectiveC()) {
-Opts.ObjC = 1;
-  }
-
-  if (LangStd == LangStandard::lang_unspecified) {
-// Based on the base language, pick one.
-switch (IK.getLanguage()) {
-case Language::Unknown:
-case Language::LLVM_IR:
-  llvm_unreachable("Invalid input kind!");
-case Language::OpenCL:
-  LangStd = LangStandard::lang_opencl12;
-  break;
-case Language::OpenCLCXX:
-  LangStd = LangStandard::lang_openclcpp10;
-  break;
-case Language::CUDA:
-  LangStd = LangStandard::lang_cuda;
-  break;
-case Language::Asm:
-case Language::C:
-#if defined(CLANG_DEFAULT_STD_C)
-  LangStd = CLANG_DEFAULT_STD_C;
-#else
-  // The PS4 uses C99 as the default C standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnu99;
-  else
-LangStd = LangStandard::lang_gnu17;
-#endif
-  break;
-case Language::ObjC:
-#if defined(CLANG_DEFAULT_STD_C)
-  LangStd = CLANG_DEFAULT_STD_C;
-#else
-  LangStd = LangStandard::lang_gnu11;
-#endif
-  break;
-case Language::CXX:
-case Language::ObjCXX:
-#if defined(CLANG_DEFAULT_STD_CXX)
-  LangStd = CLANG_DEFAULT_STD_CXX;
-#else
-  LangStd = LangStandard::lang_gnucxx14;
-#endif
-  break;
-case Language::RenderScript:
-  LangStd = LangStandard::lang_c99;
-  break;
-case Language::HIP:
-  LangStd = LangStandard::lang_hip;
-  break;
-}
-  }
-
-  const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd);
-  Opts.LangStd = LangStd;
-  Opts.LineComment = Std.hasLineComments();
-  Opts.C99 = Std.isC99();
-  Opts.C11 = Std.isC11();
-  Opts.C17 = Std.isC17();
-  Opts.C2x = Std.isC2x();
-  Opts.CPlusPlus = Std.isCPlusPlus();
-  Opts.CPlusPlus11 = Std.isCPlusPlus11();
-  Opts.CPlusPlus14 = Std.isCPlusPlus14();
-  Opts.CPlusPlus17 = Std.isCPlusPlus17();
-  Opts.CPlusPlus20 = Std.isCPlusPlus20();
-  Opts.CPlusPlus2b = Std.isCPlusPlus2b();
-  Opts.GNUMode = Std.isGNUMode();
-  Opts.GNUCVersion = 0;
-  Opts.HexFloats = Std.hasHexFloats();
-  Opts.ImplicitInt = Std.hasImplicitInt();
-
-  // Set OpenCL Version.
-  Opts.OpenCL = Std.isOpenCL();
-  if (LangStd == LangStandard::lang_opencl10)
-Opts.OpenCLVersion = 100;
-  else if (LangStd == LangStandard::lang_opencl11)
-Opts.OpenCLVersion = 110;
-  else if (LangStd == LangStandard::lang_opencl12)
-Opts.OpenCLVersion = 120;
-  else if (LangStd == LangStandard::lang_opencl20)
-Opts.OpenCLVersion = 200;
-  else if (LangStd == LangStandard::lang_opencl30)
-Opts.OpenCLVersion = 300;
-  else if (LangStd == LangStandard::lang_openclcpp10)
-Opts.OpenCLCPlusPlusVersion = 100;
-  else if (LangStd == LangStandard::lang_openclcpp2021)
-Opts.OpenCLCPlusPlusVersion = 202100;
-
-  // OpenCL has some additional defaults.
-  if (Opts.OpenCL) {
-Opts.AltiVec = 0;
-Opts.ZVector = 0;
-Opts.setDefaultFPContractMode(LangOptions::FPM_On);
-Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
-Opts.OpenCLPipes = Opts.getOpenCLCompatibleVersion() == 200;
-Opts.OpenCLGenericAddressSpace = Opts.getOpenCLCompatibleVersion() == 200;
-
-// Include default header file for OpenCL.
-if (Opts.IncludeDefaultHeader) {
-  if (Opts.DeclareOpenCLBuiltins) {
-// Only include base header file for builtin types and constants.
-Includes.push_back("opencl-c-base.h");
-  } else {
-  

[PATCH] D91000: [clang-tidy] Add cert-msc24-msc33-c checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely added a comment.
Herald added a project: All.

In D91000#3313892 , @balazske wrote:

> Now it would be better to have a checker called `UnsafeFunctionsCheck` 
> (probably in bugprone) and add the cert checkers "msc24-c" and "msc33-c" as 
> aliases. This makes the check extendable if more (CERT rule related or not) 
> cases for unsafe functions are added.

Done


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

https://reviews.llvm.org/D91000

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

[Fütő Gergely via Phabricator via cfe-commits]

futogergely updated this revision to Diff 414359.
futogergely retitled this revision from "[clang-tidy] Add cert-msc24-msc33-c 
checker." to "[clang-tidy] Add bugprone-unsafe-functions checker.".
futogergely added a comment.

Checker has been moved to bugprone.


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

https://reviews.llvm.org/D91000

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-usafe-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsafe-functions.c
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K%s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__
+// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__   -D__STDC_WANT_LIB_EXT1__=1
+// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY  %s bugprone-unsafe-functions %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \
+// RUN:-- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
+
+typedef __SIZE_TYPE__ size_t;
+typedef char wchar_t;
+
+char *gets(char *s);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+
+void f1(char *s) {
+  gets(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'fgets' should be used instead
+
+  strlen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+
+  wcslen(s);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead
+  // no-warning WITHOUT-ANNEX-K
+}
+
+struct tm;
+char *asctime(const struct tm *timeptr);
+
+void f2(const struct tm *timeptr) {
+  asctime(timeptr);
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr1)(const struct tm *) = asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITHOUT-ANNEX-K::[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+  char *(*f_ptr2)(const struct tm *) = &asctime;
+  // CHECK-MESSAGES-WITH-ANNEX-K:   :[[@LINE-1]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+  // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asc

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

[Shraiysh via Phabricator via cfe-commits]

shraiysh added a comment.

LGTM (will let others review as this includes changes to clang too and I am not 
familiar with it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121374

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

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/mllvm.f90:1
+! Test the `-mlvm` option
+

xbolva00 wrote:
> mllvm
Does it matter? `-mllvm` is consistent with other tests in this directory (e.g. 
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/emit-llvm.f90#L1).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121374

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

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added subscribers: phosek, mstorsjo.
mstorsjo added a comment.

FWIW I think D116689  interacts with this 
somewhat. I think D116689  is useful for the 
fixes I want to do wrt libcxxabi integration on Windows (but I haven't studied 
it, nor this one, closely yet though). @phosek Do you see anything in this one 
that isn't compatible with D116689 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120727

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

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

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

xbolva00 added inline comments.



Comment at: flang/test/Driver/mllvm.f90:1
+! Test the `-mlvm` option
+

awarzynski wrote:
> xbolva00 wrote:
> > mllvm
> Does it matter? `-mllvm` is consistent with other tests in this directory 
> (e.g. 
> https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/emit-llvm.f90#L1).
Just typo “mlvm”


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121374

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

[PATCH] D121374: [flang][driver] Add support for `-mllvm`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/mllvm.f90:1
+! Test the `-mlvm` option
+

xbolva00 wrote:
> awarzynski wrote:
> > xbolva00 wrote:
> > > mllvm
> > Does it matter? `-mllvm` is consistent with other tests in this directory 
> > (e.g. 
> > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/emit-llvm.f90#L1).
> Just typo “mlvm”
Sorry, I misread that :/ I thought that you meant `mllvm` instead of `-mllvm`. 
My bad, thanks for pointing this out! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121374

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/Format.cpp:2685
 const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

I'd rather see handling separately the `#import`/`#include` case on one hand 
and `@import` on the other.
IIUC, `@import` is followed by `ns1.ns2.something` without quotes nor `<>`. And 
it ends with a `;`.
Trying to put everything together is IMO error-prone and not very readable.

You could just do (pseudo-code):
```
CppIncludeRegexPattern = (current_regex) | "@\s*import\s+[^;]+;"
```
(`\s` is probably not supported, but we could use character class `[:space:]` 
for clarity, cf. llvm\lib\Support\regcomp.c)

Not sure whether whitespace is allowed after `@`.
Could you please post a link to the specification of `@import` syntax?

Not sure fixing `<"` / `">` issue is worth it, but in all cases I'd prefer 
seeing it in a different patch.



Comment at: clang/lib/Format/Format.cpp:2759
+  IncludeName =
+  StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+}

HazardyKnusperkeks wrote:
> How does that not go out of scope and I have a dangling reference?
Why not doing it the other way round, i.e. trimming `<>"` from include name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/Format.cpp:2685
 const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

curdeius wrote:
> I'd rather see handling separately the `#import`/`#include` case on one hand 
> and `@import` on the other.
> IIUC, `@import` is followed by `ns1.ns2.something` without quotes nor `<>`. 
> And it ends with a `;`.
> Trying to put everything together is IMO error-prone and not very readable.
> 
> You could just do (pseudo-code):
> ```
> CppIncludeRegexPattern = (current_regex) | "@\s*import\s+[^;]+;"
> ```
> (`\s` is probably not supported, but we could use character class `[:space:]` 
> for clarity, cf. llvm\lib\Support\regcomp.c)
> 
> Not sure whether whitespace is allowed after `@`.
> Could you please post a link to the specification of `@import` syntax?
> 
> Not sure fixing `<"` / `">` issue is worth it, but in all cases I'd prefer 
> seeing it in a different patch.
`[:space:]` would match newlines and we don't want this in all cases, but 
`[:blank:]` should be a good replacement for `[\t ]`.

On a different note, why there's a new line `\n` and backslash `\\` in `[\t\n\ 
\\]`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 414371.
jhuber6 added a comment.
Herald added subscribers: abrachet, phosek.

Fix architecture parsing and still include the GPU binary so cuobjcopy can use 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -71,8 +71,8 @@
   if (Args.hasArg(options::OPT_static))
 if (const Arg *A =
 Args.getLastArg(options::OPT_dynamic, options::OPT_mdynamic_no_pic))
-  D.Diag(diag::err_drv_argument_not_allowed_with) << A->getAsString(Args)
-  << "-static";
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "-static";
 }
 
 // Add backslashes to escape spaces and other backslashes.
@@ -157,8 +157,8 @@
 /// parameter in reciprocal argument strings. Return false if there is an error
 /// parsing the refinement step. Otherwise, return true and set the Position
 /// of the refinement step in the input string.
-static bool getRefinementStep(StringRef In, const Driver &D,
-  const Arg &A, size_t &Position) {
+static bool getRefinementStep(StringRef In, const Driver &D, const Arg &A,
+  size_t &Position) {
   const char RefinementStepToken = ':';
   Position = In.find(RefinementStepToken);
   if (Position != StringRef::npos) {
@@ -510,7 +510,7 @@
 }
 
 static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
-  switch (Triple.getArch()){
+  switch (Triple.getArch()) {
   default:
 return false;
   case llvm::Triple::arm:
@@ -705,7 +705,7 @@
 
 /// Add a CC1 and CC1AS option to specify the coverage file path prefix map.
 static void addCoveragePrefixMapArg(const Driver &D, const ArgList &Args,
-   ArgStringList &CmdArgs) {
+ArgStringList &CmdArgs) {
   for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
 options::OPT_fcoverage_prefix_map_EQ)) {
 StringRef Map = A->getValue();
@@ -801,13 +801,12 @@
   CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
 CSPGOGenerateArg = nullptr;
 
-  auto *ProfileGenerateArg = Args.getLastArg(
-  options::OPT_fprofile_instr_generate,
-  options::OPT_fprofile_instr_generate_EQ,
-  options::OPT_fno_profile_instr_generate);
-  if (ProfileGenerateArg &&
-  ProfileGenerateArg->getOption().matches(
-  options::OPT_fno_profile_instr_generate))
+  auto *ProfileGenerateArg =
+  Args.getLastArg(options::OPT_fprofile_instr_generate,
+  options::OPT_fprofile_instr_generate_EQ,
+  options::OPT_fno_profile_instr_generate);
+  if (ProfileGenerateArg && ProfileGenerateArg->getOption().matches(
+options::OPT_fno_profile_instr_generate))
 ProfileGenerateArg = nullptr;
 
   if (PGOGenerateArg && ProfileGenerateArg)
@@ -1334,8 +1333,8 @@
   }
 
   if (ThroughHeader.empty()) {
-Cm

[PATCH] D121165: [pseudo] Add crude heuristics to choose taken preprocessor branches.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/DirectiveMap.h:123
+/// The choices are stored in Conditional::Taken nodes.
+void chooseConditionalBranches(DirectiveMap &, const TokenStream &Code);
+

off-topic: a random idea on the name of `DirectiveMap` came up to my mind when 
reading the code, how about `DirectiveTree`, the comment of it says the 
structure is a tree, and the `BranchChooser` is actually a tree-visiting 
pattern.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Token.h:84
+  /// Returns the next token in the stream, skipping over comments.
+  const Token &next_nc() const {
+const Token *T = this;

nit: the name doesn't match the LLVM code-style.



Comment at: clang/lib/Tooling/Syntax/Pseudo/DirectiveMap.cpp:210
+namespace {
+class BranchChooser {
+public:

We evaluate each conditional directive *independently*, I wonder whether it is 
important to use any low-hanging tricks to ensure consistent choices are made 
among different conditional directives. (my gut feeling is that it is nice to 
have, but we should not worry too much about it at the moment).

For example,

```
#ifdef __cplusplus
extern "C" {
#endif

#ifdef __cplusplus
}
#endif
```

If we enable the first #if, and the second one should be enabled as well. (this 
example is a trivial and common pattern, it has been handled by the existing 
code).



Comment at: clang/lib/Tooling/Syntax/Pseudo/DirectiveMap.cpp:218
+  // Describes the code seen by making particular branch choices.
+  struct Score {
+unsigned Tokens = 0; // excluding comments and directives

nit: add a comment saying `higher is better`.



Comment at: clang/lib/Tooling/Syntax/Pseudo/DirectiveMap.cpp:225
+  // Seeing errors is bad, other things are good.
+  return std::tie(Other.Errors, Tokens, Directives) >
+ std::tie(Errors, Other.Tokens, Other.Directives);

Maybe `(-Errors, Tokens, Directives) > (-Other.Errors, ...)`



Comment at: clang/lib/Tooling/Syntax/Pseudo/DirectiveMap.cpp:295
+  // Is this the best branch so far? (Including if it's #if 1).
+  if (TookTrivial || !C.Taken || BranchScore > Best) {
+Best = BranchScore;

nit: `!C.Taken` => `!C.Token.hasValue()`, which is clearer.



Comment at: clang/lib/Tooling/Syntax/Pseudo/DirectiveMap.cpp:305
+  // false if the branch is never taken, and None otherwise.
+  llvm::Optional isTriviallyTaken(const DirectiveMap::Directive &Dir) {
+switch (Dir.Kind) {

nit: instead of using `trivially`, I think `confidently` fits better what the 
method does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121165

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

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D120727#3372594 , @mstorsjo wrote:

> FWIW I think D116689  interacts with this 
> somewhat. I think D116689  is useful for 
> the fixes I want to do wrt libcxxabi integration on Windows (but I haven't 
> studied it, nor this one, closely yet though). @phosek Do you see anything in 
> this one that isn't compatible with D116689 
> ?

It does interact a lot, in fact. If we land this patch, I suspect that the 
libc++abi parts of D116689  may not be 
necessary anymore. And I also had a plan (not concrete) to use a similar 
approach for deciding how the unwind library is picked up, which may be worth 
exploring. As far as I can tell, the main thing that is nicer with D116689 
's approach over this patch is that we can 
get rid of logic around `"-Wl,-force_load"` for merging the static library into 
libc++, but I think the approach proposed here would be compatible with that 
too.

The thing I like about this approach is that it simplifies the amount of logic 
we need by properly encoding dependencies in the `libcxx-abi-shared|static` 
targets.




Comment at: libcxx/src/CMakeLists.txt:233-239
 if (APPLE)
-  target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" 
"${LIBCXX_CXX_STATIC_ABI_LIBRARY}")
+  target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" 
"$")
 else()
-  target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" 
"${LIBCXX_CXX_STATIC_ABI_LIBRARY}" "-Wl,-Bdynamic,--no-whole-archive")
+  target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" 
"$" "-Wl,-Bdynamic,--no-whole-archive")
 endif()
   else()
+target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)

This part would become nicer if we had an `OBJECT` library like in D116689. I 
think this approach does not preclude improving this in the future by using an 
object library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120727

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

[PATCH] D119590: exclude openembedded distributions from setting rpath on openmp executables

[Khem Raj via Phabricator via cfe-commits]

raj.khem added a comment.
Herald added a project: All.

In D119590#3316050 , @JonChesterfield 
wrote:

> Cross compilers are a hazard here. I'd expect there to be a fairly long list 
> of magic flags you need to pass to clang to get it to find the right 
> libraries. Can you add fno-openmp-implicit-rpath to that list instead?

hmmm, I would say the original patch made assumption about native compile is 
the only option, clang claims to be inherently cross compiler.  Anyway adding 
`-fno-openmp-implicit-rpath` would mean that all SDKs generated by 
OpenEmbedded/Yocto project will have to somehow specify this option by default 
as well. it might work for system builds by specifying in global CFLAGS or 
adding to CC var itself.

> A better solution might be a cmake flag to specify where to use for the 
> implicit rpath directory instead of deriving it from sys::path::parent_path. 
> That would let your target set up a cross compiling toolchain that creates 
> binaries that are able to find libomp et al in whatever directory they're 
> located, without assuming a whole llvm toolchain installed onto the target.

right. Cmake flag route seems a good one. I will explore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119590

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

[PATCH] D121378: [clang][dataflow] Model the behavior of various optional members

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
sgatev requested review of this revision.
Herald added a project: clang.

Model `make_optional`, optional's default constructor, `emplace`,
`reset`, and `operator bool` members.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121378

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -30,8 +30,21 @@
 using ::testing::UnorderedElementsAre;
 
 static constexpr char StdTypeTraitsHeader[] = R"(
+#ifndef TYPE_TRAITS_H
+#define TYPE_TRAITS_H
+
 namespace std {
 
+typedef decltype(sizeof(char)) size_t;
+
+template 
+struct integral_constant {
+  static constexpr T value = V;
+};
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
 template< class T > struct remove_reference  {typedef T type;};
 template< class T > struct remove_reference  {typedef T type;};
 template< class T > struct remove_reference {typedef T type;};
@@ -39,21 +52,135 @@
 template 
   using remove_reference_t = typename remove_reference::type;
 
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct is_array : false_type {};
+
+template 
+struct is_array : true_type {};
+
+template 
+struct is_array : true_type {};
+
+template 
+struct is_function : false_type {};
+
+template 
+struct is_function : true_type {};
+
+namespace detail {
+
+template 
+struct type_identity {
+  using type = T;
+};  // or use type_identity (since C++20)
+
+template 
+auto try_add_pointer(int) -> type_identity::type*>;
+template 
+auto try_add_pointer(...) -> type_identity;
+
+}  // namespace detail
+
+template 
+struct add_pointer : decltype(detail::try_add_pointer(0)) {};
+
+template 
+struct conditional {
+  typedef T type;
+};
+
+template 
+struct conditional {
+  typedef F type;
+};
+
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+
+template 
+struct decay {
+ private:
+  typedef typename remove_reference::type U;
+
+ public:
+  typedef typename conditional<
+  is_array::value, typename remove_extent::type*,
+  typename conditional::value, typename add_pointer::type,
+   typename remove_cv::type>::type>::type type;
+};
+
 } // namespace std
+
+#endif // TYPE_TRAITS_H
 )";
 
 static constexpr char StdUtilityHeader[] = R"(
+#ifndef UTILITY_H
+#define UTILITY_H
+
 #include "std_type_traits.h"
 
 namespace std {
 
 template 
-constexpr std::remove_reference_t&& move(T&& x);
+constexpr remove_reference_t&& move(T&& x);
 
 } // namespace std
+
+#endif // UTILITY_H
+)";
+
+static constexpr char StdInitializerListHeader[] = R"(
+#ifndef INITIALIZER_LIST_H
+#define INITIALIZER_LIST_H
+
+namespace std {
+
+template 
+class initializer_list {
+ public:
+  initializer_list() noexcept;
+};
+
+} // namespace std
+
+#endif // INITIALIZER_LIST_H
 )";
 
 static constexpr char StdOptionalHeader[] = R"(
+#include "std_type_traits.h"
+#include "std_utility.h"
+#include "std_initializer_list.h"
+
 namespace std {
 
 template 
@@ -74,13 +201,41 @@
   const T&& value() const&&;
   T&& value() &&;
 
+  template 
+  constexpr T value_or(U&& v) const&;
+  template 
+  T value_or(U&& v) &&;
+
+  template 
+  T& emplace(Args&&... args);
+
+  template 
+  T& emplace(std::initializer_list ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
 };
 
+template 
+constexpr optional::type> make_optional(T&& v);
+
+template 
+constexpr optional make_optional(Args&&... args);
+
+template 
+constexpr optional make_optional(std::initializer_list il,
+Args&&... args);
+
 } // namespace std
 )";
 
 static constexpr char AbslOptionalHeader[] = R"(
+#include "std_type_traits.h"
+#include "std_utility.h"
+#include "std_initializer_list.h"
+
 namespace absl {
 
 template 
@@ -101,13 +256,41 @@
   const T&& value() const&&;
   T&& value() &&;
 
+  template 
+  constexpr T value_or(U&& v) const&;
+  template 
+  T value_or(U&& v) &&;
+
+  template 
+  T& emplace(Args&&... args);
+
+  template 
+  T& emplace(std::initializer_list ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explic

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D118511#3371432 , @tstellar wrote:

> I'm fine with reverting if you think this is the best solution.  I just would 
> like to conclude soon so I can make the final release candidate.

ISTM that reverting the ABI change in the 14.x branch makes sense, to avoid 
ping-ponging the ABI for packed structs which would become non-packed (breaking 
ABI) in 14.x and packed again (breaking ABI) in 
https://reviews.llvm.org/D119051.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D120989: Support debug info for alias variable

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

This looks mostly fine to me, I have a couple of superficial comments inline.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3914
 
+  // imported declaration
+  auto IE = ImportedDeclCache.find(D->getCanonicalDecl());

Nit: The LLVM coding style wants all comments to be full sentences, including a 
trailing `.`



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3921
+  return cast(GVE);
+return dyn_cast_or_null(N);
+  }

When would we enter a nullptr into the cache?



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:5496
+  if (!AliaseeDecl)
+/* FIXME: Aliasee not declared yet - possibly declared later
+ * For example,

See LLVM coding style



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1502
+const GlobalDecl CodeGenModule::getMangledNameDecl(StringRef Name) {
+  auto it = MangledDeclNames.begin();
+  while (it != MangledDeclNames.end()) {

std::find()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

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

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
zahiraam added reviewers: aaron.ballman, andrew.w.kaylor.
Herald added a project: All.
zahiraam requested review of this revision.
Herald added a project: clang.

Diagnose when `#pragma clang fp eval_method` doesn't have a supported value. 
Currently the compiler is crashing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121380

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Sema/fp-eval-pragma.cpp


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+ // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source' or 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+ // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source' or 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121378: [clang][dataflow] Model the behavior of various optional members

[Gábor Horváth via Phabricator via cfe-commits]

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

Thanks!




Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:64
 static BoolValue *getHasValue(Value *Val) {
-  if (auto *OptionalVal = cast_or_null(Val)) {
+  if (auto *OptionalVal = cast_or_null(Val))
 return cast(OptionalVal->getProperty("has_value"));

Nit: unintended change?



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:33
 static constexpr char StdTypeTraitsHeader[] = R"(
+#ifndef TYPE_TRAITS_H
+#define TYPE_TRAITS_H

I wonder if it is more sustainable in the long term to have these headers in a 
separate file and `#include` them here (and build the raw string literal with a 
macro if that is possible at all).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121378

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

[PATCH] D121378: [clang][dataflow] Model the behavior of various optional members

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 414396.
sgatev marked an inline comment as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121378

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -29,9 +29,23 @@
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
+// FIXME: Move header definitions in separate file(s).
 static constexpr char StdTypeTraitsHeader[] = R"(
+#ifndef TYPE_TRAITS_H
+#define TYPE_TRAITS_H
+
 namespace std {
 
+typedef decltype(sizeof(char)) size_t;
+
+template 
+struct integral_constant {
+  static constexpr T value = V;
+};
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
 template< class T > struct remove_reference  {typedef T type;};
 template< class T > struct remove_reference  {typedef T type;};
 template< class T > struct remove_reference {typedef T type;};
@@ -39,21 +53,135 @@
 template 
   using remove_reference_t = typename remove_reference::type;
 
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct remove_extent {
+  typedef T type;
+};
+
+template 
+struct is_array : false_type {};
+
+template 
+struct is_array : true_type {};
+
+template 
+struct is_array : true_type {};
+
+template 
+struct is_function : false_type {};
+
+template 
+struct is_function : true_type {};
+
+namespace detail {
+
+template 
+struct type_identity {
+  using type = T;
+};  // or use type_identity (since C++20)
+
+template 
+auto try_add_pointer(int) -> type_identity::type*>;
+template 
+auto try_add_pointer(...) -> type_identity;
+
+}  // namespace detail
+
+template 
+struct add_pointer : decltype(detail::try_add_pointer(0)) {};
+
+template 
+struct conditional {
+  typedef T type;
+};
+
+template 
+struct conditional {
+  typedef F type;
+};
+
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+template 
+struct remove_cv {
+  typedef T type;
+};
+
+template 
+struct decay {
+ private:
+  typedef typename remove_reference::type U;
+
+ public:
+  typedef typename conditional<
+  is_array::value, typename remove_extent::type*,
+  typename conditional::value, typename add_pointer::type,
+   typename remove_cv::type>::type>::type type;
+};
+
 } // namespace std
+
+#endif // TYPE_TRAITS_H
 )";
 
 static constexpr char StdUtilityHeader[] = R"(
+#ifndef UTILITY_H
+#define UTILITY_H
+
 #include "std_type_traits.h"
 
 namespace std {
 
 template 
-constexpr std::remove_reference_t&& move(T&& x);
+constexpr remove_reference_t&& move(T&& x);
+
+} // namespace std
+
+#endif // UTILITY_H
+)";
+
+static constexpr char StdInitializerListHeader[] = R"(
+#ifndef INITIALIZER_LIST_H
+#define INITIALIZER_LIST_H
+
+namespace std {
+
+template 
+class initializer_list {
+ public:
+  initializer_list() noexcept;
+};
 
 } // namespace std
+
+#endif // INITIALIZER_LIST_H
 )";
 
 static constexpr char StdOptionalHeader[] = R"(
+#include "std_initializer_list.h"
+#include "std_type_traits.h"
+#include "std_utility.h"
+
 namespace std {
 
 template 
@@ -74,13 +202,41 @@
   const T&& value() const&&;
   T&& value() &&;
 
+  template 
+  constexpr T value_or(U&& v) const&;
+  template 
+  T value_or(U&& v) &&;
+
+  template 
+  T& emplace(Args&&... args);
+
+  template 
+  T& emplace(std::initializer_list ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
 };
 
+template 
+constexpr optional::type> make_optional(T&& v);
+
+template 
+constexpr optional make_optional(Args&&... args);
+
+template 
+constexpr optional make_optional(std::initializer_list il,
+Args&&... args);
+
 } // namespace std
 )";
 
 static constexpr char AbslOptionalHeader[] = R"(
+#include "std_initializer_list.h"
+#include "std_type_traits.h"
+#include "std_utility.h"
+
 namespace absl {
 
 template 
@@ -101,13 +257,41 @@
   const T&& value() const&&;
   T&& value() &&;
 
+  template 
+  constexpr T value_or(U&& v) const&;
+  template 
+  T value_or(U&& v) &&;
+
+  template 
+  T& emplace(Args&&... args);
+
+  template 
+  T& emplace(std::initializer_list ilist, Args&&... args);
+
+  void reset() noexcept;
+
+  constexpr explicit operator bool() const noexcept;
   constexpr bool has_value

[PATCH] D121378: [clang][dataflow] Model the behavior of various optional members

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:33
 static constexpr char StdTypeTraitsHeader[] = R"(
+#ifndef TYPE_TRAITS_H
+#define TYPE_TRAITS_H

xazax.hun wrote:
> I wonder if it is more sustainable in the long term to have these headers in 
> a separate file and `#include` them here (and build the raw string literal 
> with a macro if that is possible at all).
Yeap, I agree that would be better. I'll consider some options and send a patch 
to move them out of here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121378

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

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

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

steakhal created this revision.
steakhal added reviewers: aaron.ballman, NoQ, Szelethus, whisperity, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

AFAIK we should prefer
https://clang.llvm.org/docs/analyzer/checkers.html to 
https://clang-analyzer.llvm.org/{available_checks,alpha_checks}.html

This patch will ensure that the doc urls produced by tablegen for the
ClangSA, will use the new url. Nothing else will be changed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121387

Files:
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/utils/TableGen/ClangSACheckersEmitter.cpp

Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ clang/utils/TableGen/ClangSACheckersEmitter.cpp
@@ -24,37 +24,39 @@
 // Static Analyzer Checkers Tables generation
 //===--===//
 
-static std::string getPackageFullName(const Record *R);
+static std::string getPackageFullName(const Record *R, StringRef Sep = ".");
 
-static std::string getParentPackageFullName(const Record *R) {
-  std::string name;
+static std::string getParentPackageFullName(const Record *R,
+StringRef Sep = ".") {
   if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage")))
-name = getPackageFullName(DI->getDef());
-  return name;
+return getPackageFullName(DI->getDef());
+  return "";
 }
 
-static std::string getPackageFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
-  if (!name.empty())
-name += ".";
-  assert(!R->getValueAsString("PackageName").empty());
-  name += R->getValueAsString("PackageName");
-  return name;
+static std::string getPackageFullName(const Record *R, StringRef Sep) {
+  std::string ParentPkgName = getParentPackageFullName(R, Sep);
+  StringRef PackageName = R->getValueAsString("PackageName");
+  assert(!PackageName.empty());
+
+  if (ParentPkgName.empty())
+return PackageName.str();
+  return (Twine{ParentPkgName} + Sep + PackageName).str();
 }
 
-static std::string getCheckerFullName(const Record *R) {
-  std::string name = getParentPackageFullName(R);
-  if (!name.empty())
-name += ".";
-  assert(!R->getValueAsString("CheckerName").empty());
-  name += R->getValueAsString("CheckerName");
-  return name;
+static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") {
+  std::string ParentPkgName = getParentPackageFullName(R, Sep);
+  StringRef CheckerName = R->getValueAsString("CheckerName");
+  assert(!CheckerName.empty());
+
+  if (ParentPkgName.empty())
+return CheckerName.str();
+  return (Twine{ParentPkgName} + Sep + CheckerName).str();
 }
 
 static std::string getStringValue(const Record &R, StringRef field) {
   if (StringInit *SI = dyn_cast(R.getValueInit(field)))
 return std::string(SI->getValue());
-  return std::string();
+  return "";
 }
 
 // Calculates the integer value representing the BitsInit object
@@ -74,20 +76,19 @@
 }
 
 static std::string getCheckerDocs(const Record &R) {
-  StringRef LandingPage;
-  if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) {
-uint64_t V = getValueFromBitsInit(BI, R);
-if (V == 1)
-  LandingPage = "available_checks.html";
-else if (V == 2)
-  LandingPage = "alpha_checks.html";
-  }
-  
-  if (LandingPage.empty())
+  const BitsInit *BI = R.getValueAsBitsInit("Documentation");
+  if (!BI)
+PrintFatalError(R.getLoc(),
+"missing Documentation for " + getCheckerFullName(&R));
+
+  // Ignore 'Documentation' checkers.
+  if (getValueFromBitsInit(BI, R) == 0)
 return "";
 
-  return (llvm::Twine("https://clang-analyzer.llvm.org/";) + LandingPage + "#" +
-  getCheckerFullName(&R))
+  std::string CheckerFullName = StringRef(getCheckerFullName(&R, "-")).lower();
+
+  return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#";) +
+  CheckerFullName)
   .str();
 }
 
Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -451,7 +451,7 @@
   "fullDescription": {
 "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)"
   },
-  "helpUri": "https://clang-an

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Fixes in LLVM require tests in LLVM - probably taking the clang test and 
compiling that to llvm IR (include the original C++ source in a comment in the 
IR test case) and then testing it in LLVM instead of clang.

Also looks like the test could be simplified a bit more:

  void f1();
  
  template 
  void f2() {
f1();
  }
  
  void f3() {
f2();
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120610

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

[PATCH] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Yeah, happy to hear other perspectives - but my rough reaction is similar: 
putting mangled names in the "name" field seems problematic (consumer wouldn't 
necessarily know that the name should be demangled, for instance? Maybe?). So 
at the IR level maybe it's better to leave those in the linkage name - and 
maybe we workaround it in the backend when tuning for gdb - putting it in the 
DW_AT_name field because it's not ideal but the best we can do for GDB? (anyone 
filed a bug with GDB and/or DWARF for this? (I'd start with GDB and see if 
they're open to the idea directly - don't use the DWARF committee as an 
indirect way to request feature work for GDB unless they'd prefer it to be 
formalized before implementing it))

The various _ and __ names are probably OK as names, but the tests that end up 
producing mangled names into linkage names seem like not the right direction - 
maybe those entities need/are missing real names anyway? Might be worth 
comparing/contrasting with GCC's behavior for similar entities (at a glance at 
the tests, looks like some thunks are in this situation?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121100

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

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[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 with a tiny tweak to the diagnostic.




Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1493
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source' or 'double' or 'extended'}2">;
 

Slight tweak here to use a list instead of multiple `or`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121380

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

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Danny Mösch via Phabricator via cfe-commits]

SimplyDanny added a comment.

I'm happy that you found a reasonable compromise. I like it too. ;)

Now, I ask you to help me a little bit with the workflow and the test failures. 
The review comments are all taken care of as far as I see. One reviewer 
approved the changes, others are still in a "needs changes"  or an undecided 
state. Are approvals of all reviewers required? I guess, the test failures have 
nothing to do with my changes, or have they? Can we just ignore them if they 
are unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D118511#3372728 , @jyknight wrote:

> In D118511#3371432 , @tstellar 
> wrote:
>
>> I'm fine with reverting if you think this is the best solution.  I just 
>> would like to conclude soon so I can make the final release candidate.
>
> ISTM that reverting the ABI change in the 14.x branch makes sense, to avoid 
> ping-ponging the ABI for packed structs which would become non-packed 
> (breaking ABI) in 14.x and packed again (breaking ABI) in 
> https://reviews.llvm.org/D119051.

Yeah - I think it'd be a pretty niche amount of code that'd churn like that, 
but doesn't seem super important to rush this either.

@tstellar - can/do you want to revert this on the release branch yourself? Is 
that something I should do? Should I revert this on trunk (would be a bit 
awkward/more churny for users - maybe not a full revert, but one that leaves 
the new ABI version flag available as a no-op so users opting out don't need to 
remove the flag only to add it back in later) so it can be integrated to the 
release?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 414416.

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

https://reviews.llvm.org/D121380

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Sema/fp-eval-pragma.cpp


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+ // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+ // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D121078#3373081 , @SimplyDanny 
wrote:

> I'm happy that you found a reasonable compromise. I like it too. ;)
>
> Now, I ask you to help me a little bit with the workflow and the test 
> failures. The review comments are all taken care of as far as I see. One 
> reviewer approved the changes, others are still in a "needs changes"  or an 
> undecided state. Are approvals of all reviewers required?

You don't need all reviewers to approve a patch before landing it, but you do 
need to determine if there's consensus amongst the active reviewers that it's 
okay to land.

I think all the review comments have been addressed here, as far as I can tell, 
but it's usually good to double-check just to be sure when someone has 
explicitly marked the review as requesting changes. I usually do that by 
pinging the people who marked as requesting changes (or anyone else who I'm 
uncertain about) and giving them a day or two to respond. If there are no 
responses after a few days, land the changes. If it turns out there are still 
comments they'd like to see addressed after the patch landed, they can 
generally be handled post commit (or if they're a major concern, we can always 
revert the changes while debating what to do). A patch is not blocked by people 
who are in an undecided state and haven't been active in the review at all.

@ldionne @tonic -- do things look reasonable to you now?

(btw, we have some documentation on that, in case you're not seen it: 
https://llvm.org/docs/CodeReview.html#code-review-workflow)

> I guess, the test failures have nothing to do with my changes, or have they? 
> Can we just ignore them if they are unrelated?

I looked at the test failures and they look unrelated, so it's fine to ignore 
them.

Btw, do you have commit rights for the project? If not, what name and email 
address would you like used for patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D121078#3373081 , @SimplyDanny 
wrote:

> I'm happy that you found a reasonable compromise. I like it too. ;)
>
> Now, I ask you to help me a little bit with the workflow and the test 
> failures. The review comments are all taken care of as far as I see. One 
> reviewer approved the changes, others are still in a "needs changes"  or an 
> undecided state. Are approvals of all reviewers required? I guess, the test 
> failures have nothing to do with my changes, or have they? Can we just ignore 
> them if they are unrelated?

Please wait for libunwind and libc++ approval. I won't approve, because I'm not 
familiar with the documentation stuff.
If the test failures are unrelated you can ignore them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2733
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  return llvm::any_of(Node.getAttrs(),

sammccall wrote:
> This definitely seems more like `hasAttr` than `isAttr` to me. An 
> AttributedStmt *is* the (sugar) statement, and *has* the attribute(s).
> 
> Maybe rather `describesAttr` so people don't confuse this for one that walks 
> up, though?
Good point on the `isAttr` name!

I'm not sure `describeAttr` works (the statement doesn't describe attributes, 
it... has/contains/uses the attribute). Maybe.. `specifiesAttr()`?

Or are we making this harder than it needs to be and we should use `hasAttr()` 
for parity with other things that have attributes associated with them?



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2713-2714
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+

sammccall wrote:
> jdoerfert wrote:
> > aaron.ballman wrote:
> > > Design-wise, I'm on the fence here. AST matchers match the AST nodes that 
> > > Clang produces, and from that perspective, this makes a lot of sense. But 
> > > `AttributedStmt` is a bit of a hack in some ways, and do we want to 
> > > expose that hack to AST matcher users? e.g., is there a reason we 
> > > shouldn't make `returnStmt(hasAttr(attr::Likely))` work directly rather 
> > > than making the user pick their way through the `AttributedStmt`? This is 
> > > more in line with how you check for a declaration with a particular 
> > > attribute and seems like the more natural way to surface this.
> > > 
> > > For the moment, since this is following the current AST structure in 
> > > Clang, I think this is fine. But I'm curious if @klimek or perhaps 
> > > @sammccall has an opinion here.
> > I think a way to find any kind of statement (or expression) that has a 
> > specific attribute is very useful. How that should look, idk.
> TL;DR: I think this matcher fits the current design best. 
> `returnStmt(hasAttr())` sounds awesome, but I think it's a significant new 
> concept, and a cross-cutting project like traversal modes.
> 
> ---
> 
> returnStmt(hasAttr()) is definitely nicer in isolation (and in combination 
> with how decls work).
> It's definitely potentially confusing to not match the AST. The AST is 
> unlikely to be fixed because (IIUC) we don't want to burden each stmt with 
> tracking if it has attrs.
> So I think the easy answer is this patch gets it right.
> 
> The inconsistency with decls is annoying, and maybe worse it doesn't yield a 
> simple way to express "a return statement, maybe with an attribute, I don't 
> care" unless you're searching recursively anyway, and this should almost be 
> the default.
> `returnStmt(hasAttr())` suggests that it would enable this, maybe by skipping 
> over the AttributedStmt with some fancy traversal mode, and then looking it 
> up again in the hasAttr matcher from the parent map.
> 
> I think this would be a less brittle way to handle mostly-transparent nodes 
> that you nevertheless want to be able to match the contents of sometimes. The 
> current options (explicitly unwrap, rely on recursive search, and traversal 
> modes) all seem to have significant limitations.
> However this is a pretty general idea (and I guess a pretty large project), 
> and I don't think it's worth introducing just for AttributedStmt.
Thanks for the feedback Sam!

> The AST is unlikely to be fixed because (IIUC) we don't want to burden each 
> stmt with tracking if it has attrs.

I'm less convinced of this. We didn't want to do it originally because there 
were so very few statement attributes. These days, there's quite a few more 
more statement attributes, so we may very well revisit this. `AttributedStmt` 
is a pain that has caused us problems in the past with things like 
`isa()` failing because it didn't expect an attributed `FooStmt`.

That said, the rest of your points are compelling, so this matcher is fine for 
me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

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

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 414424.

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

https://reviews.llvm.org/D121380

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Sema/fp-eval-pragma.cpp


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D118511#3373082 , @dblaikie wrote:

> In D118511#3372728 , @jyknight 
> wrote:
>
>> In D118511#3371432 , @tstellar 
>> wrote:
>>
>>> I'm fine with reverting if you think this is the best solution.  I just 
>>> would like to conclude soon so I can make the final release candidate.
>>
>> ISTM that reverting the ABI change in the 14.x branch makes sense, to avoid 
>> ping-ponging the ABI for packed structs which would become non-packed 
>> (breaking ABI) in 14.x and packed again (breaking ABI) in 
>> https://reviews.llvm.org/D119051.
>
> Yeah - I think it'd be a pretty niche amount of code that'd churn like that, 
> but doesn't seem super important to rush this either.
>
> @tstellar - can/do you want to revert this on the release branch yourself? Is 
> that something I should do? Should I revert this on trunk (would be a bit 
> awkward/more churny for users - maybe not a full revert, but one that leaves 
> the new ABI version flag available as a no-op so users opting out don't need 
> to remove the flag only to add it back in later) so it can be integrated to 
> the release?

I can revert in the release branch.  Is this the commit: 
https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6?

It doesn't seem necessary to me to revert in the main branch, but I think that 
can be your call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2713-2714
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+

aaron.ballman wrote:
> sammccall wrote:
> > jdoerfert wrote:
> > > aaron.ballman wrote:
> > > > Design-wise, I'm on the fence here. AST matchers match the AST nodes 
> > > > that Clang produces, and from that perspective, this makes a lot of 
> > > > sense. But `AttributedStmt` is a bit of a hack in some ways, and do we 
> > > > want to expose that hack to AST matcher users? e.g., is there a reason 
> > > > we shouldn't make `returnStmt(hasAttr(attr::Likely))` work directly 
> > > > rather than making the user pick their way through the 
> > > > `AttributedStmt`? This is more in line with how you check for a 
> > > > declaration with a particular attribute and seems like the more natural 
> > > > way to surface this.
> > > > 
> > > > For the moment, since this is following the current AST structure in 
> > > > Clang, I think this is fine. But I'm curious if @klimek or perhaps 
> > > > @sammccall has an opinion here.
> > > I think a way to find any kind of statement (or expression) that has a 
> > > specific attribute is very useful. How that should look, idk.
> > TL;DR: I think this matcher fits the current design best. 
> > `returnStmt(hasAttr())` sounds awesome, but I think it's a significant new 
> > concept, and a cross-cutting project like traversal modes.
> > 
> > ---
> > 
> > returnStmt(hasAttr()) is definitely nicer in isolation (and in combination 
> > with how decls work).
> > It's definitely potentially confusing to not match the AST. The AST is 
> > unlikely to be fixed because (IIUC) we don't want to burden each stmt with 
> > tracking if it has attrs.
> > So I think the easy answer is this patch gets it right.
> > 
> > The inconsistency with decls is annoying, and maybe worse it doesn't yield 
> > a simple way to express "a return statement, maybe with an attribute, I 
> > don't care" unless you're searching recursively anyway, and this should 
> > almost be the default.
> > `returnStmt(hasAttr())` suggests that it would enable this, maybe by 
> > skipping over the AttributedStmt with some fancy traversal mode, and then 
> > looking it up again in the hasAttr matcher from the parent map.
> > 
> > I think this would be a less brittle way to handle mostly-transparent nodes 
> > that you nevertheless want to be able to match the contents of sometimes. 
> > The current options (explicitly unwrap, rely on recursive search, and 
> > traversal modes) all seem to have significant limitations.
> > However this is a pretty general idea (and I guess a pretty large project), 
> > and I don't think it's worth introducing just for AttributedStmt.
> Thanks for the feedback Sam!
> 
> > The AST is unlikely to be fixed because (IIUC) we don't want to burden each 
> > stmt with tracking if it has attrs.
> 
> I'm less convinced of this. We didn't want to do it originally because there 
> were so very few statement attributes. These days, there's quite a few more 
> more statement attributes, so we may very well revisit this. `AttributedStmt` 
> is a pain that has caused us problems in the past with things like 
> `isa()` failing because it didn't expect an attributed `FooStmt`.
> 
> That said, the rest of your points are compelling, so this matcher is fine 
> for me.
> We didn't want to do it originally because there were so very few statement 
> attributes.

Ah, i assumed it was some kind of issue with size or the logistics of 
allocation. Fixing the AST sounds good then! (But I wouldn't block this patch 
on it unless someone's ready to work on it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

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

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2713-2714
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+

sammccall wrote:
> aaron.ballman wrote:
> > sammccall wrote:
> > > jdoerfert wrote:
> > > > aaron.ballman wrote:
> > > > > Design-wise, I'm on the fence here. AST matchers match the AST nodes 
> > > > > that Clang produces, and from that perspective, this makes a lot of 
> > > > > sense. But `AttributedStmt` is a bit of a hack in some ways, and do 
> > > > > we want to expose that hack to AST matcher users? e.g., is there a 
> > > > > reason we shouldn't make `returnStmt(hasAttr(attr::Likely))` work 
> > > > > directly rather than making the user pick their way through the 
> > > > > `AttributedStmt`? This is more in line with how you check for a 
> > > > > declaration with a particular attribute and seems like the more 
> > > > > natural way to surface this.
> > > > > 
> > > > > For the moment, since this is following the current AST structure in 
> > > > > Clang, I think this is fine. But I'm curious if @klimek or perhaps 
> > > > > @sammccall has an opinion here.
> > > > I think a way to find any kind of statement (or expression) that has a 
> > > > specific attribute is very useful. How that should look, idk.
> > > TL;DR: I think this matcher fits the current design best. 
> > > `returnStmt(hasAttr())` sounds awesome, but I think it's a significant 
> > > new concept, and a cross-cutting project like traversal modes.
> > > 
> > > ---
> > > 
> > > returnStmt(hasAttr()) is definitely nicer in isolation (and in 
> > > combination with how decls work).
> > > It's definitely potentially confusing to not match the AST. The AST is 
> > > unlikely to be fixed because (IIUC) we don't want to burden each stmt 
> > > with tracking if it has attrs.
> > > So I think the easy answer is this patch gets it right.
> > > 
> > > The inconsistency with decls is annoying, and maybe worse it doesn't 
> > > yield a simple way to express "a return statement, maybe with an 
> > > attribute, I don't care" unless you're searching recursively anyway, and 
> > > this should almost be the default.
> > > `returnStmt(hasAttr())` suggests that it would enable this, maybe by 
> > > skipping over the AttributedStmt with some fancy traversal mode, and then 
> > > looking it up again in the hasAttr matcher from the parent map.
> > > 
> > > I think this would be a less brittle way to handle mostly-transparent 
> > > nodes that you nevertheless want to be able to match the contents of 
> > > sometimes. The current options (explicitly unwrap, rely on recursive 
> > > search, and traversal modes) all seem to have significant limitations.
> > > However this is a pretty general idea (and I guess a pretty large 
> > > project), and I don't think it's worth introducing just for 
> > > AttributedStmt.
> > Thanks for the feedback Sam!
> > 
> > > The AST is unlikely to be fixed because (IIUC) we don't want to burden 
> > > each stmt with tracking if it has attrs.
> > 
> > I'm less convinced of this. We didn't want to do it originally because 
> > there were so very few statement attributes. These days, there's quite a 
> > few more more statement attributes, so we may very well revisit this. 
> > `AttributedStmt` is a pain that has caused us problems in the past with 
> > things like `isa()` failing because it didn't expect an attributed 
> > `FooStmt`.
> > 
> > That said, the rest of your points are compelling, so this matcher is fine 
> > for me.
> > We didn't want to do it originally because there were so very few statement 
> > attributes.
> 
> Ah, i assumed it was some kind of issue with size or the logistics of 
> allocation. Fixing the AST sounds good then! (But I wouldn't block this patch 
> on it unless someone's ready to work on it).
Yeah, I'm not ready to work on it and I don't know of anyone else planning to 
do that work any time soon (it's more of an idle "someday" task than anything 
we need to do immediately). So definitely agreed, let's not block this patch on 
it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

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

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D118511#3373173 , @tstellar wrote:

> In D118511#3373082 , @dblaikie 
> wrote:
>
>> In D118511#3372728 , @jyknight 
>> wrote:
>>
>>> In D118511#3371432 , @tstellar 
>>> wrote:
>>>
 I'm fine with reverting if you think this is the best solution.  I just 
 would like to conclude soon so I can make the final release candidate.
>>>
>>> ISTM that reverting the ABI change in the 14.x branch makes sense, to avoid 
>>> ping-ponging the ABI for packed structs which would become non-packed 
>>> (breaking ABI) in 14.x and packed again (breaking ABI) in 
>>> https://reviews.llvm.org/D119051.
>>
>> Yeah - I think it'd be a pretty niche amount of code that'd churn like that, 
>> but doesn't seem super important to rush this either.
>>
>> @tstellar - can/do you want to revert this on the release branch yourself? 
>> Is that something I should do? Should I revert this on trunk (would be a bit 
>> awkward/more churny for users - maybe not a full revert, but one that leaves 
>> the new ABI version flag available as a no-op so users opting out don't need 
>> to remove the flag only to add it back in later) so it can be integrated to 
>> the release?
>
> I can revert in the release branch.  Is this the commit: 
> https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6?

Yep, that's the one!

> It doesn't seem necessary to me to revert in the main branch, but I think 
> that can be your call.

Yeah, I'm leaning towards not reverting on main & hoping we can sort out the 
POD ABI issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:258
+  // Fast-math is enabled.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+PP.setCurrentFPEvalMethod(SourceLocation(),

Shouldn't this be looking at `getLangOpts().FastMath`?


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

https://reviews.llvm.org/D121122

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

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Danny Mösch via Phabricator via cfe-commits]

SimplyDanny added a comment.

In D121078#3373139 , @philnik wrote:

> In D121078#3373081 , @SimplyDanny 
> wrote:
>
>> I'm happy that you found a reasonable compromise. I like it too. ;)
>>
>> Now, I ask you to help me a little bit with the workflow and the test 
>> failures. The review comments are all taken care of as far as I see. One 
>> reviewer approved the changes, others are still in a "needs changes"  or an 
>> undecided state. Are approvals of all reviewers required? I guess, the test 
>> failures have nothing to do with my changes, or have they? Can we just 
>> ignore them if they are unrelated?
>
> Please wait for libunwind and libc++ approval. I won't approve, because I'm 
> not familiar with the documentation stuff.
> If the test failures are unrelated you can ignore them.

Okay, I'll wait a few more days and probably ping people again if there is no 
response until then,

In D121078#3373128 , @aaron.ballman 
wrote:

> Btw, do you have commit rights for the project? If not, what name and email 
> address would you like used for patch attribution?

I don't have commit rights. Please use "Danny Mösch" with the email address 
"danny.moe...@icloud.com". Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:258
+  // Fast-math is enabled.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+PP.setCurrentFPEvalMethod(SourceLocation(),

aaron.ballman wrote:
> Shouldn't this be looking at `getLangOpts().FastMath`?
when the -ffast-math is enabled on the command line, it triggers all these math 
driver options:

"-menable-no-infs" "-menable-no-nans" "-fapprox-func" "-menable-unsafe-fp-math" 
"-fno-signed-zeros" "-mreassociate" "-freciprocal-math" "-ffp-contract=fast"  
"-ffast-math" "-ffinite-math-only"

That's a bit restrictive. We want the eval-method set to -1 when either reassoc 
or allowrecip are enabled.




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

https://reviews.llvm.org/D121122

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

[PATCH] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Ideally, this would be fixed in GDB. If that's really not feasibly I would 
rather like to see this as a gdb debugger tuning instead f the default behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121100

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

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:258
+  // Fast-math is enabled.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+PP.setCurrentFPEvalMethod(SourceLocation(),

zahiraam wrote:
> aaron.ballman wrote:
> > Shouldn't this be looking at `getLangOpts().FastMath`?
> when the -ffast-math is enabled on the command line, it triggers all these 
> math driver options:
> 
> "-menable-no-infs" "-menable-no-nans" "-fapprox-func" 
> "-menable-unsafe-fp-math" "-fno-signed-zeros" "-mreassociate" 
> "-freciprocal-math" "-ffp-contract=fast"  "-ffast-math" "-ffinite-math-only"
> 
> That's a bit restrictive. We want the eval-method set to -1 when either 
> reassoc or allowrecip are enabled.
> 
> 
Okay, then I think the comments about fast math should be fixed up; otherwise 
that's going to get confusing.


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

https://reviews.llvm.org/D121122

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

[PATCH] D121269: [clang-format] Fix namespace format when the name is followed by a macro

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 414442.
zequanwu marked 7 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121269

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -189,6 +189,49 @@
 "int i;\n"
 "int j;\n"
 "}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace A M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A M(x)",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ(
+  "#define B __attribute__((availability(macos, introduced=10.15)))\n"
+  "namespace A B {\n"
+  "int i;\n"
+  "int j;\n"
+  "}// namespace A B",
+  fixNamespaceEndComments(
+  "#define B __attribute__((availability(macos, introduced=10.15)))\n"
+  "namespace A B {\n"
+  "int i;\n"
+  "int j;\n"
+  "}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::B M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::B",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::B M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ(
+  "namespace A __attribute__((availability(macos, introduced=10.15))) {\n"
+  "int i;\n"
+  "int j;\n"
+  "}// namespace A",
+  fixNamespaceEndComments(
+  "namespace A __attribute__((availability(macos, introduced=10.15))) "
+  "{\n"
+  "int i;\n"
+  "int j;\n"
+  "}"));
   EXPECT_EQ("inline namespace A {\n"
 "int i;\n"
 "int j;\n"
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "NamespaceEndCommentsFixer.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Regex.h"
 
@@ -22,6 +23,40 @@
 namespace format {
 
 namespace {
+// Iterates all tokens starting from StartTok to EndTok and apply Fn to all
+// tokens between them including StartTok and EndTok. Returns the token after
+// EndTok.
+const FormatToken *
+processTokens(const FormatToken *Tok, tok::TokenKind StartTok,
+  tok::TokenKind EndTok,
+  llvm::function_ref Fn) {
+  if (!Tok || Tok->isNot(StartTok))
+return Tok;
+  int NestLevel = 0;
+  do {
+if (Tok->is(StartTok))
+  ++NestLevel;
+else if (Tok->is(EndTok))
+  --NestLevel;
+if (Fn)
+  Fn(Tok);
+Tok = Tok->getNextNonComment();
+  } while (Tok && NestLevel > 0);
+  return Tok;
+}
+
+const FormatToken *skipAttribute(const FormatToken *Tok) {
+  if (!Tok)
+return nullptr;
+  if (Tok->is(tok::kw___attribute)) {
+Tok = Tok->getNextNonComment();
+Tok = processTokens(Tok, tok::l_paren, tok::r_paren, nullptr);
+  } else if (Tok->is(tok::l_square)) {
+Tok = processTokens(Tok, tok::l_square, tok::r_square, nullptr);
+  }
+  return Tok;
+}
+
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
 std::string computeName(const FormatToken *NamespaceTok) {
@@ -39,48 +74,69 @@
   name += Tok->TokenText;
   Tok = Tok->getNextNonComment();
 }
-  } else {
-// Skip attributes.
-if (Tok && Tok->is(tok::l_square)) {
-  for (int NestLevel = 1; NestLevel > 0;) {
-Tok = Tok->getNextNonComment();
-if (!Tok)
-  break;
-if (Tok->is(tok::l_square))
-  ++NestLevel;
-else if (Tok->is(tok::r_square))
-  --NestLevel;
-  }
-  if (Tok)
-Tok = Tok->getNextNonComment();
-}
+return name;
+  }
+  Tok = skipAttribute(Tok);
 
-// Use the string after `namespace` as a name candidate until `{` or `::` or
-// `(`. If the name is empty, use the candicate.
-std::string FirstNSName;
-// For `namespace [[foo]] A::B::inline C {` or
-// `namespace MACRO1 MACRO2 A::B::inline C {`, returns "A::B::inline C".
-// Peek for the first '::' (or '{' or '(')

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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

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

https://reviews.llvm.org/D121122

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/eval-method-fast-math.c

Index: clang/test/CodeGen/eval-method-fast-math.c
===
--- /dev/null
+++ clang/test/CodeGen/eval-method-fast-math.c
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
+// RUN: | FileCheck %s -check-prefixes=CHECK
+
+// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
+// RUN: -ffast-math -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
+// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+float res;
+int add(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK: ret i32 0
+  // CHECK-EXT: ret i32 2
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+int add_precise(float a, float b, float c) {
+#pragma float_control(precise, on)
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+#pragma float_control(precise, on)
+int add_precise_1(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int add_not_precise(float a, float b, float c) {
+  // Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+// Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+int add_not_precise_1(float a, float b, float c) {
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int getFPEvalMethod() {
+  // CHECK: ret i32 0
+  return __FLT_EVAL_METHOD__;
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -517,6 +517,9 @@
 else
   NewFPFeatures.setFPPreciseEnabled(false);
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
+// `AllowFPReassoc` or `AllowReciprocal` option is enabled.
+PP.setCurrentFPEvalMethod(
+Loc, LangOptions::FPEvalMethodKind::FEM_Indeterminable);
 break;
   case PFC_Except:
 if (!isPreciseFPEnabled())
@@ -540,6 +543,12 @@
 }
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
 NewFPFeatures = FpPragmaStack.CurrentValue;
+if (CurFPFeatures.getAllowFPReassociate() ||
+CurFPFeatures.getAllowReciprocal())
+  // Since we are popping the pragma, we don't want to be passing
+  // a location here.
+  PP.setCurrentFPEvalMethod(SourceLocation(),
+CurFPFeatures.getFPEvalMethod());
 break;
   }
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -254,6 +254,12 @@
 PP.setCurrentFPEvalMethod(SourceLocation(),
   getLangOpts().getFPEvalMethod());
   CurFPFeatures.setFPEvalMethod(PP.getCurrentFPEvalMethod());
+  // When `-ffast-math` option is enabled, it triggers several driver math
+  // options to be enabled. Among these, only one the following two modes
+  // affect the eval-method:  reciprocal or reassociate.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+PP.setCurrentFPEvalMethod(SourceLocation(),
+  LangOptions::FEM_Indeterminable);
 }
 
 // Anchor Sema's type info to this TU.
Index: clang/lib/Lex/PPMacroExpansion.cpp

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 414461.

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

https://reviews.llvm.org/D121122

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/eval-method-fast-math.c

Index: clang/test/CodeGen/eval-method-fast-math.c
===
--- /dev/null
+++ clang/test/CodeGen/eval-method-fast-math.c
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
+// RUN: | FileCheck %s -check-prefixes=CHECK
+
+// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
+// RUN: -ffast-math -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
+// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+float res;
+int add(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK: ret i32 0
+  // CHECK-EXT: ret i32 2
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+int add_precise(float a, float b, float c) {
+#pragma float_control(precise, on)
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+#pragma float_control(precise, on)
+int add_precise_1(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int add_not_precise(float a, float b, float c) {
+  // Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+// Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+int add_not_precise_1(float a, float b, float c) {
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int getFPEvalMethod() {
+  // CHECK: ret i32 0
+  return __FLT_EVAL_METHOD__;
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -517,6 +517,9 @@
 else
   NewFPFeatures.setFPPreciseEnabled(false);
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
+// `AllowFPReassoc` or `AllowReciprocal` option is enabled.
+PP.setCurrentFPEvalMethod(
+Loc, LangOptions::FPEvalMethodKind::FEM_Indeterminable);
 break;
   case PFC_Except:
 if (!isPreciseFPEnabled())
@@ -540,6 +543,12 @@
 }
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
 NewFPFeatures = FpPragmaStack.CurrentValue;
+if (CurFPFeatures.getAllowFPReassociate() ||
+CurFPFeatures.getAllowReciprocal())
+  // Since we are popping the pragma, we don't want to be passing
+  // a location here.
+  PP.setCurrentFPEvalMethod(SourceLocation(),
+CurFPFeatures.getFPEvalMethod());
 break;
   }
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -254,6 +254,12 @@
 PP.setCurrentFPEvalMethod(SourceLocation(),
   getLangOpts().getFPEvalMethod());
   CurFPFeatures.setFPEvalMethod(PP.getCurrentFPEvalMethod());
+  // When `-ffast-math` option is enabled, it triggers several driver math
+  // options to be enabled. Among those, only one the following two modes
+  // affect the eval-method:  reciprocal or reassociate.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+PP.setCurrentFPEvalMethod(SourceLocation(),
+  LangOptions::FEM_Indeterminable);
 }
 
 // Anchor Sema's type info to this TU.
Index: clang/lib/Lex/PPMacroExpansion.cpp
===

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/eval-method-fast-math.c:83
+#pragma float_control(pop)
+
+int getFPEvalMethod() {

What should happen in a case like this?
```
int whatever(float a, float b, float c) {
  #pragma float_control(precise, off)
  res = a + b + c;
  int val = __FLT_EVAL_METHOD__;
  #pragma float_control(precise, on)
  return __FLT_EVAL_METHOD__;
}
```
I would expect that `int val` would hold `-1` and the return statement would 
return `0`?


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

https://reviews.llvm.org/D121122

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

[PATCH] D121405: [ASan] Moved optimized callbacks out of asan_static to avoid DSO size increase.

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov created this revision.
Herald added a subscriber: mgorny.
Herald added a project: All.
kstoimenov requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121405

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/lib/asan/CMakeLists.txt
  compiler-rt/lib/asan/asan_rtl_x86_64.S


Index: compiler-rt/lib/asan/asan_rtl_x86_64.S
===
--- compiler-rt/lib/asan/asan_rtl_x86_64.S
+++ compiler-rt/lib/asan/asan_rtl_x86_64.S
@@ -42,7 +42,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 1, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##1_asm ;\
+jmp__asan_report_##op##1 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_2(reg, op, i) \
 CLABEL(reg, op, 2, i): ;\
@@ -54,7 +54,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 2, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##2_asm ;\
+jmp__asan_report_##op##2 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_4(reg, op, i) \
 CLABEL(reg, op, 4, i): ;\
@@ -66,7 +66,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 4, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##4_asm ;\
+jmp__asan_report_##op##4 ;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_1(reg, op) \
 BEGINF(reg, op, 1, add) ;\
@@ -97,7 +97,7 @@
 #define ASAN_MEMORY_ACCESS_FAIL(reg, op, s, i) \
 FLABEL(reg, op, s, i): ;\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##s##_asm;\
+jmp__asan_report_##op##s##;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_8(reg, op) \
 BEGINF(reg, op, 8, add) ;\
Index: compiler-rt/lib/asan/CMakeLists.txt
===
--- compiler-rt/lib/asan/CMakeLists.txt
+++ compiler-rt/lib/asan/CMakeLists.txt
@@ -34,6 +34,7 @@
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_SOURCES
+asan_rtl_x86_64.S
 asan_interceptors_vfork.S
 )
 endif()
@@ -48,7 +49,6 @@
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_STATIC_SOURCES
-asan_rtl_x86_64.S
   )
 endif()
 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -30,7 +30,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-EXECUTABLE-LINUX %s
 //
-// CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan-x86_64
 
 // RUN: %clang -fsanitize=address -shared %s -### -o %t.o 2>&1  \
@@ -39,7 +38,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-SHARED-LINUX %s
 //
-// CHECK-ASAN-SHARED-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-SHARED-LINUX-NOT: libclang_rt.asan-x86_64
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -842,10 +842,6 @@
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
 StaticRuntimes.push_back("stats_client");
 
-  // Always link the static runtime regardless of DSO or executable.
-  if (SanArgs.needsAsanRt())
-HelperStaticRuntimes.push_back("asan_static");
-
   // Collect static runtimes.
   if (Args.hasArg(options::OPT_shared)) {
 // Don't link static runtimes into DSOs.


Index: compiler-rt/lib/asan/asan_rtl_x86_64.S
===
--- compiler-rt/lib/asan/asan_rtl_x86_64.S
+++ compiler-rt/lib/asan/asan_rtl_x86_64.S
@@ -42,7 +42,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 1, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##1_asm ;\
+jmp__asan_report_##op##1 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_2(reg, op, i) \
 CLABEL(reg, op, 2, i): ;\
@@ -54,7 +54,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 2, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##2_asm ;\
+jmp__asan_report_##op##2 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_4(reg, op, i) \
 CLABEL(reg, op, 4, i): ;\
@@ -66,7 +66,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 4, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##4_asm ;\
+jmp__asan_report_##op##4 ;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_1(reg, op) \
 BEGINF(reg, op, 1, add) ;\
@@ -97,7 +97,7 @@
 #define ASAN_MEMORY_ACCESS_FAIL(reg, op, s, i) \
 FLABEL(reg, op, s, i): ;\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##s##_asm;\
+jmp__asan_report_##op#

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 414467.

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

https://reviews.llvm.org/D121380

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Sema/fp-eval-pragma.cpp


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source', 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source', 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 414469.
kwk marked 2 inline comments as done.
kwk added a comment.

- Make @ or # not optional again
- Remove [\t\n\ \\]*
- Properly concat string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -458,6 +458,19 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SupportAtImportLines) {
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "#import \n"
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   EXPECT_EQ("#include \"llvm/a.h\"\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -170,11 +170,11 @@
 }
 
 inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
+  return IncludeName.trim("\"<>;");
 }
 
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)[^"<]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from 
sys::path::stem:
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2682,7 +2682,7 @@
 namespace {
 
 const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)[^"<]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
 } // anonymous namespace
 
@@ -2752,6 +2752,15 @@
 if (!FormattingOff && !MergeWithNextLine) {
   if (IncludeRegex.match(Line, &Matches)) {
 StringRef IncludeName = Matches[2];
+SmallString<0> IncludeNameStr;
+// HACK(kkleine): Sort C++ module includes/imports that are not 
enclosed
+// in "" or <> as if they are enclosed with <.
+if (!IncludeName.startswith("\"") && !IncludeName.startswith("<")) {
+  IncludeName = Twine("<", IncludeName)
+.concat(Twine(">"))
+.toStringRef(IncludeNameStr);
+}
+
 if (Line.contains("/*") && !Line.contains("*/")) {
   // #include with a start of a block comment, but without the end.
   // Need to keep all the lines until the end of the comment together.


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -458,6 +458,19 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SupportAtImportLines) {
+  EXPECT_EQ("#import \"a.h\"\n"
+"#import \"b.h\"\n"
+"#import \"c.h\"\n"
+"#import \n"
+"@import Foundation;\n",
+sort("#import \"b.h\"\n"
+ "#import \"c.h\"\n"
+ "#import \n"
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   EXPECT_EQ("#include \"llvm/a.h\"\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -170,11 +170,11 @@
 }
 
 inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
+  return IncludeName.trim("\"<>;");
 }
 
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#][\t\ ]*(import|include)[^"<]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2682,7 +2682,7 @@
 namespace {
 
 const char CppIncludeRegexPatt

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

Thank you for your comments! I've addressed many of them and now address the 
rest.




Comment at: clang/lib/Format/Format.cpp:2759
+  IncludeName =
+  StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+}

curdeius wrote:
> HazardyKnusperkeks wrote:
> > How does that not go out of scope and I have a dangling reference?
> Why not doing it the other way round, i.e. trimming `<>"` from include name?
> Why not doing it the other way round, i.e. trimming `<>"` from include name?

Because the include name is used for sorting later and if the `<>` is removed 
from the name, the `<>` included will be mixed with the `""`. That's 
undesirable. 



Comment at: clang/lib/Format/Format.cpp:2759
+  IncludeName =
+  StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+}

kwk wrote:
> curdeius wrote:
> > HazardyKnusperkeks wrote:
> > > How does that not go out of scope and I have a dangling reference?
> > Why not doing it the other way round, i.e. trimming `<>"` from include name?
> > Why not doing it the other way round, i.e. trimming `<>"` from include name?
> 
> Because the include name is used for sorting later and if the `<>` is removed 
> from the name, the `<>` included will be mixed with the `""`. That's 
> undesirable. 
> How does that not go out of scope and I have a dangling reference?

Sorry I was a bit lost with this but with some help I found other places in 
which `concat` was used and `toStringRef`. I hope everything is fine now.



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > I don't see why this is needed. It should match with the previous stuff.
> Do we want that optional?
> I don't see why this is needed. It should match with the previous stuff.

That's why I thought too. I swear I ran the tests before and I wouldn't work 
without this. But I re-ran the tests and now it works. Thanks for bringing up 
this very obvious part.



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

kwk wrote:
> HazardyKnusperkeks wrote:
> > HazardyKnusperkeks wrote:
> > > I don't see why this is needed. It should match with the previous stuff.
> > Do we want that optional?
> > I don't see why this is needed. It should match with the previous stuff.
> 
> That's why I thought too. I swear I ran the tests before and I wouldn't work 
> without this. But I re-ran the tests and now it works. Thanks for bringing up 
> this very obvious part.
> Do we want that optional?

We don't need it optional for the tests to pass. Only for C++20 Modules, we 
need it optional. But this is *not* part of this change. I've removed the 
optional part of this. 



Comment at: clang/unittests/Format/SortIncludesTest.cpp:471
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}

HazardyKnusperkeks wrote:
> I've no idea about obj-c, is it feasible to mix `#` and `@`? If so please add 
> a test where the `@` is between `#`s.
> I've no idea about obj-c, is it feasible to mix `#` and `@`? If so please add 
> a test where the `@` is between `#`s.

This test is copied from the issue itself. I have no idea about Obj-c as well. 
The `@` already *is* between the `#`. Did I miss something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121405: [ASan] Moved optimized callbacks out of asan_static to avoid DSO size increase.

[Vitaly Buka via Phabricator via cfe-commits]

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

LGTM, but maybe we can chat about that later today


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121405

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

[PATCH] D121283: [Clang] Support multiple attributes in a single pragma

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this, I plan on giving it a more thorough review when I can. But the 
summary led me to a design question: why do we support multiple attribute 
*specifiers* in the same pragma? I would not expect to be able to mix attribute 
styles in the same pragma given that all of the individual styles allow you to 
specify multiple attributes within a single specifier. e.g., 
https://godbolt.org/z/9v7r6Eanz

  __declspec(deprecated, naked) void f();
  __attribute__((annotate("test"), constructor(0))) void g();
  [[clang::annotate("test"), gnu::constructor(0)]] void h();

What's the use case for letting someone mix and match attribute styles as in:

  #pragma clang attribute push ([[clang::disable_tail_calls]] 
__attribute__((annotate("test"))), apply_to = function)

and why is whitespace the correct separator as opposed to a comma-delimited 
list?

(Note, I'm still thinking about whether this approach poses any actual 
problems; it may be perfectly fine, but it'd help to know why we're being lax 
in mixing forms and what the rationale is for whitespace separation; that's not 
a common approach to lists of things in C or C++).

Also, I'd expect there to be some documentation changes along with this patch 
and a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121283

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk planned changes to this revision.
kwk added a comment.

A test if failing. Need to address this first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/test/CodeGen/eval-method-fast-math.c:83
+#pragma float_control(pop)
+
+int getFPEvalMethod() {

aaron.ballman wrote:
> What should happen in a case like this?
> ```
> int whatever(float a, float b, float c) {
>   #pragma float_control(precise, off)
>   res = a + b + c;
>   int val = __FLT_EVAL_METHOD__;
>   #pragma float_control(precise, on)
>   return __FLT_EVAL_METHOD__;
> }
> ```
> I would expect that `int val` would hold `-1` and the return statement would 
> return `0`?
This test case is failing with this error (at the second pragma):

  t1.cpp:9:11: error: '#pragma float_control' can only appear at file scope or 
at
  the start of a compound statement
  #pragma float_control(precise, off)
  ^
  1 error generated.

Tried this:

  int whatever(float a, float b, float c) {
  #pragma float_control(precise, off)
  res = a + b + c;
  int val =__FLT_EVAL_METHOD__;
  {
  #pragma float_control(precise, on)
  return __FLT_EVAL_METHOD__;
  }
}

This generated these last IR instructions:
  store float %add1, float* @"?res@@3MA", align 4
  store i32 -1, i32* %val, align 4
  ret i32 -1

Not sure if this is correct. I guess the first pragma (off) didn't popped, so 
it's still on?  But inside the scope of the second pragma the first pragma 
shouldn't be visible? Not sure what should happen here. 


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

https://reviews.llvm.org/D121122

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

[clang] 035441f - [ASan] Moved optimized callbacks out of asan_static to avoid DSO size increase.

[Kirill Stoimenov via cfe-commits]

Author: Kirill Stoimenov
Date: 2022-03-10T21:11:32Z
New Revision: 035441ff3008c2a1930363751c6db61b71a5f089

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

LOG: [ASan] Moved optimized callbacks out of asan_static to avoid DSO size 
increase.

Reviewed By: vitalybuka

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/sanitizer-ld.c
compiler-rt/lib/asan/CMakeLists.txt
compiler-rt/lib/asan/asan_rtl_x86_64.S

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index abbba00e97537..2f3dc86eaad1d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -842,10 +842,6 @@ collectSanitizerRuntimes(const ToolChain &TC, const 
ArgList &Args,
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
 StaticRuntimes.push_back("stats_client");
 
-  // Always link the static runtime regardless of DSO or executable.
-  if (SanArgs.needsAsanRt())
-HelperStaticRuntimes.push_back("asan_static");
-
   // Collect static runtimes.
   if (Args.hasArg(options::OPT_shared)) {
 // Don't link static runtimes into DSOs.

diff  --git a/clang/test/Driver/sanitizer-ld.c 
b/clang/test/Driver/sanitizer-ld.c
index 966edbd7d038b..9a7a7db284c5d 100644
--- a/clang/test/Driver/sanitizer-ld.c
+++ b/clang/test/Driver/sanitizer-ld.c
@@ -30,7 +30,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-EXECUTABLE-LINUX %s
 //
-// CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan-x86_64
 
 // RUN: %clang -fsanitize=address -shared %s -### -o %t.o 2>&1  \
@@ -39,7 +38,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-SHARED-LINUX %s
 //
-// CHECK-ASAN-SHARED-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-SHARED-LINUX-NOT: libclang_rt.asan-x86_64
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \

diff  --git a/compiler-rt/lib/asan/CMakeLists.txt 
b/compiler-rt/lib/asan/CMakeLists.txt
index 0e7250a8fa10b..0862a3d648fa2 100644
--- a/compiler-rt/lib/asan/CMakeLists.txt
+++ b/compiler-rt/lib/asan/CMakeLists.txt
@@ -34,6 +34,7 @@ set(ASAN_SOURCES
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_SOURCES
+asan_rtl_x86_64.S
 asan_interceptors_vfork.S
 )
 endif()
@@ -48,7 +49,6 @@ set(ASAN_STATIC_SOURCES
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_STATIC_SOURCES
-asan_rtl_x86_64.S
   )
 endif()
 

diff  --git a/compiler-rt/lib/asan/asan_rtl_x86_64.S 
b/compiler-rt/lib/asan/asan_rtl_x86_64.S
index 50e039e53d626..92376f5b9a78f 100644
--- a/compiler-rt/lib/asan/asan_rtl_x86_64.S
+++ b/compiler-rt/lib/asan/asan_rtl_x86_64.S
@@ -42,7 +42,7 @@ CLABEL(reg, op, 1, i): ;\
 pop%rcx ;\
 jl RLABEL(reg, op, 1, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##1_asm ;\
+jmp__asan_report_##op##1 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_2(reg, op, i) \
 CLABEL(reg, op, 2, i): ;\
@@ -54,7 +54,7 @@ CLABEL(reg, op, 2, i): ;\
 pop%rcx ;\
 jl RLABEL(reg, op, 2, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##2_asm ;\
+jmp__asan_report_##op##2 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_4(reg, op, i) \
 CLABEL(reg, op, 4, i): ;\
@@ -66,7 +66,7 @@ CLABEL(reg, op, 4, i): ;\
 pop%rcx ;\
 jl RLABEL(reg, op, 4, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##4_asm ;\
+jmp__asan_report_##op##4 ;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_1(reg, op) \
 BEGINF(reg, op, 1, add) ;\
@@ -97,7 +97,7 @@ ENDF
 #define ASAN_MEMORY_ACCESS_FAIL(reg, op, s, i) \
 FLABEL(reg, op, s, i): ;\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##s##_asm;\
+jmp__asan_report_##op##s##;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_8(reg, op) \
 BEGINF(reg, op, 8, add) ;\



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

[PATCH] D121405: [ASan] Moved optimized callbacks out of asan_static to avoid DSO size increase.

[Kirill Stoimenov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG035441ff3008: [ASan] Moved optimized callbacks out of 
asan_static to avoid DSO size increase. (authored by kstoimenov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121405

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/lib/asan/CMakeLists.txt
  compiler-rt/lib/asan/asan_rtl_x86_64.S


Index: compiler-rt/lib/asan/asan_rtl_x86_64.S
===
--- compiler-rt/lib/asan/asan_rtl_x86_64.S
+++ compiler-rt/lib/asan/asan_rtl_x86_64.S
@@ -42,7 +42,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 1, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##1_asm ;\
+jmp__asan_report_##op##1 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_2(reg, op, i) \
 CLABEL(reg, op, 2, i): ;\
@@ -54,7 +54,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 2, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##2_asm ;\
+jmp__asan_report_##op##2 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_4(reg, op, i) \
 CLABEL(reg, op, 4, i): ;\
@@ -66,7 +66,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 4, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##4_asm ;\
+jmp__asan_report_##op##4 ;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_1(reg, op) \
 BEGINF(reg, op, 1, add) ;\
@@ -97,7 +97,7 @@
 #define ASAN_MEMORY_ACCESS_FAIL(reg, op, s, i) \
 FLABEL(reg, op, s, i): ;\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##s##_asm;\
+jmp__asan_report_##op##s##;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_8(reg, op) \
 BEGINF(reg, op, 8, add) ;\
Index: compiler-rt/lib/asan/CMakeLists.txt
===
--- compiler-rt/lib/asan/CMakeLists.txt
+++ compiler-rt/lib/asan/CMakeLists.txt
@@ -34,6 +34,7 @@
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_SOURCES
+asan_rtl_x86_64.S
 asan_interceptors_vfork.S
 )
 endif()
@@ -48,7 +49,6 @@
 
 if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_STATIC_SOURCES
-asan_rtl_x86_64.S
   )
 endif()
 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -30,7 +30,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-EXECUTABLE-LINUX %s
 //
-// CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-EXECUTABLE-LINUX: libclang_rt.asan-x86_64
 
 // RUN: %clang -fsanitize=address -shared %s -### -o %t.o 2>&1  \
@@ -39,7 +38,6 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-SHARED-LINUX %s
 //
-// CHECK-ASAN-SHARED-LINUX: libclang_rt.asan_static-x86_64
 // CHECK-ASAN-SHARED-LINUX-NOT: libclang_rt.asan-x86_64
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -842,10 +842,6 @@
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
 StaticRuntimes.push_back("stats_client");
 
-  // Always link the static runtime regardless of DSO or executable.
-  if (SanArgs.needsAsanRt())
-HelperStaticRuntimes.push_back("asan_static");
-
   // Collect static runtimes.
   if (Args.hasArg(options::OPT_shared)) {
 // Don't link static runtimes into DSOs.


Index: compiler-rt/lib/asan/asan_rtl_x86_64.S
===
--- compiler-rt/lib/asan/asan_rtl_x86_64.S
+++ compiler-rt/lib/asan/asan_rtl_x86_64.S
@@ -42,7 +42,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 1, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##1_asm ;\
+jmp__asan_report_##op##1 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_2(reg, op, i) \
 CLABEL(reg, op, 2, i): ;\
@@ -54,7 +54,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 2, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##2_asm ;\
+jmp__asan_report_##op##2 ;\
 
 #define ASAN_MEMORY_ACCESS_EXTRA_CHECK_4(reg, op, i) \
 CLABEL(reg, op, 4, i): ;\
@@ -66,7 +66,7 @@
 pop%rcx ;\
 jl RLABEL(reg, op, 4, i);\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##4_asm ;\
+jmp__asan_report_##op##4 ;\
 
 #define ASAN_MEMORY_ACCESS_CALLBACK_ADD_1(reg, op) \
 BEGINF(reg, op, 1, add) ;\
@@ -97,7 +97,7 @@
 #define ASAN_MEMORY_ACCESS_FAIL(reg, op, s, i) \
 FLABEL(reg, op, s, i): ;\
 mov%##reg,%rdi ;\
-jmp__asan_report_##op##s

[clang] 15ef06f - Diagnose when `#pragma clang fp eval_method` doesn't have a supported value.

[Zahira Ammarguellat via cfe-commits]

Author: Zahira Ammarguellat
Date: 2022-03-10T13:19:53-08:00
New Revision: 15ef06f453c6f9513b476e80dd9567d01fc662e8

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

LOG: Diagnose when `#pragma clang fp eval_method` doesn't have a supported 
value.
Currently the compiler is crashing.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/test/Sema/fp-eval-pragma.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 92ae27d6212a0..7af15f5504ff9 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@ def err_pragma_fp_invalid_argument : Error<
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;

diff  --git a/clang/test/Sema/fp-eval-pragma.cpp 
b/clang/test/Sema/fp-eval-pragma.cpp
index 42d88fd438e81..571b8097e9abf 100644
--- a/clang/test/Sema/fp-eval-pragma.cpp
+++ b/clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@ int foo2() {
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source', 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {



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

[PATCH] D121380: Pragma `clang fp eval_method` needs to take as input a supported value.

[Zahira Ammarguellat via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG15ef06f453c6: Diagnose when `#pragma clang fp eval_method` 
doesn't have a supported value. (authored by zahiraam).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121380

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Sema/fp-eval-pragma.cpp


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp 
eval_method'; expected 'source', 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;


Index: clang/test/Sema/fp-eval-pragma.cpp
===
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+  // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source', 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source', 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121269: [clang-format] Fix namespace format when the name is followed by a macro

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:108
+  // which one is name. For example, `namespace A B {`.
+  while (Tok && !Tok->is(tok::l_brace)) {
+if (FirstNSTok) {





Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:116
+}
+if (!FirstNSTok->is(tok::coloncolon)) {
+  NameFinished = true;





Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:135
   }
-  return name;
+  if (!NameFinished && FirstNSTok && !FirstNSTok->is(tok::l_brace))
+name += FirstNSTok->TokenText;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121269

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

[clang] e2b219b - [clang-format] Handle "// clang-format off" for RemoveBracesLLVM

[via cfe-commits]

Author: owenca
Date: 2022-03-10T13:30:53-08:00
New Revision: e2b219bded113160500d1084f87ee11ec926877b

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

LOG: [clang-format] Handle "// clang-format off" for RemoveBracesLLVM

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

Added: 


Modified: 
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 6c2765c038436..a8b8efb5b22f5 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1808,7 +1808,8 @@ class BracesRemover : public TokenAnalyzer {
   removeBraces(Line->Children, Result);
   if (!Line->Affected)
 continue;
-  for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+  for (FormatToken *Token = Line->First; Token && !Token->Finalized;
+   Token = Token->Next) {
 if (!Token->Optional)
   continue;
 assert(Token->isOneOf(tok::l_brace, tok::r_brace));

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 1d9b158ee6475..dab8f83fffe3c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24805,6 +24805,19 @@ TEST_F(FormatTest, RemoveBraces) {
"}",
Style);
 
+  verifyFormat("// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0)\n"
+   "  ++j;",
+   "// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0) { ++j; }",
+   Style);
+
   verifyFormat("if (a)\n"
"  b; // comment\n"
"else if (c)\n"



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

[PATCH] D121352: [clang-format] Handle "// clang-format off" for RemoveBracesLLVM

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe2b219bded11: [clang-format] Handle "// clang-format 
off" for RemoveBracesLLVM (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121352

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24805,6 +24805,19 @@
"}",
Style);
 
+  verifyFormat("// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0)\n"
+   "  ++j;",
+   "// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0) { ++j; }",
+   Style);
+
   verifyFormat("if (a)\n"
"  b; // comment\n"
"else if (c)\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1808,7 +1808,8 @@
   removeBraces(Line->Children, Result);
   if (!Line->Affected)
 continue;
-  for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+  for (FormatToken *Token = Line->First; Token && !Token->Finalized;
+   Token = Token->Next) {
 if (!Token->Optional)
   continue;
 assert(Token->isOneOf(tok::l_brace, tok::r_brace));


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24805,6 +24805,19 @@
"}",
Style);
 
+  verifyFormat("// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0)\n"
+   "  ++j;",
+   "// clang-format off\n"
+   "// comment\n"
+   "while (i > 0) { --i; }\n"
+   "// clang-format on\n"
+   "while (j < 0) { ++j; }",
+   Style);
+
   verifyFormat("if (a)\n"
"  b; // comment\n"
"else if (c)\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1808,7 +1808,8 @@
   removeBraces(Line->Children, Result);
   if (!Line->Affected)
 continue;
-  for (FormatToken *Token = Line->First; Token; Token = Token->Next) {
+  for (FormatToken *Token = Line->First; Token && !Token->Finalized;
+   Token = Token->Next) {
 if (!Token->Optional)
   continue;
 assert(Token->isOneOf(tok::l_brace, tok::r_brace));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.
Herald added a project: All.

ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D119675

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

[PATCH] D121410: Have cpu-specific variants set 'tune-cpu' as an optimization hint

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added a reviewer: aaron.ballman.
Herald added a subscriber: pengfei.
Herald added a project: All.
erichkeane requested review of this revision.

Due to various implementation constraints, despite the programmer
choosing a 'processor' cpu_dispatch/cpu_specific needs to use the
'feature' list of a processor to identify it.  This results in the
identified processor in source-code not being propogated to the
optimizer, and thus, not able to be tuned for.

This patch changes to use the actual cpu as written for tune-cpu so that
opt can make decisions based on the cpu-as-spelled, which should better
match the behavior expected by the programmer.

Note that the 'valid' list of processors for x86 is in
llvm/include/llvm/Support/X86TargetParser.def.  At the moment, this list
contains only Intel processors, but other vendors may wish to add their
own entries as 'alias'es (or wiht different feature lists!).

If this is not done, there is two potential performance issues with the 
patch, but I believe them to be worth it in light of the improvements to
behavior and performance.

1- In the event that the user spelled "ProcessorB", but we only have the
features available to test for "ProcessorA" (where A is B minus features),
AND there is an optimization opportunity for "B" that negatively affects
"A", the optimizer will likely choose to do so.

2- In the even that the user spelled VendorI's processor, and the feature
list allows it to run on VendorA's processor of similar features, AND there
is an optimization opportunity for VendorIs that negatively affects "A"s,
the optimizer will likely choose to do so.  This can be fixed by adding an
alias to X86TargetParser.def.


https://reviews.llvm.org/D121410

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific-avx-abi.c
  clang/test/CodeGen/attr-cpuspecific.c


Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -340,5 +340,8 @@
 void OrderDispatchUsageSpecific(void) {}
 
 // CHECK: attributes #[[S]] = 
{{.*}}"target-features"="+avx,+cmov,+crc32,+cx8,+f16c,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="ivybridge"
 // CHECK: attributes #[[K]] = 
{{.*}}"target-features"="+adx,+avx,+avx2,+avx512cd,+avx512er,+avx512f,+avx512pf,+bmi,+cmov,+crc32,+cx8,+f16c,+fma,+lzcnt,+mmx,+movbe,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="knl"
 // CHECK: attributes #[[O]] = 
{{.*}}"target-features"="+cmov,+cx8,+mmx,+movbe,+sse,+sse2,+sse3,+ssse3,+x87"
+// CHECK-SAME: "tune-cpu"="atom"
Index: clang/test/CodeGen/attr-cpuspecific-avx-abi.c
===
--- clang/test/CodeGen/attr-cpuspecific-avx-abi.c
+++ clang/test/CodeGen/attr-cpuspecific-avx-abi.c
@@ -23,4 +23,6 @@
 // CHECK: define{{.*}} @foo.V() #[[V:[0-9]+]]
 
 // CHECK: attributes #[[A]] = 
{{.*}}"target-features"="+avx,+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="generic"
 // CHECK: attributes #[[V]] = 
{{.*}}"target-features"="+avx,+avx2,+bmi,+cmov,+crc32,+cx8,+f16c,+fma,+lzcnt,+mmx,+movbe,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="core_4th_gen_avx"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2060,6 +2060,12 @@
   getTarget().isValidCPUName(ParsedAttr.Tune))
 TuneCPU = ParsedAttr.Tune;
 }
+
+if (SD) {
+  // Apply the given CPU name as the 'tune-cpu' so that the optimizer can
+  // favor this processor.
+  TuneCPU = SD->getCPUName(GD.getMultiVersionIndex())->getName();
+}
   } else {
 // Otherwise just add the existing target cpu and target features to the
 // function.


Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -340,5 +340,8 @@
 void OrderDispatchUsageSpecific(void) {}
 
 // CHECK: attributes #[[S]] = {{.*}}"target-features"="+avx,+cmov,+crc32,+cx8,+f16c,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="ivybridge"
 // CHECK: attributes #[[K]] = {{.*}}"target-features"="+adx,+avx,+avx2,+avx512cd,+avx512er,+avx512f,+avx512pf,+bmi,+cmov,+crc32,+cx8,+f16c,+fma,+lzcnt,+mmx,+movbe,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
+// CHECK-SAME: "tune-cpu"="knl"
 // CHECK: attributes #[[O]] = {{.*}}"target-features"="+cmov,+cx8,+mmx,+movbe,+sse,+sse2,+sse3,+ssse3,+x87"
+// CHECK-SAME: "tune-cpu"="atom"
Index: clang/test/CodeGen/attr-cpuspecific-avx-abi.c
==

[PATCH] D121410: Have cpu-specific variants set 'tune-cpu' as an optimization hint

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

@aaron.ballman : if you can add other reviewers or subscribers (particularly 
those from "VendorA") it would be greatly appreciated!


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

https://reviews.llvm.org/D121410

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

[PATCH] D121412: Complete the list of single-underscore keywords for MSVC compat.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: rnk, mstorsjo.
Herald added a subscriber: dexonsmith.
Herald added a project: All.
efriedma requested review of this revision.
Herald added a project: clang.

List derived from https://docs.microsoft.com/en-us/cpp/cpp/keywords-cpp .  Not 
that this is something we really want to encourage, but some of these show up 
in practice, so I figured I should just complete the list.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121412

Files:
  clang/include/clang/Basic/TokenKinds.def


Index: clang/include/clang/Basic/TokenKinds.def
===
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -676,24 +676,40 @@
 KEYWORD(__virtual_inheritance , KEYMS)
 KEYWORD(__interface   , KEYMS)
 ALIAS("__int8"   , char   , KEYMS)
-ALIAS("_int8", char   , KEYMS)
 ALIAS("__int16"  , short  , KEYMS)
-ALIAS("_int16"   , short  , KEYMS)
 ALIAS("__int32"  , int, KEYMS)
-ALIAS("_int32"   , int, KEYMS)
-ALIAS("_int64"   , __int64, KEYMS)
 ALIAS("__wchar_t", wchar_t, KEYMS)
-ALIAS("_asm" , asm, KEYMS)
-ALIAS("_alignof" , __alignof  , KEYMS)
 ALIAS("__builtin_alignof", __alignof  , KEYMS)
-ALIAS("_cdecl"   , __cdecl, KEYMS | KEYBORLAND)
-ALIAS("_fastcall", __fastcall , KEYMS | KEYBORLAND)
-ALIAS("_stdcall" , __stdcall  , KEYMS | KEYBORLAND)
-ALIAS("_thiscall", __thiscall , KEYMS)
-ALIAS("_vectorcall"  , __vectorcall, KEYMS)
-ALIAS("_uuidof"  , __uuidof   , KEYMS | KEYBORLAND)
-ALIAS("_inline"  , inline , KEYMS)
-ALIAS("_declspec", __declspec , KEYMS)
+
+// Microsoft single-underscore prefixed aliases for double-underscore prefixed
+// keywords.
+ALIAS("_asm" , asm  , KEYMS)
+ALIAS("_alignof" , __alignof, KEYMS)
+ALIAS("_cdecl"   , __cdecl  , KEYMS | KEYBORLAND)
+ALIAS("_declspec", __declspec   , KEYMS)
+ALIAS("_except"  , __except , KEYMS)
+ALIAS("_fastcall", __fastcall   , KEYMS | KEYBORLAND)
+ALIAS("_finally" , __finally, KEYMS)
+ALIAS("_forceinline" , __forceinline, KEYMS)
+ALIAS("_inline"  , inline   , KEYMS)
+ALIAS("_int8", char , KEYMS)
+ALIAS("_int16"   , short, KEYMS)
+ALIAS("_int32"   , int  , KEYMS)
+ALIAS("_int64"   , __int64  , KEYMS)
+ALIAS("_leave"   , __leave  , KEYMS)
+ALIAS("_multiple_inheritance", __multiple_inheritance, KEYMS)
+ALIAS("_ptr32"   , __ptr32  , KEYMS)
+ALIAS("_ptr64"   , __ptr64  , KEYMS)
+ALIAS("_restrict", restrict , KEYMS)
+ALIAS("_stdcall" , __stdcall, KEYMS | KEYBORLAND)
+ALIAS("_thiscall", __thiscall   , KEYMS)
+ALIAS("_try" , __try, KEYMS)
+ALIAS("_vectorcall"  , __vectorcall , KEYMS)
+ALIAS("_unaligned"   , __unaligned  , KEYMS)
+ALIAS("_uptr", __uptr   , KEYMS)
+ALIAS("_uuidof"  , __uuidof , KEYMS | KEYBORLAND)
+ALIAS("_virtual_inheritance", __virtual_inheritance, KEYMS)
+ALIAS("_w64" , __w64, KEYMS)
 
 // Borland Extensions which should be disabled in strict conformance mode.
 ALIAS("_pascal"  , __pascal   , KEYBORLAND)


Index: clang/include/clang/Basic/TokenKinds.def
===
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -676,24 +676,40 @@
 KEYWORD(__virtual_inheritance , KEYMS)
 KEYWORD(__interface   , KEYMS)
 ALIAS("__int8"   , char   , KEYMS)
-ALIAS("_int8", char   , KEYMS)
 ALIAS("__int16"  , short  , KEYMS)
-ALIAS("_int16"   , short  , KEYMS)
 ALIAS("__int32"  , int, KEYMS)
-ALIAS("_int32"   , int, KEYMS)
-ALIAS("_int64"   , __int64, KEYMS)
 ALIAS("__wchar_t", wchar_t, KEYMS)
-ALIAS("_asm" , asm, KEYMS)
-ALIAS("_alignof" , __alignof  , KEYMS)
 ALIAS("__builtin_alignof", __alignof  , KEYMS)
-ALIAS("_cdecl"   , __cdecl, KEYMS | KEYBORLAND)
-ALIAS("_fastcall", __fastcall , KEYMS | KEYBORLAND)
-ALIAS("_stdcall" , __stdcall  , KEYMS | KEYBORLAND)
-ALIAS("_thiscall", __thiscall , KEYMS)
-ALIAS("_vectorcall"  , __vectorcall, KEYMS)
-ALIAS("_uuidof"  , __uuidof   , KEYMS | KEYBORLAND)
-ALIAS("_inline"  , inline , KEYMS)
-ALIAS("_declspec", __declspec , KEYMS)
+
+// Microsoft single-underscore prefixed aliases for double-underscore prefixed
+// keywords.
+ALIAS("_asm" , asm  , KEYMS)
+ALIAS("_alignof" , __alignof, KEYMS)
+ALIAS("_cde

[PATCH] D121413: [clang-repl] Add an accessor to our underlying execution engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, lhames, sgraenitz.
Herald added a project: All.
v.g.vassilev requested review of this revision.

This patch will allow better incremental adoption of these changes in 
downstream cling and other users which want to experiment by customizing the 
execution engine.


Repository:
  rC Clang

https://reviews.llvm.org/D121413

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -196,6 +196,12 @@
   return IncrParser->getCI();
 }
 
+const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
+  if (IncrExecutor)
+return IncrExecutor->getExecutionEngine();
+  return nullptr;
+}
+
 llvm::Expected
 Interpreter::Parse(llvm::StringRef Code) {
   return IncrParser->Parse(Code);
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -45,6 +45,7 @@
   llvm::Error runCtors() const;
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
+  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
 };
 
 } // end namespace clang
Index: clang/include/clang/Interpreter/Interpreter.h
===
--- clang/include/clang/Interpreter/Interpreter.h
+++ clang/include/clang/Interpreter/Interpreter.h
@@ -27,6 +27,7 @@
 namespace llvm {
 namespace orc {
 class ThreadSafeContext;
+class LLJIT;
 }
 class Module;
 } // namespace llvm
@@ -58,6 +59,7 @@
   static llvm::Expected>
   create(std::unique_ptr CI);
   const CompilerInstance *getCompilerInstance() const;
+  const llvm::orc::LLJIT *getExecutionEngine() const;
   llvm::Expected Parse(llvm::StringRef Code);
   llvm::Error Execute(PartialTranslationUnit &T);
   llvm::Error ParseAndExecute(llvm::StringRef Code) {


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -196,6 +196,12 @@
   return IncrParser->getCI();
 }
 
+const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
+  if (IncrExecutor)
+return IncrExecutor->getExecutionEngine();
+  return nullptr;
+}
+
 llvm::Expected
 Interpreter::Parse(llvm::StringRef Code) {
   return IncrParser->Parse(Code);
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -45,6 +45,7 @@
   llvm::Error runCtors() const;
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
+  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
 };
 
 } // end namespace clang
Index: clang/include/clang/Interpreter/Interpreter.h
===
--- clang/include/clang/Interpreter/Interpreter.h
+++ clang/include/clang/Interpreter/Interpreter.h
@@ -27,6 +27,7 @@
 namespace llvm {
 namespace orc {
 class ThreadSafeContext;
+class LLJIT;
 }
 class Module;
 } // namespace llvm
@@ -58,6 +59,7 @@
   static llvm::Expected>
   create(std::unique_ptr CI);
   const CompilerInstance *getCompilerInstance() const;
+  const llvm::orc::LLJIT *getExecutionEngine() const;
   llvm::Expected Parse(llvm::StringRef Code);
   llvm::Error Execute(PartialTranslationUnit &T);
   llvm::Error ParseAndExecute(llvm::StringRef Code) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121413: [clang-repl] Add an accessor to our underlying execution engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 414494.
v.g.vassilev added a comment.

Order fwd decls alphabetically.


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

https://reviews.llvm.org/D121413

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -196,6 +196,12 @@
   return IncrParser->getCI();
 }
 
+const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
+  if (IncrExecutor)
+return IncrExecutor->getExecutionEngine();
+  return nullptr;
+}
+
 llvm::Expected
 Interpreter::Parse(llvm::StringRef Code) {
   return IncrParser->Parse(Code);
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -45,6 +45,7 @@
   llvm::Error runCtors() const;
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
+  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
 };
 
 } // end namespace clang
Index: clang/include/clang/Interpreter/Interpreter.h
===
--- clang/include/clang/Interpreter/Interpreter.h
+++ clang/include/clang/Interpreter/Interpreter.h
@@ -26,6 +26,7 @@
 
 namespace llvm {
 namespace orc {
+class LLJIT;
 class ThreadSafeContext;
 }
 class Module;
@@ -58,6 +59,7 @@
   static llvm::Expected>
   create(std::unique_ptr CI);
   const CompilerInstance *getCompilerInstance() const;
+  const llvm::orc::LLJIT *getExecutionEngine() const;
   llvm::Expected Parse(llvm::StringRef Code);
   llvm::Error Execute(PartialTranslationUnit &T);
   llvm::Error ParseAndExecute(llvm::StringRef Code) {


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -196,6 +196,12 @@
   return IncrParser->getCI();
 }
 
+const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
+  if (IncrExecutor)
+return IncrExecutor->getExecutionEngine();
+  return nullptr;
+}
+
 llvm::Expected
 Interpreter::Parse(llvm::StringRef Code) {
   return IncrParser->Parse(Code);
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -45,6 +45,7 @@
   llvm::Error runCtors() const;
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
+  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
 };
 
 } // end namespace clang
Index: clang/include/clang/Interpreter/Interpreter.h
===
--- clang/include/clang/Interpreter/Interpreter.h
+++ clang/include/clang/Interpreter/Interpreter.h
@@ -26,6 +26,7 @@
 
 namespace llvm {
 namespace orc {
+class LLJIT;
 class ThreadSafeContext;
 }
 class Module;
@@ -58,6 +59,7 @@
   static llvm::Expected>
   create(std::unique_ptr CI);
   const CompilerInstance *getCompilerInstance() const;
+  const llvm::orc::LLJIT *getExecutionEngine() const;
   llvm::Expected Parse(llvm::StringRef Code);
   llvm::Error Execute(PartialTranslationUnit &T);
   llvm::Error ParseAndExecute(llvm::StringRef Code) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 69924cc - Remove redundant lit config already handled in llvm_config. NFCI

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-03-10T23:11:01+01:00
New Revision: 69924ccf7a328f5f9bcc7073f38fa608a0dbfe2c

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

LOG: Remove redundant lit config already handled in llvm_config. NFCI

This logic duplicates lit.llvm.initialize, which we're already calling
(in lit.site.cfg.py.in).
The equivalent logic was removed from clang in d4401d354a938dd366bf but
never cleaned up here.

Added: 


Modified: 
clang-tools-extra/test/lit.cfg.py

Removed: 




diff  --git a/clang-tools-extra/test/lit.cfg.py 
b/clang-tools-extra/test/lit.cfg.py
index 858e42ace2a82..5c3934fb0b162 100644
--- a/clang-tools-extra/test/lit.cfg.py
+++ b/clang-tools-extra/test/lit.cfg.py
@@ -8,39 +8,18 @@
 import lit.formats
 import lit.util
 
+from lit.llvm import llvm_config
+
 # Configuration file for the 'lit' test runner.
 
 # name: The name of this test suite.
 config.name = 'Clang Tools'
 
-# Tweak PATH for Win32
-if platform.system() == 'Windows':
-# Seek sane tools in directories and set to $PATH.
-path = getattr(config, 'lit_tools_dir', None)
-path = lit_config.getToolsPath(path,
-   config.environment['PATH'],
-   ['cmp.exe', 'grep.exe', 'sed.exe'])
-if path is not None:
-path = os.path.pathsep.join((path,
- config.environment['PATH']))
-config.environment['PATH'] = path
-
-# Choose between lit's internal shell pipeline runner and a real shell.  If
-# LIT_USE_INTERNAL_SHELL is in the environment, we use that as an override.
-use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
-if use_lit_shell:
-# 0 is external, "" is default, and everything else is internal.
-execute_external = (use_lit_shell == "0")
-else:
-# Otherwise we default to internal on Windows and external elsewhere, as
-# bash on Windows is usually very slow.
-execute_external = (not sys.platform in ['win32'])
-
 # testFormat: The test format to use to interpret tests.
 #
 # For now we require '&&' between commands, until they get globally killed and
 # the test runner updated.
-config.test_format = lit.formats.ShTest(execute_external)
+config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', 
'.s',
@@ -103,10 +82,6 @@
 # Set available features we allow tests to conditionalize on.
 #
 
-# Shell execution
-if execute_external:
-config.available_features.add('shell')
-
 # Exclude MSYS due to transforming '/' to 'X:/mingwroot/'.
 if not platform.system() in ['Windows'] or not execute_external:
 config.available_features.add('shell-preserves-root')



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

[clang-tools-extra] 6ed2f89 - Fix reference to execute_external leftover in 69924ccf7a328f

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-03-10T23:20:03+01:00
New Revision: 6ed2f8902b8f109ead47e20a519860c803900d29

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

LOG: Fix reference to execute_external leftover in 69924ccf7a328f

Added: 


Modified: 
clang-tools-extra/test/lit.cfg.py

Removed: 




diff  --git a/clang-tools-extra/test/lit.cfg.py 
b/clang-tools-extra/test/lit.cfg.py
index 5c3934fb0b162..820914b51f15e 100644
--- a/clang-tools-extra/test/lit.cfg.py
+++ b/clang-tools-extra/test/lit.cfg.py
@@ -83,7 +83,7 @@
 #
 
 # Exclude MSYS due to transforming '/' to 'X:/mingwroot/'.
-if not platform.system() in ['Windows'] or not execute_external:
+if not platform.system() in ['Windows'] or llvm_config.use_lit_shell:
 config.available_features.add('shell-preserves-root')
 
 # ANSI escape sequences in non-dumb terminal



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

[clang-tools-extra] 2d58ba2 - [clang-tools-extra] Remove unused lit features/substitutions. NFCI

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-03-10T23:28:43+01:00
New Revision: 2d58ba200a39ddacaa9923cb623321c295ae5949

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

LOG: [clang-tools-extra] Remove unused lit features/substitutions. NFCI

Added: 


Modified: 
clang-tools-extra/test/lit.cfg.py

Removed: 




diff  --git a/clang-tools-extra/test/lit.cfg.py 
b/clang-tools-extra/test/lit.cfg.py
index 820914b51f15e..1527b07a33ee6 100644
--- a/clang-tools-extra/test/lit.cfg.py
+++ b/clang-tools-extra/test/lit.cfg.py
@@ -16,9 +16,6 @@
 config.name = 'Clang Tools'
 
 # testFormat: The test format to use to interpret tests.
-#
-# For now we require '&&' between commands, until they get globally killed and
-# the test runner updated.
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
@@ -73,23 +70,6 @@
   config.environment.get('LD_LIBRARY_PATH','')))
 config.environment['LD_LIBRARY_PATH'] = path
 
-# When running under valgrind, we mangle '-vg' onto the end of the triple so we
-# can check it with XFAIL and XTARGET.
-if lit_config.useValgrind:
-config.target_triple += '-vg'
-
-config.available_features.add('crash-recovery')
-# Set available features we allow tests to conditionalize on.
-#
-
-# Exclude MSYS due to transforming '/' to 'X:/mingwroot/'.
-if not platform.system() in ['Windows'] or llvm_config.use_lit_shell:
-config.available_features.add('shell-preserves-root')
-
-# ANSI escape sequences in non-dumb terminal
-if platform.system() not in ['Windows']:
-config.available_features.add('ansi-escape-sequences')
-
 if config.clang_tidy_staticanalyzer:
 config.available_features.add('static-analyzer')
 
@@ -119,11 +99,6 @@
 ('%run_clang_tidy',
  '%s %s' % (python_exec, run_clang_tidy)) )
 
-clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
- "tools", "clang", "tools", "extra",
- "clangd", "benchmarks")
-config.substitutions.append(('%clangd-benchmark-dir',
- '%s' % (clangd_benchmarks_dir)))
 config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
 config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
 



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

[clang] 0be56c8 - [clang-format][NFC] Group all C++ passes under isCpp()

[via cfe-commits]

Author: owenca
Date: 2022-03-10T14:30:30-08:00
New Revision: 0be56c87013b585405209f324cc8dfc5ebde416f

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

LOG: [clang-format][NFC] Group all C++ passes under isCpp()

Also removes a not very helpful comment.

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index a8b8efb5b22f5..9e3eaf72e121e 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1800,7 +1800,6 @@ class BracesRemover : public TokenAnalyzer {
   }
 
 private:
-  // Remove optional braces.
   void removeBraces(SmallVectorImpl &Lines,
 tooling::Replacements &Result) {
 const auto &SourceMgr = Env.getSourceManager();
@@ -3181,17 +3180,17 @@ reformat(const FormatStyle &Style, StringRef Code,
 });
   }
 
-  if (Style.isCpp() && Style.InsertBraces)
-Passes.emplace_back([&](const Environment &Env) {
-  return BracesInserter(Env, Expanded).process();
-});
+  if (Style.isCpp()) {
+if (Style.InsertBraces)
+  Passes.emplace_back([&](const Environment &Env) {
+return BracesInserter(Env, Expanded).process();
+  });
 
-  if (Style.isCpp() && Style.RemoveBracesLLVM)
-Passes.emplace_back([&](const Environment &Env) {
-  return BracesRemover(Env, Expanded).process();
-});
+if (Style.RemoveBracesLLVM)
+  Passes.emplace_back([&](const Environment &Env) {
+return BracesRemover(Env, Expanded).process();
+  });
 
-  if (Style.isCpp()) {
 if (Style.FixNamespaceComments)
   Passes.emplace_back([&](const Environment &Env) {
 return NamespaceEndCommentsFixer(Env, Expanded).process();



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