[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Daniel via Phabricator via cfe-commits]

Daniel599 updated this revision to Diff 267489.
Daniel599 marked an inline comment as done.

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

https://reviews.llvm.org/D80753

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
  llvm/include/llvm/ADT/StringMap.h

Index: llvm/include/llvm/ADT/StringMap.h
===
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -248,6 +248,24 @@
 return count(MapEntry.getKey());
   }
 
+  /// equal - check whether both of the containers are equal
+  bool operator==(const StringMap ) const {
+if (size() != RHS.size())
+  return false;
+
+for (const auto  : *this) {
+  auto FindInRHS = RHS.find(KeyValue.getKey());
+
+  if (FindInRHS == RHS.end())
+return false;
+
+  if (!(KeyValue.getValue() == FindInRHS->getValue()))
+return false;
+}
+
+return true;
+  }
+
   /// insert - Insert the specified key/value pair into the map.  If the key
   /// already exists in the map, return false and ignore the request, otherwise
   /// insert it and return true.
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
@@ -2,8 +2,7 @@
 
 void alwaysThrows() {
   int ex = 42;
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp]
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err61-cpp]
+  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp,cert-err61-cpp]
   throw ex;
 }
 
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t
+
+namespace std {
+template 
+class initializer_list {
+public:
+  initializer_list() noexcept {}
+};
+
+template 
+class vector {
+public:
+  vector() = default;
+  vector(initializer_list) {}
+
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template 
+  void emplace_back(Args &&... args){};
+  ~vector();
+};
+} // namespace std
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:
+  int _num1;
+  int _num2;
+  // CHECK-FIXES: _num2{};
+};
+
+int should_use_emplace(std::vector ) {
+  v.push_back(Foo());
+  // CHECK-FIXES: v.emplace_back();
+  // CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace,modernize-use-emplace]
+}
+
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t -- \
+ RUN: -config='{CheckOptions: [ \
+ RUN: {key: cppcoreguidelines-pro-type-member-init.UseAssignment, value: 1}, \
+ RUN: ]}'
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
+  // CHECK-MESSAGES: note: cannot apply fix-it because an alias checker has suggested a different fix-it, please remove one of the checkers or make sure they are both configured the same. Aliased checkers: 'cppcoreguidelines-pro-type-member-init', 'hicpp-member-init'
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:
+  int _num1;
+  int _num2;
+  // CHECK-FIXES: _num2;
+};
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- 

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Daniel via Phabricator via cfe-commits]

Daniel599 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:250
   void removeIncompatibleErrors();
+  void removeDuplicatedFixesOfAliasCheckers();
 

maybe I need to rename this method since now it's removing the errors also, 
I`ll do it when I get back from work.


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

https://reviews.llvm.org/D80753



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

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Daniel via Phabricator via cfe-commits]

Daniel599 marked 2 inline comments as done.
Daniel599 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:51
   bool IsWarningAsError;
+  std::vector EnabledDiagnosticAliases;
 };

njames93 wrote:
> Just ignore this, but do you ever get so bored and feel the need to save 24 
> bytes... https://gist.github.com/njames93/aac662ca52d345245e5e6f5dbc47d484 :)
That's a really cool trick, good to know :)


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

https://reviews.llvm.org/D80753



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

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` with the following script:

  arcfilter () {
arc amend
git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed 
By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people 
(https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You 
should keep the tag. (I started to use `--date=now` because some people find 
author date != committer date annoying. The committer date is usually what 
people care.))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80668



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

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` with the following script:

  arcfilter () {
arc amend
git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed 
By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people 
(https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You 
should keep the tag. (I started to use `--date=now` because some people find 
author date != committer date annoying. The committer date is usually what 
people care.))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80830



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

[PATCH] D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` with the following script:

  arcfilter () {
arc amend
git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed 
By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people 
(https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You 
should keep the tag. (I started to use `--date=now` because some people find 
author date != committer date annoying. The committer date is usually what 
people care.))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77658



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

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` with the following script:

  arcfilter () {
arc amend
git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed 
By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people 
(https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You 
should keep the tag. (I started to use `--date=now` because some people find 
author date != committer date annoying. The committer date is usually what 
people care.))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80631



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Oh, sorry. You merged all of my patch, right? It was not ready and even now it 
is unclear if the representation change in my patch is good or not. I was 
trying to suggest you only take the InternalOMPIRBuilder stuff so you can avoid 
the static function, not all of my patch. Sorry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D80222#2065141 , @jdoerfert wrote:

> Initialize the internal OpenMPIRbuilder (see my patch).


Isn't it initialized on line 1063 in `CGOpenMPRuntime.cpp`? There weren't any 
conflicts listed there when I merged your patch so it should be the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:291
  ///< including them in the name).
+CODEGENOPT(DebugUnusedTypes, 1, 0) /// < Whether to emit typedefs that may be
+   /// < unused.

This flag is now more than "typedefs"



Comment at: clang/include/clang/Driver/Options.td:3346
 defm reorder_blocks : BooleanFFlag<"reorder-blocks">, 
Group;
-defm eliminate_unused_debug_types : 
BooleanFFlag<"eliminate-unused-debug-types">, Group;
+defm eliminate_unused_debug_types :
+  BooleanFFlag<"eliminate-unused-debug-types">, Group,

This group is for ignored options. Add this near other f options.

I fixed the FIXME about BooleanFFlag with TableGen special identifier `NAME`. 
The way we define boolean options still look verbose to me, so I just created 
D80883.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



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

[PATCH] D80883: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: dblaikie, echristo, efriedma, joerg, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80883

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


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -66,6 +66,26 @@
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+// A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
+// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// This is useful if the option is usually disabled.
+multiclass OptInFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags,
+   HelpText;
+  def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
+// A boolean option which is opt-out in CC1. The negative option exists in CC1 
and
+// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+multiclass OptOutFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags, 
HelpText;
+  def fno_ #NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
 /
 // Groups
 
@@ -824,10 +844,7 @@
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 
-def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, 
CC1Option]>,
-  HelpText<"Emit an address-significance table">;
-def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, 
Flags<[CoreOption]>,
-  HelpText<"Don't emit an address-significance table">;
+defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " an 
address-significance table", [CoreOption]>;
 def fblocks : Flag<["-"], "fblocks">, Group, Flags<[CoreOption, 
CC1Option]>,
   HelpText<"Enable the 'blocks' language feature">;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group;
@@ -969,15 +986,8 @@
 def fbracket_depth_EQ : Joined<["-"], "fbracket-depth=">, Group, 
Flags<[CoreOption]>;
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;
-def fjump_tables : Flag<["-"], "fjump-tables">, Group;
-def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, 
Flags<[CC1Option]>,
-  HelpText<"Do not use jump tables for lowering switches">;
-def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Enable support for int128_t type">;
-def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Disable support for int128_t type">;
+defm jump_tables : OptOutFFlag<"jump-tables", "Use", "Do not use", " jump 
tables for lowering switches">;
+defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", 
"Disable", " support for int128_t type">;
 def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
   HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -66,6 +66,26 @@
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+// A boolean option which is opt-in in CC1. The positive option exists in CC1 and
+// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// This is useful if the option is usually disabled.
+multiclass OptInFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags,
+   HelpText;
+  def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
+// A boolean option which is opt-out in CC1. The negative option exists in CC1 and
+// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+multiclass OptOutFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags, HelpText;
+  def fno_ #NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
 /
 // Groups
 
@@ -824,10 +844,7 @@
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 
-def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, CC1Option]>,
-  HelpText<"Emit an address-significance table">;
-def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, Flags<[CoreOption]>,
-  HelpText<"Don't emit an address-significance table">;
+defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " an address-significance table", [CoreOption]>;
 def fblocks : Flag<["-"], "fblocks">, Group, 

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Initialize the internal OpenMPIRbuilder (see my patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 267483.
njames93 added a comment.

Fix compile error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80879

Files:
  clang-tools-extra/clang-query/tool/ClangQuery.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
@@ -333,8 +334,14 @@
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
-  CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
-cl::ZeroOrMore);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangTidyCategory,
+  cl::ZeroOrMore);
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
+
   llvm::IntrusiveRefCntPtr BaseFS(
   new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
 
@@ -365,7 +372,7 @@
   SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
 
   StringRef FileName("dummy");
-  auto PathList = OptionsParser.getSourcePathList();
+  auto PathList = OptionsParser->getSourcePathList();
   if (!PathList.empty()) {
 FileName = PathList.front();
   }
@@ -433,7 +440,7 @@
   ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers);
   std::vector Errors =
-  runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
+  runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
EnableCheckProfile, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError ) {
return E.DiagLevel == ClangTidyError::Error;
Index: clang-tools-extra/clang-query/tool/ClangQuery.cpp
===
--- clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -35,6 +35,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/WithColor.h"
 #include 
 #include 
 
@@ -86,7 +87,14 @@
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
-  CommonOptionsParser OptionsParser(argc, argv, ClangQueryCategory);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangQueryCategory,
+  llvm::cl::OneOrMore);
+
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
 
   if (!Commands.empty() && !CommandFiles.empty()) {
 llvm::errs() << argv[0] << ": cannot specify both -c and -f\n";
@@ -99,8 +107,8 @@
 return 1;
   }
 
-  ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
+  ClangTool Tool(OptionsParser->getCompilations(),
+ OptionsParser->getSourcePathList());
   std::vector> ASTs;
   int Status = Tool.buildASTs(ASTs);
   int ASTStatus = 0;


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
@@ -333,8 +334,14 @@
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
-  CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
-cl::ZeroOrMore);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangTidyCategory,
+  cl::ZeroOrMore);
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
+
   llvm::IntrusiveRefCntPtr BaseFS(
   new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
 
@@ -365,7 +372,7 @@
   SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
 
   StringRef FileName("dummy");
-  auto PathList = OptionsParser.getSourcePathList();
+  auto PathList = OptionsParser->getSourcePathList();
   if (!PathList.empty()) {
 FileName = PathList.front();
   }
@@ -433,7 

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D80753#2065067 , @Daniel599 wrote:

> Thank you @njames93, I already started and took a bit different approach.
>  In case of a conflict, leaving it to clang-tidy itself didn't help as it 
> added both of the fix-its together resulting in `= 0{};`, so I thought it 
> will be better to disable both of them.


Fair enough, your solution looks a lot nicer than what I suggested anyway.

> Sadly I didn't find 3 aliased checkers which can be configured to produce 
> different code so I can't check this edge case.
>  Please let me know if another changes are needed for this patch

You could create a unit test for 3 aliased checks with different options, 
however if there isn't a check that users can use to reproduce that edge case 
there is no point in worrying about it.




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:765-778
+  (*Inserted.first)->Message.Fix;
+
+  if (!(CandidateFix == ExistingFix)) {
+std::string AliasedCheckerFixConflict =
+"cannot apply fix-it because an alias checker has "
+"suggested a different fix-it, please remove one of the checkers "
+"or make sure they are both configured the same. "

`s/(*Inserted.first)->/ExistingError.`



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:779
+(*Inserted.first)
+->Notes.emplace_back(std::move(AliasedCheckerFixConflict));
+  }

```
ExistingError.Notes.emplace_back(llvm::formatv(
"cannot apply fix-it because an alias checker has "
"suggested a different fix-it, please remove one of the checkers "
"or make sure they are both configured the same. "
"Aliased checkers: '{0}', '{1}'",
ExistingError.DiagnosticName, Error.DiagnosticName).str());```



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:51
   bool IsWarningAsError;
+  std::vector EnabledDiagnosticAliases;
 };

Just ignore this, but do you ever get so bored and feel the need to save 24 
bytes... https://gist.github.com/njames93/aac662ca52d345245e5e6f5dbc47d484 :)


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

https://reviews.llvm.org/D80753



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

[clang] 1b6d29e - [Driver] Fix BooleanFFlag identifiers to use 'f' 'fno_' prefixes instead of suffixes

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-05-30T15:41:38-07:00
New Revision: 1b6d29e06b07e518025b6f06445ad3275d6f5684

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

LOG: [Driver] Fix BooleanFFlag identifiers to use 'f' 'fno_' prefixes instead 
of suffixes

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index e88e6cf8a130..729cbfb6ad4a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3316,11 +3316,9 @@ def Z_reserved_lib_cckext : Flag<["-"], 
"Z-reserved-lib-cckext">,
 Flags<[LinkerInput, NoArgumentUnused, Unsupported]>, 
Group;
 
 // Ignored options
-// FIXME: multiclasess produce suffixes, not prefixes. This is fine for now
-// since it is only used in ignored options.
 multiclass BooleanFFlag {
-  def _f : Flag<["-"], "f"#name>;
-  def _fno : Flag<["-"], "fno-"#name>;
+  def f#NAME : Flag<["-"], "f"#name>;
+  def fno_#NAME : Flag<["-"], "fno-"#name>;
 }
 
 defm : BooleanFFlag<"keep-inline-functions">, 
Group;



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

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Motivated by clang-tidy crashed for unknown command line argument. 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80879

Files:
  clang-tools-extra/clang-query/tool/ClangQuery.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
@@ -333,8 +334,14 @@
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
-  CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
-cl::ZeroOrMore);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangTidyCategory,
+  cl::ZeroOrMore);
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
+
   llvm::IntrusiveRefCntPtr BaseFS(
   new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
 
@@ -365,7 +372,7 @@
   SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
 
   StringRef FileName("dummy");
-  auto PathList = OptionsParser.getSourcePathList();
+  auto PathList = OptionsParser->getSourcePathList();
   if (!PathList.empty()) {
 FileName = PathList.front();
   }
@@ -433,7 +440,7 @@
   ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers);
   std::vector Errors =
-  runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
+  runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
EnableCheckProfile, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError ) {
return E.DiagLevel == ClangTidyError::Error;
Index: clang-tools-extra/clang-query/tool/ClangQuery.cpp
===
--- clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -35,6 +35,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/WithColor.h"
 #include 
 #include 
 
@@ -86,7 +87,13 @@
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
-  CommonOptionsParser OptionsParser(argc, argv, ClangQueryCategory);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangQueryCategory);
+
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
 
   if (!Commands.empty() && !CommandFiles.empty()) {
 llvm::errs() << argv[0] << ": cannot specify both -c and -f\n";
@@ -99,8 +106,8 @@
 return 1;
   }
 
-  ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
+  ClangTool Tool(OptionsParser->getCompilations(),
+ OptionsParser->getSourcePathList());
   std::vector> ASTs;
   int Status = Tool.buildASTs(ASTs);
   int ASTStatus = 0;


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
@@ -333,8 +334,14 @@
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
-  CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
-cl::ZeroOrMore);
+  llvm::Expected OptionsParser =
+  CommonOptionsParser::create(argc, argv, ClangTidyCategory,
+  cl::ZeroOrMore);
+  if (!OptionsParser) {
+llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+return 1;
+  }
+
   llvm::IntrusiveRefCntPtr BaseFS(
   new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
 
@@ -365,7 +372,7 @@
   SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
 
   StringRef FileName("dummy");
-  auto PathList = OptionsParser.getSourcePathList();
+  auto PathList = OptionsParser->getSourcePathList();
   if (!PathList.empty()) {

[PATCH] D80880: [clang] [MinGW] Link kernel32 once after the last instance of msvcrt

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: jacek, mati865, amccarth, rnk.
Herald added a project: clang.

The msvcrt library isn't a pure import library; it does contain regular object 
files with wrappers/fallbacks, and these can require linking against kernel32.

This only makes a difference when linking with ld.bfd, as lld always searches 
all static libraries.

This matches a similar change made recently in gcc in 
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=850533ab160ef40eccfd039e1e3b138cf26e76b8,
 although clang adds --start-group --end-group around these libraries if 
-static is specified, which gcc doesn't. But try to match gcc's linking order 
in any case, for consistency.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80880

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-msvcrt.c


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -4,6 +4,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_UCRT %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
+// CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
 // CHECK_MSVCR120: "-lmsvcr120"
 // CHECK_MSVCR120-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRTBASE: "-lucrtbase"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -310,10 +310,13 @@
 CmdArgs.push_back("-lkernel32");
   }
 
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static)) {
 CmdArgs.push_back("--end-group");
-  else
+  } else {
 AddLibGCC(Args, CmdArgs);
+if (!HasWindowsApp)
+  CmdArgs.push_back("-lkernel32");
+  }
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles)) {


Index: clang/test/Driver/mingw-msvcrt.c
===
--- clang/test/Driver/mingw-msvcrt.c
+++ clang/test/Driver/mingw-msvcrt.c
@@ -4,6 +4,7 @@
 // RUN: %clang -v -target i686-pc-windows-gnu -lucrt -### %s 2>&1 | FileCheck -check-prefix=CHECK_UCRT %s
 
 // CHECK_DEFAULT: "-lmingwex" "-lmsvcrt" "-ladvapi32"
+// CHECK_DEFAULT-SAME: "-lmsvcrt" "-lkernel32" "{{.*}}crtend.o"
 // CHECK_MSVCR120: "-lmsvcr120"
 // CHECK_MSVCR120-SAME: "-lmingwex" "-ladvapi32"
 // CHECK_UCRTBASE: "-lucrtbase"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -310,10 +310,13 @@
 CmdArgs.push_back("-lkernel32");
   }
 
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static)) {
 CmdArgs.push_back("--end-group");
-  else
+  } else {
 AddLibGCC(Args, CmdArgs);
+if (!HasWindowsApp)
+  CmdArgs.push_back("-lkernel32");
+  }
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Daniel via Phabricator via cfe-commits]

Daniel599 updated this revision to Diff 267478.
Daniel599 added a comment.

Thank you @njames93, I already started and took a bit different approach.
In case of a conflict, leaving it to clang-tidy itself didn't help as it added 
both of the fix-its together resulting in `= 0{};`, so I thought it will be 
better to disable both of them.
Sadly I didn't find 3 aliased checkers which can be configured to produce 
different code so I can't check this edge case.
Please let me know if another changes are needed for this patch


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

https://reviews.llvm.org/D80753

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
  llvm/include/llvm/ADT/StringMap.h

Index: llvm/include/llvm/ADT/StringMap.h
===
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -248,6 +248,24 @@
 return count(MapEntry.getKey());
   }
 
+  /// equal - check whether both of the containers are equal
+  bool operator==(const StringMap ) const {
+if (size() != RHS.size())
+  return false;
+
+for (const auto  : *this) {
+  auto FindInRHS = RHS.find(KeyValue.getKey());
+
+  if (FindInRHS == RHS.end())
+return false;
+
+  if (!(KeyValue.getValue() == FindInRHS->getValue()))
+return false;
+}
+
+return true;
+  }
+
   /// insert - Insert the specified key/value pair into the map.  If the key
   /// already exists in the map, return false and ignore the request, otherwise
   /// insert it and return true.
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
@@ -2,8 +2,7 @@
 
 void alwaysThrows() {
   int ex = 42;
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp]
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err61-cpp]
+  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp,cert-err61-cpp]
   throw ex;
 }
 
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t
+
+namespace std {
+template 
+class initializer_list {
+public:
+  initializer_list() noexcept {}
+};
+
+template 
+class vector {
+public:
+  vector() = default;
+  vector(initializer_list) {}
+
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template 
+  void emplace_back(Args &&... args){};
+  ~vector();
+};
+} // namespace std
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:
+  int _num1;
+  int _num2;
+  // CHECK-FIXES: _num2{};
+};
+
+int should_use_emplace(std::vector ) {
+  v.push_back(Foo());
+  // CHECK-FIXES: v.emplace_back();
+  // CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace,modernize-use-emplace]
+}
+
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t -- \
+ RUN: -config='{CheckOptions: [ \
+ RUN: {key: cppcoreguidelines-pro-type-member-init.UseAssignment, value: 1}, \
+ RUN: ]}'
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
+  // CHECK-MESSAGES: note: cannot apply fix-it because an alias checker has suggested a different fix-it, please remove one of the checkers or make sure they are both configured the 

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG44119626dedf: [clang-tidy] RenamerClangTidyChecks ignore 
builtin and command line macros (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80631

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -80,7 +80,7 @@
 // RUN: {key: readability-identifier-naming.LocalPointerPrefix, value: 
'l_'}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerCase, 
value: CamelCase}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerPrefix, 
value: 'lc_'}, \
-// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro \
 // RUN:   -I%S/Inputs/readability-identifier-naming \
 // RUN:   -isystem %S/Inputs/readability-identifier-naming/system
 
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -73,6 +73,14 @@
   /// MacroDefined calls checkMacro for macros in the main file
   void MacroDefined(const Token ,
 const MacroDirective *MD) override {
+if (MD->getMacroInfo()->isBuiltinMacro())
+  return;
+if (PP->getSourceManager().isWrittenInBuiltinFile(
+MacroNameTok.getLocation()))
+  return;
+if (PP->getSourceManager().isWrittenInCommandLineFile(
+MacroNameTok.getLocation()))
+  return;
 Check->checkMacro(PP->getSourceManager(), MacroNameTok, 
MD->getMacroInfo());
   }
 


Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -80,7 +80,7 @@
 // RUN: {key: readability-identifier-naming.LocalPointerPrefix, value: 'l_'}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerPrefix, value: 'lc_'}, \
-// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro \
 // RUN:   -I%S/Inputs/readability-identifier-naming \
 // RUN:   -isystem %S/Inputs/readability-identifier-naming/system
 
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -73,6 +73,14 @@
   /// MacroDefined calls checkMacro for macros in the main file
   void MacroDefined(const Token ,
 const MacroDirective *MD) override {
+if (MD->getMacroInfo()->isBuiltinMacro())
+  return;
+if (PP->getSourceManager().isWrittenInBuiltinFile(
+MacroNameTok.getLocation()))
+  return;
+if (PP->getSourceManager().isWrittenInCommandLineFile(
+MacroNameTok.getLocation()))
+  return;
 Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo());
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80877: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Updates the docs to include `MacroDefinition` documentation. The docs are still 
missing `ObjCIVar` however I don't have a clue about how that looks in code. If 
someone wants to show the code block needed for the example I'll add that in 
too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80877

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -62,6 +62,7 @@
  - :option:`LocalConstantPointerCase`, :option:`LocalConstantPointerPrefix`, 
:option:`LocalConstantPointerSuffix`
  - :option:`LocalPointerCase`, :option:`LocalPointerPrefix`, 
:option:`LocalPointerSuffix`
  - :option:`LocalVariableCase`, :option:`LocalVariablePrefix`, 
:option:`LocalVariableSuffix`
+ - :option:`MacroDefinitionCase`, :option:`MacroDefinitionPrefix`, 
:option:`MacroDefinitionSuffix`
  - :option:`MemberCase`, :option:`MemberPrefix`, :option:`MemberSuffix`
  - :option:`MethodCase`, :option:`MethodPrefix`, :option:`MethodSuffix`
  - :option:`NamespaceCase`, :option:`NamespacePrefix`, 
:option:`NamespaceSuffix`
@@ -1076,6 +1077,44 @@
 
 void foo() { int pre_local_constant_post; }
 
+.. option:: MacroDefinitionCase
+
+When defined, the check will ensure macro definitions conform to the
+selected casing.
+
+.. option:: MacroDefinitionPrefix
+
+When defined, the check will ensure macro definitions will add the
+prefixed with the given value (regardless of casing).
+
+.. option:: MacroDefinitionSuffix
+
+When defined, the check will ensure macro definitions will add the
+suffix with the given value (regardless of casing).
+
+For example using values of:
+
+   - MacroDefinitionCase of ``lower_case``
+   - MacroDefinitionPrefix of ``pre_``
+   - MacroDefinitionSuffix of ``_post``
+
+Identifies and/or transforms macro definitions as follows:
+
+Before:
+
+.. code-block:: c
+
+#define MY_MacroDefinition
+
+After:
+
+.. code-block:: c
+
+#define pre_my_macro_definition_post
+
+Note: This will not warn on builtin macros or macros defined on the command 
line
+using the ``-D`` flag.
+
 .. option:: MemberCase
 
 When defined, the check will ensure member names conform to the


Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -62,6 +62,7 @@
  - :option:`LocalConstantPointerCase`, :option:`LocalConstantPointerPrefix`, :option:`LocalConstantPointerSuffix`
  - :option:`LocalPointerCase`, :option:`LocalPointerPrefix`, :option:`LocalPointerSuffix`
  - :option:`LocalVariableCase`, :option:`LocalVariablePrefix`, :option:`LocalVariableSuffix`
+ - :option:`MacroDefinitionCase`, :option:`MacroDefinitionPrefix`, :option:`MacroDefinitionSuffix`
  - :option:`MemberCase`, :option:`MemberPrefix`, :option:`MemberSuffix`
  - :option:`MethodCase`, :option:`MethodPrefix`, :option:`MethodSuffix`
  - :option:`NamespaceCase`, :option:`NamespacePrefix`, :option:`NamespaceSuffix`
@@ -1076,6 +1077,44 @@
 
 void foo() { int pre_local_constant_post; }
 
+.. option:: MacroDefinitionCase
+
+When defined, the check will ensure macro definitions conform to the
+selected casing.
+
+.. option:: MacroDefinitionPrefix
+
+When defined, the check will ensure macro definitions will add the
+prefixed with the given value (regardless of casing).
+
+.. option:: MacroDefinitionSuffix
+
+When defined, the check will ensure macro definitions will add the
+suffix with the given value (regardless of casing).
+
+For example using values of:
+
+   - MacroDefinitionCase of ``lower_case``
+   - MacroDefinitionPrefix of ``pre_``
+   - MacroDefinitionSuffix of ``_post``
+
+Identifies and/or transforms macro definitions as follows:
+
+Before:
+
+.. code-block:: c
+
+#define MY_MacroDefinition
+
+After:
+
+.. code-block:: c
+
+#define pre_my_macro_definition_post
+
+Note: This will not warn on builtin macros or macros defined on the command line
+using the ``-D`` flag.
+
 .. option:: MemberCase
 
 When defined, the check will ensure member names conform to the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80876: [clang] Default to windows response files when running on windows

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, sunfish, aheejin.
Herald added a project: clang.
sbc100 added a reviewer: ruiu.

This matches other tools such as llvm-ar, lld, etc.

This consistency is useful when authoring downstream tools as it means
they can always use windows-style response files on windows, rather than
generate a different style for clang compared to the other tools.

This was discussed in the equivalent llvm-ar change:

  https://reviews.llvm.org/D69665

wasm-ld behaviour was changed there:

  https://reviews.llvm.org/D75577


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80876

Files:
  clang/tools/driver/driver.cpp


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -385,10 +385,22 @@
   bool MarkEOLs = ClangCLMode;
 
   llvm::cl::TokenizerCallback Tokenizer;
-  if (RSPQuoting == Windows || (RSPQuoting == Default && ClangCLMode))
+  switch (RSPQuoting) {
+  case Default: {
+if (ClangCLMode ||
+llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows())
+  Tokenizer = ::cl::TokenizeWindowsCommandLine;
+else
+  Tokenizer = ::cl::TokenizeGNUCommandLine;
+break;
+  }
+  case Windows:
 Tokenizer = ::cl::TokenizeWindowsCommandLine;
-  else
+break;
+  case POSIX:
 Tokenizer = ::cl::TokenizeGNUCommandLine;
+break;
+  }
 
   if (MarkEOLs && argv.size() > 1 && StringRef(argv[1]).startswith("-cc1"))
 MarkEOLs = false;


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -385,10 +385,22 @@
   bool MarkEOLs = ClangCLMode;
 
   llvm::cl::TokenizerCallback Tokenizer;
-  if (RSPQuoting == Windows || (RSPQuoting == Default && ClangCLMode))
+  switch (RSPQuoting) {
+  case Default: {
+if (ClangCLMode ||
+llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows())
+  Tokenizer = ::cl::TokenizeWindowsCommandLine;
+else
+  Tokenizer = ::cl::TokenizeGNUCommandLine;
+break;
+  }
+  case Windows:
 Tokenizer = ::cl::TokenizeWindowsCommandLine;
-  else
+break;
+  case POSIX:
 Tokenizer = ::cl::TokenizeGNUCommandLine;
+break;
+  }
 
   if (MarkEOLs && argv.size() > 1 && StringRef(argv[1]).startswith("-cc1"))
 MarkEOLs = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 4411962 - [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-05-30T20:49:33+01:00
New Revision: 44119626dedfebe245fe6ce26487949201299d38

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

LOG: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

Summary: Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=42635 | 
readability-identifier-naming option MacroDefinitionCase should ignore macros 
passed as parameters. ]]

Reviewers: aaron.ballman, alexfh, gribozavr2, hokein

Reviewed By: aaron.ballman

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index dd05b3a45c0d..3301ba6343c7 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -73,6 +73,14 @@ class RenamerClangTidyCheckPPCallbacks : public PPCallbacks {
   /// MacroDefined calls checkMacro for macros in the main file
   void MacroDefined(const Token ,
 const MacroDirective *MD) override {
+if (MD->getMacroInfo()->isBuiltinMacro())
+  return;
+if (PP->getSourceManager().isWrittenInBuiltinFile(
+MacroNameTok.getLocation()))
+  return;
+if (PP->getSourceManager().isWrittenInCommandLineFile(
+MacroNameTok.getLocation()))
+  return;
 Check->checkMacro(PP->getSourceManager(), MacroNameTok, 
MD->getMacroInfo());
   }
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
index 7983bb30ca64..1bb435e02eb5 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -80,7 +80,7 @@
 // RUN: {key: readability-identifier-naming.LocalPointerPrefix, value: 
'l_'}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerCase, 
value: CamelCase}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerPrefix, 
value: 'lc_'}, \
-// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro \
 // RUN:   -I%S/Inputs/readability-identifier-naming \
 // RUN:   -isystem %S/Inputs/readability-identifier-naming/system
 



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5565
+if (CGDebugInfo *DI = getModuleDebugInfo())
+  if (getCodeGenOpts().DebugUnusedTypes) {
+QualType QT = getContext().getTypedefType(cast(D));

nickdesaulniers wrote:
> dblaikie wrote:
> > Rather than testing DebugUnusedType for every call - might be best to add a 
> > "EmitUnusedType" function that tests the flag and does the emission? (same 
> > way EmitExplicitCastType already checks for a certain level of debug info 
> > before emitting)
> It can be; what I don't like about that approach is that we have to determine 
> the `QualType` here to pass in, at which point such function might just 
> return without doing anything. (ie. we have to do slightly more work in the 
> case where the debugging of unused types was *not* requested).  But that's a 
> minor nit and we can live with it?
I don't believe getting the QualType from a TypeDecl is an expensive 
operation/enough to try to avoid it here. If it is I'd rather have a 
"EmitUnusedType(TypeDecl*)" function, and then the conditional QualType 
retrieval could be in there, written once rather than several times. 



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:768
   Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params);
+  Opts.DebugUnusedTypes = Args.hasArg(OPT_eliminate_unused_debug_types_fno);
   Opts.EmbedSource = Args.hasArg(OPT_gembed_source);

dblaikie wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > dblaikie wrote:
> > > > Could this be rolled into the debug-info-kind? (a kind beyond 
> > > > "standalone")
> > > That sounds like a good idea.  Looking at the definition of 
> > > `DebugInfoKind` 
> > > (llvm/llvm-project/clang/include/clang/Basic/DebugInfoOptions.h), it 
> > > seems that `DebugInfoKind` is an `enum` that defines a "level" of debug 
> > > info to emit? Looking at the guard in `CGDebugInfo::EmitExplicitCastType` 
> > > (llvm/llvm-project/clang/lib/CodeGen/CGDebugInfo.cpp), it calls 
> > > `CodeGenOptions::hasReducedDebugInfo()` which does a comparison against a 
> > > certain level.  That seems to indicate the order of the enumerations is 
> > > important.  Do you have a suggestion what order I should add the new enum?
> > > 
> > > I'm guessing after `LimitedDebugInfo` or `DebugInfoConstructor`, but 
> > > before `FullDebugInfo`? (I find the name of the method 
> > > `hasReducedDebugInfo` confusing in that regard).
> > Ok, so I have a diff that implements this approach.  I feel like I should 
> > maybe publish it as a child commit to this one, to be able to more easily 
> > discuss it?
> > 
> > Two problems I run into:
> > 1. as alluded to in my previous response in this thread, `DebugInfoKind` is 
> > an enum that specifies a level.  It tends to "drag" other debug flags in 
> > based on the ordering.  Looking at extending the `switch` in 
> > `CGDebugInfo::CreateCompileUnit` (clang/lib/CodeGen/CGDebugInfo.cpp), it's 
> > not at all clear to me which we existing case should we choose?
> > 2. we want the flag `-fno-eliminate-unused-debug-types` to match GCC for 
> > compatibility.  We can additionally add a new debug info kind like 
> > `"standalone"` (clang/lib/Frontend/CompilerInvocation.cpp), but it's not 
> > clear how the two flags together should interact.
> > 
> > The suggestion for a new debug info kind feels like a recommendation to add 
> > a new "level" of debug info, but `-fno-eliminate-unused-debug-types` feels 
> > like it should be mutually exclusive of debug info kind? (I guess GCC does 
> > *require* `-g` with `-fno-eliminate-unused-debug-types`)
> > 
> > @dblaikie maybe you have more recommendations on this?
> This value would probably go "after" "full" (full isn't full enough, as 
> you've found - you need a mode that's even "fullerer")
> 
> Perhaps renaming "Full" to "Referenced" and then introducing this new kind as 
> the new "Full" or maybe under a somewhat different name to avoid ambiguity. 
> 
> Any suggestions on a less confusing name for "hasReducedDebugInfo"? (I think 
> it was basically "Has less than full debug info"... but, yep, it doesn't do 
> that at all - it's "HasTypeAndVariableDebugInfo" by the looks of it/by the 
> comment)
> Ok, so I have a diff that implements this approach. I feel like I should 
> maybe publish it as a child commit to this one, to be able to more easily 
> discuss it?
> 
> Two problems I run into:
> 
> 1. as alluded to in my previous response in this thread, DebugInfoKind is an 
> enum that specifies a level. It tends to "drag" other debug flags in based on 
> the ordering. Looking at extending the switch in 
> CGDebugInfo::CreateCompileUnit (clang/lib/CodeGen/CGDebugInfo.cpp), it's not 
> at all clear to me which we existing case should we choose?
> 2. we want the flag -fno-eliminate-unused-debug-types to match GCC for 
> compatibility. We can additionally add a 

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);

erichkeane wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > I guess its a problem for all of these, but why is the last 
> > > 'true'/'false' (KnownToGCC) not checked from the Spelling?  It has:  let 
> > > KnownToGCC = 1;.
> > > 
> > > I would presume that line either is meaningless, or should be used for 
> > > the true/false bits in here.
> > The `KnownToGCC` flag affects whether we produce certain warnings, not 
> > which spellings we register. This does seem fishy: we have the same 
> > information represented and examined both by considering whether the 
> > attribute has a `GNU` vs `GCC` spelling and by considering whether the 
> > `KnownToGCC` flag is set. I imagine we could factor this better. (The two 
> > concerns are, I suppose, notionally orthogonal, but I can't imagine we 
> > would ever want them to differ.)
> Well, both are used to set the value in "FlattenedSpelling".  Here we use 
> true/false, but if you construct one via a record it uses the KnownToGCC 
> flag.  It seems to me that these values should be initialized consistently.
> 
> The "spelling" Variety themselves I can see being different, but for 
> consistency's sake, these trues in the emplace_back should set KnownToGCC 
> with the attribute.  That, or we abandon the KnownToGCC 'Value' in the 
> spelling and just always infer it based on the variety.
> 
> Like I said, its just weird that sometimes we set this via the tablegen file, 
> sometimes we do it automagically.
Yeah, I agree that this is a bit weird. IIRC, this came from a time when GCC 
hadn't yet made all __attribute__ spellings available as [[]] spellings and so 
there was a worry we'd have some __attribute__ spellings that use GNU instead 
of GCC, despite being supported by GCC. I don't think that situation is 
plausible any longer.

I think a better approach is to keep the known to GCC bit on the `ParsedAttr` 
object so we don't have to check attribute vendor name when issuing a 
diagnostic, but we should infer that bit from the spelling instead of setting 
it in the Attr.td file.


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

https://reviews.llvm.org/D80836



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

[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

[Kristina Bessonova via Phabricator via cfe-commits]

krisb created this revision.
krisb added reviewers: phosek, thakis, russell.gallop.
Herald added subscribers: llvm-commits, cfe-commits, mgorny.
Herald added projects: clang, LLVM.

(I'm trying to get a bootstrap self-build on Windows, where lld is used as a
linker for the stage2.)

I assume BOOTSTRAP_LLVM_ENABLE_LLD works the same way as LLVM_ENABLE_LLD,
but enables to use just-built lld for the next stage.

The issue with LLVM_ENABLE_LLD is that it just passes -fuse-ld=lld
to compiler/linker options which makes sense only for those platforms
where cmake invokes a compiler driver for linking.

On Windows cmake invokes a linker directly and requires CMAKE_LINKER
option to be specified otherwise it defaults CMAKE_LINKER to be link.exe.
Passing CMAKE_LINKER is easy for 1-stage builds, but it's notquite handy
to do for multistage builds, because it's not possible to know for sure
where to find just-built lld.

This patch allows BOOTSTRAP_LLVM_ENABLE_LLD to set CMAKE_LINKER in the case
of building for host Windows. It also skips adding '-fuse-ld=lld' to make
lld-link not warning about 'unknown argument'. The latter part is taken
from https://reviews.llvm.org/D69030 with additional checks that
CMAKE_LINKER doesn't contain a path to another compiler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80873

Files:
  clang/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -264,7 +264,15 @@
   set(LLVM_USE_LINKER "lld")
 endif()
 
-if( LLVM_USE_LINKER )
+# cmake defaults to invoke a linker directly on Windows, so skip adding
+# '-fuse-ld' flag in this case.
+if( LINKER_IS_LLD_LINK AND ( NOT CMAKE_LINKER OR CMAKE_LINKER MATCHES 
"lld-link" ))
+  set(IS_FUSE_LD_FLAG_NEEDED FALSE)
+else()
+  set(IS_FUSE_LD_FLAG_NEEDED TRUE)
+endif()
+
+if( LLVM_USE_LINKER AND IS_FUSE_LD_FLAG_NEEDED )
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} 
-fuse-ld=${LLVM_USE_LINKER}")
   check_cxx_source_compiles("int main() { return 0; }" 
CXX_SUPPORTS_CUSTOM_LINKER)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -745,6 +745,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified in case of MSVC or it defaults
+  # the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(MSVC AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG 
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -264,7 +264,15 @@
   set(LLVM_USE_LINKER "lld")
 endif()
 
-if( LLVM_USE_LINKER )
+# cmake defaults to invoke a linker directly on Windows, so skip adding
+# '-fuse-ld' flag in this case.
+if( LINKER_IS_LLD_LINK AND ( NOT CMAKE_LINKER OR CMAKE_LINKER MATCHES "lld-link" ))
+  set(IS_FUSE_LD_FLAG_NEEDED FALSE)
+else()
+  set(IS_FUSE_LD_FLAG_NEEDED TRUE)
+endif()
+
+if( LLVM_USE_LINKER AND IS_FUSE_LD_FLAG_NEEDED )
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fuse-ld=${LLVM_USE_LINKER}")
   check_cxx_source_compiles("int main() { return 0; }" CXX_SUPPORTS_CUSTOM_LINKER)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -745,6 +745,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified in case of MSVC or it defaults
+  # the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(MSVC AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER -DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 267466.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Updated to remove the KnownToGCC tablegen bit.


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

https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -60,9 +60,7 @@
 assert(V != "GCC" && V != "Clang" &&
"Given a GCC spelling, which means this hasn't been flattened!");
 if (V == "CXX11" || V == "C2x" || V == "Pragma")
-  NS = std::string(Spelling.getValueAsString("Namespace"));
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
+  NS = std::string(Spelling.getValueAsString("Namespace"));  
   }
 
   const std::string () const { return V; }
@@ -82,9 +80,10 @@
 StringRef Variety = Spelling->getValueAsString("Variety");
 StringRef Name = Spelling->getValueAsString("Name");
 if (Variety == "GCC") {
-  // Gin up two new spelling objects to add into the list.
   Ret.emplace_back("GNU", std::string(Name), "", true);
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);
 } else if (Variety == "Clang") {
   Ret.emplace_back("GNU", std::string(Name), "", false);
   Ret.emplace_back("CXX11", std::string(Name), "clang", false);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[nodiscard]] int without_underscores(void);
 [[__nodiscard__]] int underscores(void);
+
+// Match GCC's behavior for C attributes as well.
+[[gnu::constructor]] void ctor_func(void);
+[[gnu::destructor]] void dtor_func(void);
+[[gnu::hot]] void hot_func(void);
+[[__gnu__::hot]] void hot_func2(void);
+[[gnu::__hot__]] void hot_func3(void);
+[[__gnu__::__hot__]] void hot_func4(void);
+
+// Note how not all GCC attributes are supported in C.
+[[gnu::abi_tag("")]] void abi_func(void); // expected-warning {{unknown 
attribute 'abi_tag' ignored}}
+struct S s [[gnu::init_priority(1)]]; // expected-warning {{unknown attribute 
'init_priority' ignored}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -257,7 +257,6 @@
 class Spelling {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU : Spelling;
@@ -277,11 +276,11 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC : Spelling {
-  let KnownToGCC = 1;
+class GCC : Spelling {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +604,7 @@
 //
 
 def AbiTag : Attr {
-  let Spellings = [GCC<"abi_tag">];
+  let Spellings = [GCC<"abi_tag", /*AllowInC*/0>];
   let Args = [VariadicStringArgument<"Tags">];
   let Subjects = SubjectList<[Struct, Var, Function, Namespace], ErrorDiag>;
   let MeaningfulToClassTemplateDefinition = 1;
@@ -2113,7 +2112,7 @@
 }
 
 def InitPriority : InheritableAttr {
-  let Spellings = [GCC<"init_priority">];
+  let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;
   let Documentation = [Undocumented];


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -60,9 +60,7 @@
 assert(V != "GCC" && V != "Clang" &&
"Given a GCC spelling, which means this hasn't been flattened!");
 if (V == "CXX11" || V == "C2x" || V == "Pragma")
-  NS = std::string(Spelling.getValueAsString("Namespace"));
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
+  NS = std::string(Spelling.getValueAsString("Namespace"));  
   }
 
   const std::string () const { return V; }
@@ -82,9 +80,10 @@
 StringRef Variety = Spelling->getValueAsString("Variety");
 StringRef Name = Spelling->getValueAsString("Name");
 if (Variety == "GCC") {
-  // Gin up two new spelling objects to add into the list.
   Ret.emplace_back("GNU", std::string(Name), "", true);
   

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.

LGMT, just a few minor nits though.




Comment at: 
clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:15-16
+#include "clang/Frontend/CompilerInstance.h"
+#include 
+#include 
+

Fairly certain these headers aren't used



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h:30
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }

`std::all_of`, `std::any_of` and `std::none_of` were only introduced in c++11, 
maybe `CPlusPlus11` should be the minimum requirement.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:202
 ---
-

This removed line in unrelated and should be added back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



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

[PATCH] D80237: [hip] Ensure pointer in struct argument has proper `addrspacecast`.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D80237#2062991 , @arsenm wrote:

> In D80237#2058108 , @rjmccall wrote:
>
> > In D80237#2055902 , @arsenm wrote:
> >
> > > In D80237#2051933 , @rjmccall 
> > > wrote:
> > >
> > > > Okay.  Can you explain why we need to coerce in the first place, 
> > > > though?  Especially if the representation is the same, why is your 
> > > > target-lowering requiring parameters to be coerced to involve pointers 
> > > > in a different address space?
> > >
> > >
> > > It's not a requirement, but an optimization. The pointer representation 
> > > is the same, but there's a penalty to using the generic pointer. From the 
> > > language context here, we know the flat pointer can never alias the 
> > > non-global address spaces.
> >
> >
> > Is that because it's a kernel argument?  I don't understand how the 
> > coercion can work in general for structs that just contain pointers.
>
>
> Yes, there's no valid way to pass a non-global address space pointer through 
> a flat pointer into a kernel. This doesn't apply in other contexts


Okay.  And you can't just treat kernel parameters specially in your pass that 
un-promotes generic pointers back to global pointers?  Or is that pass not 
quite so precise as that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80237



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

[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives

[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, but please wait for a few days in case @rsmith has further concerns.




Comment at: clang/lib/Sema/SemaInit.cpp:8182
   !ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
-   diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()

The comment should end with a period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77836



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

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D80753#2064857 , @Daniel599 wrote:

> I have made the needed changes in order to detect a conflict in duplicated 
> fix-it of aliased checkers and also added a test for it.
>  Now I`ll try to work on combining aliased errors together,  any tips 
> regarding this feature?


Here is a quick draft I built up for this ClangTidyDiagnosticsConsumer.cpp 
 lmk what 
you think.
Some of the handling probably can be changed for conflicting fix its.


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

https://reviews.llvm.org/D80753



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

[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

btw, if you're interested in exploring a static analyzer solution for this, 
here's a recent review of an analyzer feature that's in the same general 
problem space: https://reviews.llvm.org/D80018


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D79437



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

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80631



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

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[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, though please wait a bit for @njames93 to speak up if they still have 
concerns.




Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:61
+  hasBody(allOf(hasDescendant(returns(true)),
+unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(returnsButNotTrue))

mgehre wrote:
> aaron.ballman wrote:
> > Should we reject other ways to break out of the loop, like `goto` or 
> > `throw`?
> I think throw statements still can be transformed. We cannot transform 
> `break` because the loop is gone and we cannot transform `goto` because we 
> cannot jump from the lambda into its caller.
> But we can keep `throw` statements because exceptions can propagate from the 
> lambda through the algorithm back into the original caller. If we could not 
> allow `throw` statements, we would also have to disallow any other kind of 
> call statements.
Ah, good point on `throw`, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



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

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

[Aaron Ballman via Phabricator via cfe-commits]

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

In D54408#2056251 , @steveire wrote:

> @aaron.ballman  I think we agreed in Belfast in November (after the most 
> recent comment) to get this in as it is and not be as draconian about `auto`. 
> Is anything blocking this?


Nothing is blocking this, but I think we may have slightly different 
understandings of what we agreed to. Reviewers asking for code to follow the 
coding guidelines is not draconian; it's expected for code to follow the coding 
guidelines and for patch authors to field requests for changes like these. I 
re-reviewed the things I was asking for changes on and have added a comment 
where I definitely insist on changes. LGTM with that nit fixed though.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:705
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly);
+

aaron.ballman wrote:
> Don't use `auto` here please.
Please change this use of `auto` -- I have no idea what the type is for 
`NestedResult` without hunting for it and so I have no idea what the type is 
for `Item` and whether using a const reference is reasonable or not (after 
doing the hunting, it looks fine though).


Repository:
  rC Clang

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

https://reviews.llvm.org/D54408



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:106-107
+void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+

This should now be done by overriding `bool isLanguageVersionSupported(const 
LangOptions ) const override;`, see 
bugprone/ThrowKeywordMissingCheck.h for an example.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:3
+
+cppcoreguidelines-prefer-member-initializer
+===

You should probably link to the rule from somewhere in these docs.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

By my reading of the core guideline 
(https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
 it looks like `n` should also be diagnosed because all of the constructors in 
the class initialize the member to the same constant value. Is there a reason 
to deviate from the rule (or have I missed something)?

Also, I'd like to see a test case like:
```
class C {
  int n;
public:
  C() { n = 0; }
  explicit C(int) { n = 12; }
};
```


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

https://reviews.llvm.org/D71199



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

[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D79437#2052704 , @DerWaschbar wrote:

> In D79437#2052109 , @aaron.ballman 
> wrote:
>
> > Have you considered writing a static analyzer check so you can do data and 
> > control flow analysis to catch issues like these?
>
>
> I have noticed those issues too, but most likely the getter/setter will be in 
> the same function body and we could measure fast how common is that issue in 
> the wild.


That doesn't match my intuition, but if you have data, that would be excellent 
for helping to make a decision.

> Also, this was my first introductory project for Clang and with that, I can 
> rewrite this as a Static Analyzer project or start working on another 
> Clang-Tidy project.

Welcome! I think this functionality is likely useful in here as a clang-tidy 
check, but I'd be curious to see data on whether it finds true or false 
positives in the wild to help judge that. My gut instinct is that to do this 
properly, we'll want it in the static analyzer, but perhaps the tidy check is 
good enough. I'd be curious to know if others have different thoughts though 
(pinging @alexfh for visibility).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D79437



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[Erich Keane via Phabricator via cfe-commits]

erichkeane closed this revision.
erichkeane added a comment.
Herald added a subscriber: sstefan1.

Yikes, I apparently misspelled "Revision" as "revisions"!.
https://github.com/llvm/llvm-project/commit/8a1c999c9b0817d4de778a62965b4af86416e4b7


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

https://reviews.llvm.org/D79118



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

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Daniel via Phabricator via cfe-commits]

Daniel599 updated this revision to Diff 267455.
Daniel599 added a comment.
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.

I have made the needed changes in order to detect a conflict in duplicated 
fix-it of aliased checkers and also added a test for it.
Now I`ll try to work on combining aliased errors together,  any tips regarding 
this feature?


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

https://reviews.llvm.org/D80753

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
  llvm/include/llvm/ADT/StringMap.h

Index: llvm/include/llvm/ADT/StringMap.h
===
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -248,6 +248,24 @@
 return count(MapEntry.getKey());
   }
 
+  /// equal - check whether both of the containers are equal
+  bool operator==(const StringMap ) const {
+if (size() != RHS.size())
+  return false;
+
+for (const auto  : *this) {
+  auto FindInRHS = RHS.find(KeyValue.getKey());
+
+  if (FindInRHS == RHS.end())
+return false;
+
+  if (!(KeyValue.getValue() == FindInRHS->getValue()))
+return false;
+}
+
+return true;
+  }
+
   /// insert - Insert the specified key/value pair into the map.  If the key
   /// already exists in the map, return false and ignore the request, otherwise
   /// insert it and return true.
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t
+
+namespace std {
+template 
+class initializer_list {
+public:
+  initializer_list() noexcept {}
+};
+
+template 
+class vector {
+public:
+  vector() = default;
+  vector(initializer_list) {}
+
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template 
+  void emplace_back(Args &&... args){};
+  ~vector();
+};
+} // namespace std
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init]
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [hicpp-member-init]
+  // CHECK-MESSAGES: note: this fix will not be applied because an alias checker has already provided it, see 'cppcoreguidelines-pro-type-member-init'
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:
+  int _num1;
+  int _num2;
+  // CHECK-FIXES: _num2{};
+};
+
+int should_use_emplace(std::vector ) {
+  v.push_back(Foo());
+  // CHECK-FIXES: v.emplace_back();
+  // CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace]
+  // CHECK-MESSAGES: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-MESSAGES: note: this fix will not be applied because an alias checker has already provided it, see 'hicpp-use-emplace'
+}
+
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t -- \
+ RUN: -config='{CheckOptions: [ \
+ RUN: {key: cppcoreguidelines-pro-type-member-init.UseAssignment, value: 1}, \
+ RUN: ]}'
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init]
+  // CHECK-MESSAGES: note: cannot apply fix-it because an alias checker has suggested a different fix-it, please remove one of the checkers or make sure they are both configured the same. Aliased checkers: 'cppcoreguidelines-pro-type-member-init', 'hicpp-member-init'
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [hicpp-member-init]
+  // CHECK-MESSAGES: note: cannot apply fix-it because an alias checker has suggested a different fix-it, please remove one of the checkers or make sure they are both configured the same. Aliased checkers: 'cppcoreguidelines-pro-type-member-init', 'hicpp-member-init'
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

> // ...
> // RUN:   -x hip --offload-arch=gfx908 \
> // RUN:   --offload-arch=gfx908:sramecc+:xnack+
> Does this mean that HIP will create two compilation passes -- one for gfx908 
> and one for gfx908:sramecc+:xnack+ ?
> Or does it mean that the first line is ignored if you get a more detailed 
> offload arch?

It means HIP will create two compilation passes: one for gfx908 and one for 
gfx908:xnack+:sramecc+.

> One thing you'll need is a way to normalize the arch+features tuple so we can 
> compare them.

We require features in target id follow a pre-defined order. This may not be 
alphabetical order since later on we may add more features.

> What I mean -- are users free to speficy any combination of {feature[+-]} and 
> would it be expected for all/most of them to make sense to the user?
> Or does it only make sense for a few specific arch:featureA+:featureB- 
> combinations?
> If we only have a limited set of valid combinations, it would make sense to 
> give users easy-to-use names.



> I.e. if the only valid ids for gfx111 are gfx111:foo+:bar- and gfx111:buz+, 
> we could call them gfx111a and gfx111b and expand it into the right set of 
> features ourselves without relying on the users not to make a typo.

This was the scheme we used before but it did not work well.

For each GPU we have a predefined set of features. Currently some GPU's support 
xnack, some GPU's support sramecc, some GPU's support both. In the future we 
may introduce more features. If we let each GPU has its own encoding for 
features, it will be confusing since each letter will have different meanings 
depending on GPU. If we let all GPU share one encoding scheme, we are facing 
combination explosion. Most importantly, target ids are used by developers for 
whom the GPU+Features are meaningful terms to refer to a GPU configuration they 
want to compile for. For example, in daily life, we would say "we need to build 
for gfx908 with xnack on and sramecc off for this machine", then just use 
-offload-arch=gfx908:xnack+:sramecc- to compile. If we use an encoding for 
features, then developers have to look up the encoding scheme for xnack on and 
sramecc off, then use it in -offload-arch, which is inconvenient.


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

https://reviews.llvm.org/D60620



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

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D77066#2062849 , @martong wrote:

> - Remove SValBuilder param


I failed to emphasize that the point was to remove the new `CheckerContext` 
parameters :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77066



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

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG50bdd6073113: [clang-format] [PR46130] When editing a file 
with unbalance {} the namespace… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80830

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
@@ -1089,6 +1089,34 @@
 "void d() {\n"
 "}\n"));
 }
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}// namespace A\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"));
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"));
+
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n"));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@
   const SourceManager  = Env.getSourceManager();
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
+
+  // Spin through the lines and ensure we have balanced braces.
+  int Braces = 0;
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+FormatToken *Tok = AnnotatedLines[I]->First;
+while (Tok) {
+  Braces += Tok->is(tok::l_brace) ? 1 : Tok->is(tok::r_brace) ? -1 : 0;
+  Tok = Tok->Next;
+}
+  }
+  // Don't attempt to comment unbalanced braces or this can
+  // lead to comments being placed on the closing brace which isn't
+  // the matching brace of the namespace. (occurs during incomplete editing).
+  if (Braces != 0) {
+return {Fixes, 0};
+  }
+
   std::string AllNamespaceNames = "";
   size_t StartLineIndex = SIZE_MAX;
   StringRef NamespaceTokenText;


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1089,6 +1089,34 @@
 "void d() {\n"
 "}\n"));
 }
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}// namespace A\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"));
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"));
+
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n"));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@
   const SourceManager  = Env.getSourceManager();
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
+
+  // Spin through the lines and ensure we have balanced braces.
+  int Braces = 0;
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+FormatToken *Tok = AnnotatedLines[I]->First;
+while 

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

We don't really have any performance tests, but the following shows some 
analysis, all builds are release build:

- clang-format-ns is built with this change (compiled with VS2017)
- clang-format is without (compiled with VS2017)
- c:/Program\ Files/LLVM/bin/clang-format is the last Feb 2020 snapshot build 
version of clang-format  (probably compiled with clang-cl)
- build_release_cl/clang-format with this change  (compiled with clang-cl from 
Fab2020 snapshot) _ build_2019/clang-format with this change (compiled with 
VS2019)

The test is to run clang-format over the largest file in LLVM

./libcxxabi/test/test_demangle.pass.cpp is 5.2M in size = 30,000 lines, 29799 
opening and closing braces

For a file this size I think I'd expect IO to dominate (I'm running this test 
on an Intel i7-800 CPU @3,.20GHz with an SSD, Running cygwin in Windows 10)

I've run these tests multiple times and they are often the same or similar 
timings (I'm running the base clang-format as a benchmark twice because the 
first run is often slower

  time /clang/build_release/bin/clang-format 
../libcxxabi/test/test_demangle.pass.cpp > speed1 (Baseline to get 
test_demangle.pass.cpp into system cache)
  
  real0m3.957s
  user0m0.000s
  sys 0m0.000s
  
  time /clang/build_release/bin/clang-format-ns 
../libcxxabi/test/test_demangle.pass.cpp > speed1
  
  real0m3.994s
  user0m0.000s
  sys 0m0.000s
  time /clang/build_release/bin/clang-format 
../libcxxabi/test/test_demangle.pass.cpp > speed1
  
  real0m3.990s
  user0m0.000s
  sys 0m0.000s
  
  time  c:/Program\ Files/LLVM/bin/clang-format 
../libcxxabi/test/test_demangle.pass.cpp > speed1
  
  real0m3.793s
  user0m0.000s
  sys 0m0.000s
  
  time /clang/build_release_cl/bin/clang-format 
../libcxxabi/test/test_demangle.pass.cpp > speed1
  
  real0m3.442s
  user0m0.000s
  sys 0m0.000s
  
  time /clang/build_2019/bin/clang-format 
../libcxxabi/test/test_demangle.pass.cpp > speed1
  
  real0m3.820s
  user0m0.000s
  sys 0m0.000s

The speed difference of this change of clang-format is fairly negligible, 
varying by about +/-20ms one way or the other (often swinging the other between 
runs or depending on which goes first)

I feel this change is relatively low impact, (perhaps the loop could be moved 
into some earlier equivalent loop but for clarity, I think it's ok here for 
now.)

I'm not 100% sure how we could reliably test performance generally in 
clang-format, but I think it would be good to have something.

The key takeaway is likely that clang-cl is quicker! and it obviously more 
about which compiler you choose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80830



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

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

OK, great. Thanks for all the information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80830



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

[clang] 50bdd60 - [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[via cfe-commits]

Author: mydeveloperday
Date: 2020-05-30T13:15:27+01:00
New Revision: 50bdd60731130dbde81fa477ba8916c58039d73a

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

LOG: [clang-format] [PR46130] When editing a file with unbalance {} the 
namespace comment fixer can incorrectly comment the wrong closing brace

Summary:
https://bugs.llvm.org/show_bug.cgi?id=46130   from Twitter 
https://twitter.com/ikautak/status/1265998988232159232

I have seen this myself many times.. if you have format on save and you work in 
an editor where you are constantly saving (:w muscle memory)

If you are in the middle of editing and somehow you've missed a { or } in your 
code, somewhere, often way below where you are at the bottom of your file the 
namespace comment fixer will have put the namespace on the previous closing 
brace.

This leads to you having to fix up the bottom of the file.

This revision prevents that happening by performing an initial pass of the 
tokens and simply counting the number of `{` and `}`  and ensuring they balance.

If they don't balance we don't do any namespace fixing as it will likely be 
unstable and incorrect.

Reviewed By: curdeius

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp 
b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index 92707150fcdb..97de45bd1965 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@ std::pair 
NamespaceEndCommentsFixer::analyze(
   const SourceManager  = Env.getSourceManager();
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
+
+  // Spin through the lines and ensure we have balanced braces.
+  int Braces = 0;
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+FormatToken *Tok = AnnotatedLines[I]->First;
+while (Tok) {
+  Braces += Tok->is(tok::l_brace) ? 1 : Tok->is(tok::r_brace) ? -1 : 0;
+  Tok = Tok->Next;
+}
+  }
+  // Don't attempt to comment unbalanced braces or this can
+  // lead to comments being placed on the closing brace which isn't
+  // the matching brace of the namespace. (occurs during incomplete editing).
+  if (Braces != 0) {
+return {Fixes, 0};
+  }
+
   std::string AllNamespaceNames = "";
   size_t StartLineIndex = SIZE_MAX;
   StringRef NamespaceTokenText;

diff  --git a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp 
b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
index fee8597b4330..463afa67e8b0 100644
--- a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1089,6 +1089,34 @@ TEST_F(NamespaceEndCommentsFixerTest, 
HandlesInlineAtEndOfLine_PR32438) {
 "void d() {\n"
 "}\n"));
 }
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}// namespace A\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"));
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"));
+
+  EXPECT_EQ("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n",
+fixNamespaceEndComments("namespace A {\n"
+"class Foo {\n"
+"}\n"
+"}\n"
+"}\n"));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang



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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 267450.
tambre marked 4 inline comments as done.
tambre added a comment.

Fix some intrinsics not being marked as builtin due to being static in the 
headers.
Make some code easier to read, fix test for sigsetjmp in 
Sema/implicit-builtin-decl.c to reflect the original intent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly 

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[Raul Tambre via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Decl.cpp:3169-3175
   } else {
-if (!getIdentifier())
+const auto *Attr = getAttr();
+
+if (!Attr)
   return 0;
 
+BuiltinID = Attr->getID();

aaron.ballman wrote:
> I think this is a bit more clear:
> ```
> } else if (const auto *A = getAttr()) {
>   BuiltinID = A->getID();
> }
> ```
> and initialize `BuiltinID` to zero above.
Done.



Comment at: clang/test/CodeGen/callback_pthread_create.c:17
 
+// FIXME: How to do builtin handling for this?
 int pthread_create(pthread_t *, const pthread_attr_t *,

As many others prior to this patch, `pthread_create` was recognized as a 
builtin due to its name and thus had attributes applied.
Unlike others however, `pthread_create` is the only builtin in `Builtins.def` 
that doesn't have its arguments specified. Doing that would require 
implementing support for function pointers in the builtin database and adding 
yet another special case for `pthread_t` and `pthread_attr_t`.
That'd be quite a bit of work, which I'm not interested in doing.

How about simply removing the hack that is the `pthread_create` builtin entry 
and disabling/removing this test?



Comment at: clang/test/Sema/implicit-builtin-decl.c:64
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration 
of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' 
type, commonly provided in the header .}}
 

aaron.ballman wrote:
> It looks like we're losing test coverage with this change?
Indeed. I've reverted this change and changed the test to instead not test for 
it being recognized as a builtin, since it shouldn't be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



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

[PATCH] D79800: [Sema] Implement DR2233

[Raul Tambre via Phabricator via cfe-commits]

tambre abandoned this revision.
tambre added a comment.

In D79800#2058985 , @rsmith wrote:

> Unfortunately, I think this approach basically can't work, because we need to 
> consider inheriting default arguments from a previous (or subsequent!) 
> declaration before dropping default arguments preceding a pack. I think we 
> will instead need to detect this situation when issuing the diagnostic for a 
> parameter with a default argument followed by a parameter without one, and 
> will need to make sure that all the parts of Clang that look at default 
> arguments can cope with them being discontiguous.


Thanks for reviewing!
Abandoning, as I lack the motivation to implement the proper solution you laid 
out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800



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

[PATCH] D80829: [OpenMP][SYCL] Do not crash on attempt to diagnose unsupported type use

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd85b7d66887: [OpenMP][SYCL] Do not crash on attempt to 
diagnose unsupported type use (authored by Fznamznon, committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80829

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp


Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -120,3 +120,14 @@
 long double qa, qb;
 decltype(qa + qb) qc;
 double qd[sizeof(-(-(qc * 2)))];
+
+struct A { };
+
+template 
+struct A_type { typedef A type; };
+
+template 
+struct B {
+  enum { value = bool(Sp::value) || bool(Tp::value) };
+  typedef typename A_type::type type;
+};
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1725,6 +1725,9 @@
   }
 
   auto CheckType = [&](QualType Ty) {
+if (Ty->isDependentType())
+  return;
+
 if ((Ty->isFloat16Type() && !Context.getTargetInfo().hasFloat16Type()) ||
 ((Ty->isFloat128Type() ||
   (Ty->isRealFloatingType() && Context.getTypeSize(Ty) == 128)) &&


Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -120,3 +120,14 @@
 long double qa, qb;
 decltype(qa + qb) qc;
 double qd[sizeof(-(-(qc * 2)))];
+
+struct A { };
+
+template 
+struct A_type { typedef A type; };
+
+template 
+struct B {
+  enum { value = bool(Sp::value) || bool(Tp::value) };
+  typedef typename A_type::type type;
+};
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1725,6 +1725,9 @@
   }
 
   auto CheckType = [&](QualType Ty) {
+if (Ty->isDependentType())
+  return;
+
 if ((Ty->isFloat16Type() && !Context.getTargetInfo().hasFloat16Type()) ||
 ((Ty->isFloat128Type() ||
   (Ty->isRealFloatingType() && Context.getTypeSize(Ty) == 128)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bd85b7d - [OpenMP][SYCL] Do not crash on attempt to diagnose unsupported type use

[Alexey Bader via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2020-05-30T12:27:58+03:00
New Revision: bd85b7d6688725e854a694f9f3e8baa6a3077a4a

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

LOG: [OpenMP][SYCL] Do not crash on attempt to diagnose unsupported type use

Summary:
Do not ask size of type if it is dependent. ASTContext doesn't seem expecting
this.

Reviewers: jdoerfert, ABataev, bader

Reviewed By: ABataev

Subscribers: yaxunl, guansong, ebevhan, Anastasia, sstefan1, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/Sema.cpp
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 8c11a1a59e9c..ffe2e4d4d56a 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1725,6 +1725,9 @@ void Sema::checkDeviceDecl(const ValueDecl *D, 
SourceLocation Loc) {
   }
 
   auto CheckType = [&](QualType Ty) {
+if (Ty->isDependentType())
+  return;
+
 if ((Ty->isFloat16Type() && !Context.getTargetInfo().hasFloat16Type()) ||
 ((Ty->isFloat128Type() ||
   (Ty->isRealFloatingType() && Context.getTypeSize(Ty) == 128)) &&

diff  --git a/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp 
b/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
index 22ce8175fd05..e56105adeb83 100644
--- a/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ b/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -120,3 +120,14 @@ void hostFoo() {
 long double qa, qb;
 decltype(qa + qb) qc;
 double qd[sizeof(-(-(qc * 2)))];
+
+struct A { };
+
+template 
+struct A_type { typedef A type; };
+
+template 
+struct B {
+  enum { value = bool(Sp::value) || bool(Tp::value) };
+  typedef typename A_type::type type;
+};



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 267447.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-arch.c
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
  llvm/lib/Target/RISCV/RISCVInstrFormatsV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoV.td
  llvm/test/MC/RISCV/rvv/convert.s
  llvm/test/MC/RISCV/rvv/ext.s
  llvm/test/MC/RISCV/rvv/fothers.s
  llvm/test/MC/RISCV/rvv/invalid.s
  llvm/test/MC/RISCV/rvv/load.s
  llvm/test/MC/RISCV/rvv/mask.s
  llvm/test/MC/RISCV/rvv/snippet.s
  llvm/test/MC/RISCV/rvv/store.s
  llvm/test/MC/RISCV/rvv/vsetvl.s

Index: llvm/test/MC/RISCV/rvv/vsetvl.s
===
--- llvm/test/MC/RISCV/rvv/vsetvl.s
+++ llvm/test/MC/RISCV/rvv/vsetvl.s
@@ -8,12 +8,72 @@
 # RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+experimental-v < %s \
 # RUN:| llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-UNKNOWN
 
+vsetvli a2, a0, e32,m1
+# CHECK-INST: vsetvli a2, a0, e32,m1
+# CHECK-ENCODING: [0x57,0x76,0x85,0x00]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 85 00 
+
+vsetvli a2, a0, e32,m2
+# CHECK-INST: vsetvli a2, a0, e32,m2
+# CHECK-ENCODING: [0x57,0x76,0x95,0x00]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 95 00 
+
 vsetvli a2, a0, e32,m4
 # CHECK-INST: vsetvli a2, a0, e32,m4
 # CHECK-ENCODING: [0x57,0x76,0xa5,0x00]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 57 76 a5 00 
 
+vsetvli a2, a0, e32,m8
+# CHECK-INST: vsetvli a2, a0, e32,m8
+# CHECK-ENCODING: [0x57,0x76,0xb5,0x00]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 b5 00 
+
+vsetvli a2, a0, e32,mf2
+# CHECK-INST: vsetvli a2, a0, e32,mf2
+# CHECK-ENCODING: [0x57,0x76,0xb5,0x02]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 b5 02 
+
+vsetvli a2, a0, e32,mf4
+# CHECK-INST: vsetvli a2, a0, e32,mf4
+# CHECK-ENCODING: [0x57,0x76,0xa5,0x02]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 a5 02 
+
+vsetvli a2, a0, e32,mf8
+# CHECK-INST: vsetvli a2, a0, e32,mf8
+# CHECK-ENCODING: [0x57,0x76,0x95,0x02]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 95 02 
+
+vsetvli a2, a0, e32,m1,ta,ma
+# CHECK-INST: vsetvli a2, a0, e32,m1,ta,ma
+# CHECK-ENCODING: [0x57,0x76,0x85,0x0c]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 85 0c 
+
+vsetvli a2, a0, e32,m1,tu,ma
+# CHECK-INST: vsetvli a2, a0, e32,m1,tu,ma
+# CHECK-ENCODING: [0x57,0x76,0x85,0x08]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 85 08 
+
+vsetvli a2, a0, e32,m1,ta,mu
+# CHECK-INST: vsetvli a2, a0, e32,m1,ta,mu
+# CHECK-ENCODING: [0x57,0x76,0x85,0x04]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 85 04 
+
+vsetvli a2, a0, e32,m1,tu,mu
+# CHECK-INST: vsetvli a2, a0, e32,m1
+# CHECK-ENCODING: [0x57,0x76,0x85,0x00]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 85 00 
+
 vsetvl a2, a0, a1
 # CHECK-INST: vsetvl a2, a0, a1
 # CHECK-ENCODING: [0x57,0x76,0xb5,0x80]
Index: llvm/test/MC/RISCV/rvv/store.s
===
--- llvm/test/MC/RISCV/rvv/store.s
+++ llvm/test/MC/RISCV/rvv/store.s
@@ -8,200 +8,296 @@
 # RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+experimental-v < %s \
 # RUN:| llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-UNKNOWN
 
-vsb.v v24, (a0), v0.t
-# CHECK-INST: vsb.v v24, (a0), v0.t
+vse8.v v24, (a0), v0.t
+# CHECK-INST: vse8.v v24, (a0), v0.t
 # CHECK-ENCODING: [0x27,0x0c,0x05,0x00]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 27 0c 05 00 
 
-vsb.v v24, (a0)
-# CHECK-INST: vsb.v v24, (a0)
+vse8.v v24, (a0)
+# CHECK-INST: vse8.v v24, (a0)
 # CHECK-ENCODING: [0x27,0x0c,0x05,0x02]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 27 0c 05 02 
 
-vsh.v v24, (a0), v0.t
-# CHECK-INST: vsh.v v24, (a0), v0.t
+vse16.v v24, (a0), v0.t
+# CHECK-INST: vse16.v v24, (a0), v0.t
 # CHECK-ENCODING: [0x27,0x5c,0x05,0x00]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 27 5c 05 00 
 
-vsh.v v24, (a0)
-# CHECK-INST: vsh.v v24, (a0)
+vse16.v v24, (a0)
+# CHECK-INST: vse16.v v24, (a0)
 # CHECK-ENCODING: [0x27,0x5c,0x05,0x02]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)