[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103440#2792814 , @NoQ wrote:

> Our pre-commit testing shows that the new tests fail, eg.:
>
> https://buildkite.com/llvm-project/premerge-checks/builds/41391#b557c86c-0587-4aee-a06b-8a4de6d771c4
>
>   Failed Tests (1):
> Clang :: Analysis/constant-folding.c
>
> Did you figure out how to *run* the tests locally? Tests for the static 
> analyzer are in the `check-clang-analysis` target.

They are actually supposed to fail, we first decide on the scope of what @manas 
will support and implement the actual feature later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103440

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

[PATCH] D101554: [clangd] Add support for the `defaultLibrary` semantic token modifier

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:436
+  if (HB.inDefaultLibrary(D->getLocation()))
+return HighlightingModifier::DefaultLibrary;
   const DeclContext *DC = D->getDeclContext();

dgoldman wrote:
> sammccall wrote:
> > dgoldman wrote:
> > > sammccall wrote:
> > > > kadircet wrote:
> > > > > I don't use semantic highlighting enough to judge whether it is more 
> > > > > useful to say `DefaultLibrary` than `{Class,Function}Scope`. (i.e. 
> > > > > having the check here vs at the end of the function as a fallback). 
> > > > > Can you provide some rationale about the decision (probably as 
> > > > > comments in code)?
> > > > > 
> > > > > By looking at the testcase, I suppose you are trying to indicate 
> > > > > "overriden"/"extended" attribute of a symbol, which makes sense but 
> > > > > would be nice to know if there's more. I am just worried that this is 
> > > > > very obj-c specific and won't really generalize to c/c++. As most of 
> > > > > the system headers only provide platform specific structs (e.g. 
> > > > > posix) and they are almost never extended. So it might be confusing 
> > > > > for user to see different colors on some memberexprs.
> > > > I think default-library isn't a scope modifier at all, it should be 
> > > > independent.
> > > > 
> > > > e.g. std::string is defaultlLibrary | globalScope, while 
> > > > std::string::push_back() is defaultLibrary | classScope.
> > > I can break it up if you'd like - just let me know what you had in mind. 
> > > I can change the scopeModifier function's name and then have it return a 
> > > list of modifiers or I can just add a new function which all existing 
> > > callers of scopeModifier will also need to call.
> > > 
> > > And what we're really trying to do here is color the system/SDK symbols 
> > > differently - this is definitely useful for iOS dev since Apple has a 
> > > very large SDK, but if it's not useful for C++ maybe we could just make 
> > > this configurable?
> > > just let me know what you had in mind
> > 
> > I think this should follow `isStatic()` etc: just a boolean function that 
> > gets called in the minimum number of places needed.
> > 
> > scopeModifier is slightly different pattern because there are a bunch of 
> > mutually-exclusive modifiers. defaultLibrary is (mostly) independent of 
> > other modifiers.
> > 
> > > if it's not useful for C++ maybe we could just make this configurable
> > 
> > No need really - sending more modifiers is cheap (one string at startup and 
> > one bit in an integer we're sending anyway).
> > It's up to clients to style the ones they care about, and at least in 
> > VSCode this can be customized per-lang I think. Important thing is that 
> > they don't interact with other modifiers.
> Swapped over, currently don't check on deduced types but we could add our 
> isDefaultLibrary check there as well
I think it'd be useful to do this for consistency, just VisitDecltypeTypeLoc 
and VisitDeclaratorDecl need to be changed I think.

We'd want a version of isDefaultLibrary that works on Type. We're only going to 
see canonical types so I think we need to:
 - unwrap pointers (getPointeeOrArrayElementType)
 - if BuiltinType -> return true
 - for types: find the decl and dispatch to isDefaultLibrary(Decl)

to find the decl it'd be nice to use targetDecl() but also sufficient to handle 
TagType + ObjCInterfaceType for now I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101554

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

[PATCH] D102914: [analyzer] Make checker silencing work for non-pathsensitive bug reports

[Gabor Marton via Phabricator via cfe-commits]

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

This makes sense, and looks good to me! Though, my review confidence is weak.


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

https://reviews.llvm.org/D102914

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> I understand it's a little bit confusing here, but I was simply trying to 
> match GCC's behavior (please see the example in my last comment) unless I 
> misunderstood its output. I definitely agree having consistent behaviors 
> between Arm and Aarch64 in Clang is more reasonable (in fact that was what I 
> implemented at first) and maybe we should fork from gcc, WDYT?

If gas handles `march` by choosing the last value, then we don't need to emit 
multiple values just to look more like GCC's processed arguments.

Let me check what gas does when you run it directly, I think that's the best 
way to guide things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 349199.
DmitryPolukhin added a comment.

Updated HeaderMapImpl::reverseLookupFilename according to the review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Lex/HeaderSearch.h"
+#include "HeaderMapTestUtils.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -45,6 +46,21 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void addHeaderMap(llvm::StringRef Filename,
+std::unique_ptr Buf) {
+VFS->addFile(Filename, 0, std::move(Buf), /*User=*/None, /*Group=*/None,
+ llvm::sys::fs::file_type::regular_file);
+auto FE = FileMgr.getFile(Filename, true);
+assert(FE);
+
+// Test class supports only one HMap at a time.
+assert(!HMap);
+HMap = HeaderMap::Create(*FE, FileMgr);
+auto DL =
+DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
+Search.AddSearchPath(DL, /*isAngled=*/false);
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -55,6 +71,7 @@
   std::shared_ptr TargetOpts;
   IntrusiveRefCntPtr Target;
   HeaderSearch Search;
+  std::unique_ptr HMap;
 };
 
 TEST_F(HeaderSearchTest, NoSearchDir) {
@@ -136,5 +153,31 @@
 "y/z/t.h");
 }
 
+// Helper struct with null terminator character to make MemoryBuffer happy.
+template 
+struct NullTerminatedFile : public FileTy {
+  PaddingTy Padding = 0;
+};
+
+TEST_F(HeaderSearchTest, HeaderMapReverseLookup) {
+  typedef NullTerminatedFile, char> FileTy;
+  FileTy File;
+  File.init();
+
+  test::HMapFileMockMaker Maker(File);
+  auto a = Maker.addString("d.h");
+  auto b = Maker.addString("b/");
+  auto c = Maker.addString("c.h");
+  Maker.addBucket("d.h", a, b, c);
+
+  addHeaderMap("/x/y/z.hmap", File.getBuffer());
+  addSearchDir("/a");
+
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c.h",
+   /*WorkingDir=*/"",
+   /*MainFile=*/""),
+"d.h");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1834,7 +1834,7 @@
   };
 
   for (unsigned I = 0; I != SearchDirs.size(); ++I) {
-// FIXME: Support this search within frameworks and header maps.
+// FIXME: Support this search within frameworks.
 if (!SearchDirs[I].isNormalDir())
   continue;
 
@@ -1848,6 +1848,19 @@
   if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
 *IsSystem = false;
 
+  // Try resolving resulting filename via reverse search in header maps,
+  // key from header name is user prefered name for the include file.
+  StringRef Filename = File.drop_front(BestPrefixLength);
+  for (unsigned I = 0; I != SearchDirs.size(); ++I) {
+if (!SearchDirs[I].isHeaderMap())
+  continue;
 
-  return path::convert_to_slash(File.drop_front(BestPrefixLength));
+StringRef SpelledFilename =
+SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
+if (!SpelledFilename.empty()) {
+  Filename = SpelledFilename;
+  break;
+}
+  }
+  return path::convert_to_slash(Filename);
 }
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -240,3 +240,32 @@
 return StringRef(DestPath.begin(), DestPath.size());
   }
 }
+
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+  if (!ReverseMap.empty())
+return ReverseMap.lookup(DestPath);
+
+  const HMapHeader &Hdr = getHeader();
+  unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
+  StringRef RetKey;
+  for (unsigned i = 0; i != NumBuckets; ++i) {
+HMapBucket B = getBucket(i);
+if (B.Key == HMAP_EmptyBucketKey)
+  continue;
+
+Optional Key = getString(B.Key);
+Optional Prefix = getString(B.Prefix);
+Optional Suffix = getString(B.Suffix);
+if (LLVM_LIKELY(Key && Prefix && Suffix)) {
+  SmallVector Buf;
+  Buf.append(Prefix->begin(), Prefix->end());
+  Buf.append(Suffix->begin(), Suffix->end());
+  StringRef Value(Buf.begin(), Buf.size());
+  ReverseMap[Value] = *Key;
+
+  if (

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added inline comments.



Comment at: clang/lib/Lex/HeaderMap.cpp:265
+  }
+  return ReverseMap.lookup(DestPath);
+}

bruno wrote:
> How about something along the lines of:
> 
> ```
> ...
> StringRef Key;
> for (unsigned i = 0; i != NumBuckets; ++i) {
> HMapBucket B = getBucket(i);
> if (B.Key == HMAP_EmptyBucketKey)
>   continue;
> 
> Key = getString(B.Key);
> Optional Prefix = getString(B.Prefix);
> Optional Suffix = getString(B.Suffix);
> if (!Key.empty() && Prefix && Suffix) {
>   SmallVector Buf;
>   Buf.append(Prefix->begin(), Prefix->end());
>   Buf.append(Suffix->begin(), Suffix->end());
>   ReverseMap[StringRef(Buf.begin(), Buf.size())] = Key;
>   break;
> }
> }
> assert(!Key.empty() && "expected valid key");
> return Key;
> ```
> 
> While proposing this change I've noticed that it would keep looking for other 
> buckets even in face of a valid result, so I've added a `break`. Was that 
> intentional? 
Yes, keep iterating over all key-value pairs in the header map is important to 
add all elements to the map for subsequent lookups. If we stop on the current 
match, next lookup will not even try to populate the reverse lookup map 
(because the map is not empty) but not all elements are in the map due to early 
return from previous lookup, so subsequent lookup may not find the match even 
if it exists in the header map. We also cannot expect that DestPath is present 
in the given header map, so we cannot assert if it is missing. This function 
should return empty string as an indication that DestPath is not found.

Updated implementation with the review suggestion, please take another look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

  $ cat /tmp/test.s
  irg x0, x0
  $ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a 
/tmp/test.s -o /dev/null
  /tmp/test.s: Assembler messages:
  /tmp/test.s:1: Error: selected processor does not support `irg x0,x0'

GAS is also taking the last value of `march`. So if we were to follow clang 
Arm's behaviour, the result would be the same.

Except we don't have to rely on the llvm backend picking the last of 
"target-feature". Which probably works for architecture versions but for 
individual extensions I don't think so.

If we make it additive this could happen:

  -march=armv8.5-a+memtag -march=armv8.2-a

Becomes: (mte is the backend name for memtag for silly reasons)

  -target-feature +v8.5-a -target-feature mte -target-feature +v8.2-a

So now instead of getting `v8.2` only, the user now has `v8.2+memtag`. Only if 
they do `armv8.2-a+nomemtag` will they get what they expect (well, what I'd 
expect). Which means they'd need to know every previous march value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D102492: [clang][AST] Add support for BindingDecl to ASTImporter.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I like it, though I've found a nit.




Comment at: clang/lib/AST/ASTImporter.cpp:2301
+  ToD->setLexicalDeclContext(LexicalDC);
+  DC->addDeclInternal(ToD);
+  ToD->setBinding(ToType, ToBinding);

Should we use rather `addDeclToContexts` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102492

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

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

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

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

The `cppcoreguidelines-special-member-functions` check has the 
`AllowSoleDefaultDtor` checker option.
It is `false` by default, and now I'm proposing to change it to `true`.
Our CodeChecker users frequently find it surprising that the check reports for 
cases like this by default:

  struct A {
virtual ~A() = default;
  };

---

Quoting from the Cpp CoreGuidelines C.21: If you define or =delete any copy, 
move, or destructor function, define or =delete them all 
:

> **Example, good** When a destructor needs to be declared just to make it 
> virtual, it can be defined as defaulted.
>
>   class AbstractBase {
>   public:
>   virtual ~AbstractBase() = default;
>   // ...
>   };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103511

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
-
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- 
-config="{CheckOptions: [{key: 
cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 
false}]}" --
 class DefinesDestructor {
   ~DefinesDestructor();
 };
Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
@@ -43,7 +43,6 @@
   MissingCopyOperator(const MissingCopyOperator &) = delete;
 };
 
-// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingAll' defines a 
default destructor but does not define a copy constructor, a copy assignment 
operator, a move constructor or a move assignment operator 
[cppcoreguidelines-special-member-functions]
 class MissingAll {
   ~MissingAll() = default;
 };
Index: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -25,7 +25,7 @@
 
 .. option:: AllowSoleDefaultDtor
 
-   When set to `true` (default is `false`), this check doesn't flag classes 
with a sole, explicitly
+   When set to `true` (default is `true`), this check doesn't flag classes 
with a sole, explicitly
defaulted destructor. An example for such a class is:

.. code-block:: c++
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -25,7 +25,7 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context), AllowMissingMoveFunctions(Options.get(
  "AllowMissingMoveFunctions", false)),
-  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", false)),
+  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", true)),
   AllowMissingMoveFunctionsWhenCopyIsDeleted(
   Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", false)) {}
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions.cpp
+++ clang-t

[PATCH] D103028: [llvm][ARM] Add CPU defs for arm2/3/6/7m

[David Spickett via Phabricator via cfe-commits]

DavidSpickett updated this revision to Diff 349221.
DavidSpickett added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of adding CPUs to llvm, remove them from clang.

If anyone was using them they weren't making any difference,
apart from generating warnings.

I checked the linux tree and found only 2 similair strings.
A Freescale "Armardillo2" aka arm2 board, which is actually using
a cortex A-9, and a comment in a file for the ARM720 which
is actually an ARM7TDMI-S CPU.

Whether we generate correct code for pre v4 is a
question for a different time, so the architectures remain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103028

Files:
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/unittests/Support/TargetParserTest.cpp


Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -124,10 +124,6 @@
 ARMCPUTestParams("invalid", "invalid", "invalid", ARM::AEK_NONE, ""),
 ARMCPUTestParams("generic", "invalid", "none", ARM::AEK_NONE, ""),
 
-ARMCPUTestParams("arm2", "armv2", "none", ARM::AEK_NONE, "2"),
-ARMCPUTestParams("arm3", "armv2a", "none", ARM::AEK_NONE, "2A"),
-ARMCPUTestParams("arm6", "armv3", "none", ARM::AEK_NONE, "3"),
-ARMCPUTestParams("arm7m", "armv3m", "none", ARM::AEK_NONE, "3M"),
 ARMCPUTestParams("arm8", "armv4", "none", ARM::AEK_NONE, "4"),
 ARMCPUTestParams("arm810", "armv4", "none", ARM::AEK_NONE, "4"),
 ARMCPUTestParams("strongarm", "armv4", "none", ARM::AEK_NONE, "4"),
@@ -388,7 +384,7 @@
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | 
ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 90;
+static constexpr unsigned NumARMCPUArchs = 86;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
@@ -420,16 +416,16 @@
 
 TEST(TargetParserTest, testARMArch) {
   EXPECT_TRUE(
-  testARMArch("armv2", "arm2", "v2",
+  testARMArch("armv2", "generic", "v2",
   ARMBuildAttrs::CPUArch::Pre_v4));
   EXPECT_TRUE(
-  testARMArch("armv2a", "arm3", "v2a",
+  testARMArch("armv2a", "generic", "v2a",
   ARMBuildAttrs::CPUArch::Pre_v4));
   EXPECT_TRUE(
-  testARMArch("armv3", "arm6", "v3",
+  testARMArch("armv3", "generic", "v3",
   ARMBuildAttrs::CPUArch::Pre_v4));
   EXPECT_TRUE(
-  testARMArch("armv3m", "arm7m", "v3m",
+  testARMArch("armv3m", "generic", "v3m",
   ARMBuildAttrs::CPUArch::Pre_v4));
   EXPECT_TRUE(
   testARMArch("armv4", "strongarm", "v4",
@@ -535,10 +531,6 @@
 }
 
 TEST(TargetParserTest, testARMExtension) {
-  EXPECT_FALSE(testARMExtension("arm2", ARM::ArchKind::INVALID, "thumb"));
-  EXPECT_FALSE(testARMExtension("arm3", ARM::ArchKind::INVALID, "thumb"));
-  EXPECT_FALSE(testARMExtension("arm6", ARM::ArchKind::INVALID, "thumb"));
-  EXPECT_FALSE(testARMExtension("arm7m", ARM::ArchKind::INVALID, "thumb"));
   EXPECT_FALSE(testARMExtension("strongarm", ARM::ArchKind::INVALID, "dsp"));
   EXPECT_FALSE(testARMExtension("arm7tdmi", ARM::ArchKind::INVALID, "dsp"));
   EXPECT_FALSE(testARMExtension("arm10tdmi",
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -201,10 +201,6 @@
 #ifndef ARM_CPU_NAME
 #define ARM_CPU_NAME(NAME, ID, DEFAULT_FPU, IS_DEFAULT, DEFAULT_EXT)
 #endif
-ARM_CPU_NAME("arm2", ARMV2, FK_NONE, true, ARM::AEK_NONE)
-ARM_CPU_NAME("arm3", ARMV2A, FK_NONE, true, ARM::AEK_NONE)
-ARM_CPU_NAME("arm6", ARMV3, FK_NONE, true, ARM::AEK_NONE)
-ARM_CPU_NAME("arm7m", ARMV3M, FK_NONE, true, ARM::AEK_NONE)
 ARM_CPU_NAME("arm8", ARMV4, FK_NONE, false, ARM::AEK_NONE)
 ARM_CPU_NAME("arm810", ARMV4, FK_NONE, false, ARM::AEK_NONE)
 ARM_CPU_NAME("strongarm", ARMV4, FK_NONE, true, ARM::AEK_NONE)
Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -1,7 +1,7 @@
 // RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix ARM
 // ARM: error: unknown target CPU 'not-a-cpu'
 // ARM: note: valid target CPU values are:
-// ARM-SAME: arm2
+// ARM-SAME: arm8
 
 // RUN: not %clang_cc1 -triple arm64--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix AARCH64
 // AARCH64: error: unknown target CPU 'not-a-cpu'


Index: llvm/unittests/Support/TargetParserTest.cpp
===

[PATCH] D103028: [clang][ARM] Remove arm2/3/6/7m CPU names

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added inline comments.



Comment at: clang/test/Misc/target-invalid-cpu-note.c:4
 // ARM: note: valid target CPU values are:
-// ARM-SAME: arm2
+// ARM-SAME: arm8
 

Despite the triple being armv5, clang just prints every CPU it knows about. The 
arm8 is now the first one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103028

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

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Chris Lattner via Phabricator via cfe-commits]

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

> This required adding quite a few additional includes, as many files were 
> relying on various things pulled in by ArrayRef.h.

This is a _good_ thing btw!

Thank you for driving this!




Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600
+  if (RHS.data() == getEmptyKey().data())
+return LHS.data() == getEmptyKey().data();
+  if (RHS.data() == getTombstoneKey().data())
+return LHS.data() == getTombstoneKey().data();
+  return LHS == RHS;
+}

I'm pretty sure this method can just be "return LHS == RHS;"  The 
tombstone/empty comparisons should work without special cases.



Comment at: llvm/lib/CodeGen/MBFIWrapper.cpp:14
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/CodeGen/MBFIWrapper.h"

This clang format warning is right, plz move under the other #include



Comment at: llvm/lib/Support/SmallPtrSet.cpp:15
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/MathExtras.h"

These are also right, plz fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[George Burgess IV via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf49cae278b4: [Clang] -Wunused-but-set-parameter and 
-Wunused-but-set-variable (authored by mbenfield, committed by 
george.burgess.iv).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  clang/test/CodeGen/2007-10-30-Volatile.c
  clang/test/CodeGen/X86/x86_32-xsave.c
  clang/test/CodeGen/X86/x86_64-xsave.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-riscv.c
  clang/test/FixIt/fixit.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/shift.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables.c
  clang/test/SemaCXX/goto.cpp
  clang/test/SemaCXX/shift.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
  clang/test/SemaObjC/foreach.m

Index: clang/test/SemaObjC/foreach.m
===
--- clang/test/SemaObjC/foreach.m
+++ clang/test/SemaObjC/foreach.m
@@ -1,4 +1,4 @@
-/* RUN: %clang_cc1 -Wall -fsyntax-only -verify -std=c89 -pedantic %s
+/* RUN: %clang_cc1 -Wall -Wno-unused-but-set-variable -fsyntax-only -verify -std=c89 -pedantic %s
  */
 
 @class NSArray;
Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+struct __attribute__((warn_unused)) SWarnUnused {
+  int j;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  // In C++, don't warn for structs. (following gcc's behavior)
+  struct S s;
+  struct S t;
+  s = t;
+
+  // Unless it's marked with the warn_unused attribute.
+  struct SWarnUnused swu; // expected-warning{{variable 'swu' set but not used}}
+  struct SWarnUnused swu2;
+  swu = swu2;
+
+  int x;
+  x = 0;
+  return x + 5;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
+
+void f2() {
+  // Don't warn for either of these cases.
+  constexpr int x = 2;
+  const int y = 1;
+  char a[x];
+  char b[y];
+}
Index: clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+// In C++, don't warn for a struct (following gcc).
+void f3(struct S s) {
+  struct S t;
+  s = t;
+}
+
+// Also don't warn for a reference.
+void f4(int &x) {
+  x = 0;
+}
+
+// Make sure this doesn't warn.
+struct A {
+  int i;
+  A(int j) : i(j) {}
+};
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std

RE: [EXTERNAL] [PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Ten Tzen via cfe-commits]

Hi, Alexandre
Thank you for reporting this .  From the callstack it does not seem related to 
this patch.  
Could you share the repro case @__compile.rsp temp.i with me?
Thanks,

--Ten

-Original Message-
From: Alexandre Ganea via Phabricator  
Sent: Tuesday, June 1, 2021 12:44 PM
To: Ten Tzen ; andrew.kay...@intel.com; r...@google.com; 
david.majne...@gmail.com; pengfei.w...@intel.com; eli.fried...@gmail.com; 
Joseph Tremoulet ; aaron.lee.sm...@gmail.com; 
jdoerf...@anl.gov; lebedev...@gmail.com; rich...@metafoo.co.uk; 
aaron.ball...@gmail.com; rjmcc...@gmail.com; jan_svob...@apple.com
Cc: markus.boec...@gmail.com; alexandre.ga...@ubisoft.com; 
reviews.llvm@jfbastien.com; belli...@codingworkshop.eu.org; 
dexonsm...@apple.com; aeuba...@google.com; riccib...@gmail.com; 
dany.grumb...@gmail.com; nemanja.i@gmail.com; kristof.be...@arm.com; 
kit.bar...@gmail.com; cfe-commits@lists.llvm.org; llvm-comm...@lists.llvm.org; 
bhuvanendra.kum...@amd.com; ...@gmail.com; dougp...@gmail.com; 
mlek...@skidmore.edu; blitzrak...@gmail.com; shen...@google.com; 
ztur...@roblox.com; yuanfang.c...@sony.com; david.gr...@arm.com; 
kuh...@google.com
Subject: [EXTERNAL] [PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING 
(MSVC -EHa) - Part 1

aganea added a comment.

Hello, this patch introduces a regression in one of our codebases. The 
compilation of several TUs fails with the following callstack:

  PLEASE submit a bug report to 
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.llvm.org%2F&data=04%7C01%7Ctentzen%40microsoft.com%7C0d1857a9e02f491b171a08d92535a510%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637581734644682500%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=aaj5xRDyKWxqKzJHHsxMLCxa%2BuEu%2FSuNy5X7%2FxTUuFs%3D&reserved=0
 and include the crash backtrace, preprocessed source, and associated run 
script.
  Stack dump:
  0.  Program arguments: /media/project2/llvm-project/stage0/bin/clang-cl 
@__compile.rsp temp.i
  1.   parser at end of file
  2.  Optimizer
   #0 0x043c2633 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/media/project2/llvm-project/llvm/lib/Support/Unix/Signals.inc:565:13
   #1 0x043c08f0 llvm::sys::RunSignalHandlers() 
/media/project2/llvm-project/llvm/lib/Support/Signals.cpp:77:18
   #2 0x043c1d7d llvm::sys::CleanupOnSignal(unsigned long) 
/media/project2/llvm-project/llvm/lib/Support/Unix/Signals.inc:0:3
   #3 0x04343206 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
/media/project2/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:75:5
   #4 0x04343206 CrashRecoverySignalHandler(int) 
/media/project2/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:388:51
   #5 0x7f58152c4a90 __restore_rt sigaction.c:0:0
   #6 0x03c5c11f 
llvm::ilist_sentinel >::empty() const 
/media/project2/llvm-project/llvm/include/llvm/ADT/ilist_node.h:248:36
   #7 0x03c5c11f llvm::simple_ilist::empty() const 
/media/project2/llvm-project/llvm/include/llvm/ADT/simple_ilist.h:131:55
   #8 0x03c5c11f llvm::BasicBlock::empty() const 
/media/project2/llvm-project/llvm/include/llvm/IR/BasicBlock.h:307:66
   #9 0x03c5c11f llvm::BasicBlock::removePredecessor(llvm::BasicBlock*, 
bool) /media/project2/llvm-project/llvm/lib/IR/BasicBlock.cpp:328:7
  #10 0x04432005 markAliveBlocks(llvm::Function&, 
llvm::SmallPtrSetImpl&, llvm::DomTreeUpdater*) 
/media/project2/llvm-project/llvm/lib/Transforms/Utils/Local.cpp:2290:15
  #11 0x04432005 llvm::removeUnreachableBlocks(llvm::Function&, 
llvm::DomTreeUpdater*, llvm::MemorySSAUpdater*) 
/media/project2/llvm-project/llvm/lib/Transforms/Utils/Local.cpp:2402:18
  #12 0x042e72c7 simplifyFunctionCFGImpl(llvm::Function&, 
llvm::TargetTransformInfo const&, llvm::DominatorTree*, 
llvm::SimplifyCFGOptions const&) 
/media/project2/llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:0:22
  #13 0x042e72c7 simplifyFunctionCFG(llvm::Function&, 
llvm::TargetTransformInfo const&, llvm::DominatorTree*, 
llvm::SimplifyCFGOptions const&) 
/media/project2/llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:275:18
  #14 0x042e70d1 llvm::SimplifyCFGPass::run(llvm::Function&, 
llvm::AnalysisManager&) 
/media/project2/llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:321:7
  #15 0x02e605bd llvm::detail::PassModel >::run(llvm::Function&, 
llvm::AnalysisManager&) BPFTargetMachine.cpp:0:0
  #16 0x03d54659 llvm::SmallPtrSet::operator=(llvm::SmallPtrSet&&) 
/media/project2/llvm-project/llvm/include/llvm/ADT/SmallPtrSet.h:488:13
  #17 0x03d54659 
llvm::PreservedAnalyses::operator=(llvm::PreservedAnalyses&&) 
/media/project2/llvm-project/llvm/include/llvm/IR/PassManager.h:155:7
  #18 0x03d54659 llvm::PassManager >::run(llvm::Function&, 
llvm::AnalysisManager&) 
/media/project2/llvm-pro

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600
+  if (RHS.data() == getEmptyKey().data())
+return LHS.data() == getEmptyKey().data();
+  if (RHS.data() == getTombstoneKey().data())
+return LHS.data() == getTombstoneKey().data();
+  return LHS == RHS;
+}

lattner wrote:
> I'm pretty sure this method can just be "return LHS == RHS;"  The 
> tombstone/empty comparisons should work without special cases.
Doesn't operator== on ArrayRef compare the elements of the arrays? So wouldn't 
that dereference the invalid pointers used by tombstone/empty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Chris Lattner via Phabricator via cfe-commits]

lattner added inline comments.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600
+  if (RHS.data() == getEmptyKey().data())
+return LHS.data() == getEmptyKey().data();
+  if (RHS.data() == getTombstoneKey().data())
+return LHS.data() == getTombstoneKey().data();
+  return LHS == RHS;
+}

craig.topper wrote:
> lattner wrote:
> > I'm pretty sure this method can just be "return LHS == RHS;"  The 
> > tombstone/empty comparisons should work without special cases.
> Doesn't operator== on ArrayRef compare the elements of the arrays? So 
> wouldn't that dereference the invalid pointers used by tombstone/empty?
Yes, implemented in terms of std::equal.  However, both of these cases have 
zero elements, so the "pointer" is never dereferenced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Andi via Phabricator via cfe-commits]

Abpostelnicu added a comment.

I think there is a false positive with this @george.burgess.iv:
In 
[this)(https://searchfox.org/mozilla-central/source/mozglue/baseprofiler/core/platform-linux-android.cpp#222-227)
 we have the warning triggered: 
mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 'r' 
set but not used [-Werror,-Wunused-but-set-variable]

  SigHandlerCoordinator() {
PodZero(&mUContext);
int r = sem_init(&mMessage2, /* pshared */ 0, 0);
r |= sem_init(&mMessage3, /* pshared */ 0, 0);
r |= sem_init(&mMessage4, /* pshared */ 0, 0);
MOZ_ASSERT(r == 0);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

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

xbolva00 added a comment.

In D100581#2792854 , @Abpostelnicu 
wrote:

> I think there is a false positive with this @george.burgess.iv:
> In this 
> 
>  we have the warning triggered: 
> mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 
> 'r' set but not used [-Werror,-Wunused-but-set-variable]
>
>   SigHandlerCoordinator() {
> PodZero(&mUContext);
> int r = sem_init(&mMessage2, /* pshared */ 0, 0);
> r |= sem_init(&mMessage3, /* pshared */ 0, 0);
> r |= sem_init(&mMessage4, /* pshared */ 0, 0);
> MOZ_ASSERT(r == 0);
>   }

If MOZ_ASSERT is expanded to nothing, than the warning is correct. (void)r; 
trick should work here..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600
+  if (RHS.data() == getEmptyKey().data())
+return LHS.data() == getEmptyKey().data();
+  if (RHS.data() == getTombstoneKey().data())
+return LHS.data() == getTombstoneKey().data();
+  return LHS == RHS;
+}

lattner wrote:
> craig.topper wrote:
> > lattner wrote:
> > > I'm pretty sure this method can just be "return LHS == RHS;"  The 
> > > tombstone/empty comparisons should work without special cases.
> > Doesn't operator== on ArrayRef compare the elements of the arrays? So 
> > wouldn't that dereference the invalid pointers used by tombstone/empty?
> Yes, implemented in terms of std::equal.  However, both of these cases have 
> zero elements, so the "pointer" is never dereferenced.
As the length is zero, wouldn't the empty key, the tombstone key and an empty 
ArrayRef all be considered equal, as the data pointer is never compared?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon accepted this revision.
RKSimon added a comment.

Thanks for dealing with this @nikic

For this refactor, keeping the implementation as close to the existing one 
makes more sense; also personally I'd prefer the missing include fixups to be 
committed first as NFCs but its up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
 
-// TODO: Support SymbolCast. Support IntSymExpr when/if we actually
-// start producing them.
 

vsavchenko wrote:
> Do we actually produce them?
You can met SymbolCast here if `evalCast` produced it.
See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
```
if (!Loc::isLocType(CastTy))
  if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
  T->isFloatingType())
return makeNonLoc(SE, T, CastTy);
```
Also my patch will produce a lot of integral cast symbols D103096. You are 
welcome to examine it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
 
-// TODO: Support SymbolCast. Support IntSymExpr when/if we actually
-// start producing them.
 

ASDenysPetrov wrote:
> vsavchenko wrote:
> > Do we actually produce them?
> You can met SymbolCast here if `evalCast` produced it.
> See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
> ```
> if (!Loc::isLocType(CastTy))
>   if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
>   T->isFloatingType())
> return makeNonLoc(SE, T, CastTy);
> ```
> Also my patch will produce a lot of integral cast symbols D103096. You are 
> welcome to examine it.
In my comment I meant `IntSymExpr`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Manas Gupta via Phabricator via cfe-commits]

manas updated this revision to Diff 349225.
manas added a comment.

Fixed test cases expecting wrong assertions and added few more test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103440

Files:
  clang/test/Analysis/constant-folding.c


Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -251,3 +251,57 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testAdditionRules(unsigned int a, unsigned int b, int c, int d) {
+  if (a == 0) {
+clang_analyzer_eval((a + 0) == 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for unsigned operands
+  clang_analyzer_eval((a + b) < 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + b) <= UINT_MAX); // expected-warning{{TRUE}}
+
+  if (a == UINT_MAX && b == UINT_MAX) {
+clang_analyzer_eval((a + b) == UINT_MAX - 1); // expected-warning{{TRUE}}
+  }
+
+  // Checks for inclusive ranges for unsigned integers
+  if (a >= 0 && a <= 10 && b >= 0 && b <= 20) {
+clang_analyzer_eval((a + b) >= 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) > 30); // expected-warning{{FALSE}}
+  }
+
+  // Checks for negative signed integers
+  if (c < 0 && d < 0) {
+clang_analyzer_eval((c + d) < 0); // expected-warning{{UNKNOWN}}
+
+if (c == INT_MIN && d == INT_MIN) {
+  clang_analyzer_eval((c + d) == 0); // expected-warning{{TRUE}}
+}
+
+if (c >= -20 && d >= -40) {
+  clang_analyzer_eval((c + d) < -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval((c + d) >= -60); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for integers with different sign bits
+  if (c < 0 && d > 0) {
+if (c >= -20 && d <= 10) {
+  clang_analyzer_eval((c + d) > -20); // expected-warning{{TRUE}}
+  clang_analyzer_eval((c + d) < 10); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for overlapping signed integers ranges
+  if (c >= -20 && c <= 20 && d >= -10 && d <= 10) {
+clang_analyzer_eval((c + d) >= -30); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) <= 30); // expected-warning{{TRUE}}
+  }
+
+  // Checks for positive signed integers
+  if (c > 0 && d > 0) {
+clang_analyzer_eval((c + d) > 1); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) < -1); // expected-warning{{TRUE}}
+  }
+}


Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -251,3 +251,57 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testAdditionRules(unsigned int a, unsigned int b, int c, int d) {
+  if (a == 0) {
+clang_analyzer_eval((a + 0) == 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for unsigned operands
+  clang_analyzer_eval((a + b) < 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + b) <= UINT_MAX); // expected-warning{{TRUE}}
+
+  if (a == UINT_MAX && b == UINT_MAX) {
+clang_analyzer_eval((a + b) == UINT_MAX - 1); // expected-warning{{TRUE}}
+  }
+
+  // Checks for inclusive ranges for unsigned integers
+  if (a >= 0 && a <= 10 && b >= 0 && b <= 20) {
+clang_analyzer_eval((a + b) >= 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) > 30); // expected-warning{{FALSE}}
+  }
+
+  // Checks for negative signed integers
+  if (c < 0 && d < 0) {
+clang_analyzer_eval((c + d) < 0); // expected-warning{{UNKNOWN}}
+
+if (c == INT_MIN && d == INT_MIN) {
+  clang_analyzer_eval((c + d) == 0); // expected-warning{{TRUE}}
+}
+
+if (c >= -20 && d >= -40) {
+  clang_analyzer_eval((c + d) < -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval((c + d) >= -60); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for integers with different sign bits
+  if (c < 0 && d > 0) {
+if (c >= -20 && d <= 10) {
+  clang_analyzer_eval((c + d) > -20); // expected-warning{{TRUE}}
+  clang_analyzer_eval((c + d) < 10); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for overlapping signed integers ranges
+  if (c >= -20 && c <= 20 && d >= -10 && d <= 10) {
+clang_analyzer_eval((c + d) >= -30); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) <= 30); // expected-warning{{TRUE}}
+  }
+
+  // Checks for positive signed integers
+  if (c > 0 && d > 0) {
+clang_analyzer_eval((c + d) > 1); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) < -1); // expected-warning{{TRUE}}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103461: [clang][deps] NFC: Do not adjust the original action

[Jan Svoboda via Phabricator via cfe-commits]

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

Fix whitespace around fixme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103461

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -439,6 +439,9 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -509,7 +512,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -530,8 +533,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -439,6 +439,9 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -509,7 +512,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -530,8 +533,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
 
-// TODO: Support SymbolCast. Support IntSymExpr when/if we actually
-// start producing them.
 

vsavchenko wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > Do we actually produce them?
> > You can met SymbolCast here if `evalCast` produced it.
> > See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
> > ```
> > if (!Loc::isLocType(CastTy))
> >   if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
> >   T->isFloatingType())
> > return makeNonLoc(SE, T, CastTy);
> > ```
> > Also my patch will produce a lot of integral cast symbols D103096. You are 
> > welcome to examine it.
> In my comment I meant `IntSymExpr`
Oh, anyway :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D103461: [clang][deps] NFC: Do not adjust the original action

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2791760 , @dexonsmith 
wrote:

> Is there (or could there be) a mode where `clang-scan-deps` prints out the 
> command-lines it sends to the dependency scanning action (maybe instead of 
> actually scanning), so this could be tested?

The command-line adjustments are an implementation detail and there's a bunch 
of tests checking the make output format that exercise the code. So I'm not 
sure there's much value in explicitly testing the transformations. There's 
currently no way to print the adjusted command-line.

My reason for the FIXME is that we could get rid of bunch of Windows-specific 
logic by adjusting `CompilerInvocation` instead.




Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:442-443
   ResourceDirectoryCache ResourceDirCache;
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  //  instead of parsing and adjusting the raw command-line.
   AdjustingCompilations->appendArgumentsAdjuster(

dexonsmith wrote:
> Nit: blank line before the comment might make it easier to read; also the 
> second line seems to have an indent, is that intentional?
The indent is intentional, my IDE doesn't pick up unindented lines part of the 
FIXME. I've removed it so it's more in line with the rest of the codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103461

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14921
+  verifyFormat("unsigned int   *a;\n"
+   "int*b;\n"
"unsigned int Const *c;\n"

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > I seem to remember in the past there was a comment from @djasper in the 
> > past that he felt you shouldn't align the * like this
> > 
> > I think it depends on your viewpoint as to whether the * goes with the type 
> > or the variable, this could be a religious debate, I don't think you can 
> > just assume everyone agrees with this, the fact you are changing unit tests 
> > just leaves me with alarm bells going off...
> > 
> > I'm not comfortable with you changing unit tests. This implies that you 
> > think its a bug, but I think the fact the test exists (Beyonce rule), means 
> > someone may have wanted to assert that behaviour. 
> > 
> >  Should we have an option to control this?
> As far as I understand it `PAS_Right` says it should stick with the variable, 
> and thus align this way, if we align declarations.
> 
> I think this test was this way because of the `FIXME`.
I agree with @HazardyKnusperkeks. But I agree that it might be a contentious 
subject.



Comment at: clang/unittests/Format/FormatTest.cpp:15046-15048
 "  int const i   = 1;\n"
-"  int * j   = 2;\n"
+"  int  *j   = 2;\n"
 "  int   big = 1;\n"

HazardyKnusperkeks wrote:
> But maybe than this should be aligned as that?
Hmm, that's a subjective choice here indeed.
We might either 1) choose one or another (I'm slightly in favor of aligning 
variable names, as it's done currently), or  2) add a config option.
For 1), it would be wise to see large projects with a similar style and what 
they do (and whethere there's some consensus).
Anyone aware of such projects?



Comment at: clang/unittests/Format/FormatTest.cpp:15046-15048
 "  int const i   = 1;\n"
-"  int * j   = 2;\n"
+"  int  *j   = 2;\n"
 "  int   big = 1;\n"

curdeius wrote:
> HazardyKnusperkeks wrote:
> > But maybe than this should be aligned as that?
> Hmm, that's a subjective choice here indeed.
> We might either 1) choose one or another (I'm slightly in favor of aligning 
> variable names, as it's done currently), or  2) add a config option.
> For 1), it would be wise to see large projects with a similar style and what 
> they do (and whethere there's some consensus).
> Anyone aware of such projects?
That makes me think. How would be this formatted:
```
int **lotsOfpointers;
int   i;
```

@gergap, could you add such a test or point me to an existing one that tests 
the same thing, please?
That sort of test might fail if pointers/references weren't accounted for in 
the length of the type name itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103245

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
 
-// TODO: Support SymbolCast. Support IntSymExpr when/if we actually
-// start producing them.
 

ASDenysPetrov wrote:
> vsavchenko wrote:
> > ASDenysPetrov wrote:
> > > vsavchenko wrote:
> > > > Do we actually produce them?
> > > You can met SymbolCast here if `evalCast` produced it.
> > > See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
> > > ```
> > > if (!Loc::isLocType(CastTy))
> > >   if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
> > >   T->isFloatingType())
> > > return makeNonLoc(SE, T, CastTy);
> > > ```
> > > Also my patch will produce a lot of integral cast symbols D103096. You 
> > > are welcome to examine it.
> > In my comment I meant `IntSymExpr`
> Oh, anyway :-)
I think we should remove **ConcreteInt** and and as a consequence 
**IntSymExpr**, **SymIntExpr** in the nearest future. SymbolVal should be 
reworked to store a QualType, a RangeSet and an expression pointer itself. IMO 
SymSymExpr and SymbolVal is sufficient to make any calculations. It may help 
generalize a lot of them. **ConcreteInt** is just a special case for SymbolVal. 
More than that **PointerToMember** can be removed as well, because we would get 
an info being a pointer to member from QualType in this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think the C++ Core Guideline wording is... confusing. The rule title is 
`C.21: If you define or =delete any copy, move, or destructor function, define 
or =delete them all`, which I take literally. Using `= default` defines the 
function, so:

  struct S {
virtual ~S() = default; // This defines the dtor
  };

Because this defines the dtor, all the rest need to be defined as well per the 
rule. The enforcement on the rule says: `Enforcement (Simple) A class should 
have a declaration (even a =delete one) for either all or none of the 
copy/move/destructor functions.` and so it seems like the rule is requiring us 
to diagnose this case by default (pun retroactively intended), and so the 
option should remain as it is. In the code example you posted from the rule, I 
imagine that to be a load-bearing `// ... ` that defines all the rest of the 
members.

Perhaps the C++ Core Guideline authors can be enticed to update the rule to be 
more clear?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103511

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

[PATCH] D103516: [clang][deps] Customize PCM path via -build-dir argument

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Dependency scanning currently performs an implicit build. When testing that the 
command-lines produced for modular dependencies work when passed to Clang, they 
would overwrite the implicitly built PCM with an explicitly built PCM. This 
makes debugging harder than it should be.

This patch adds new flag to `clang-scan-deps` that allows developers to 
customize the PCM build directory, preventing file overwrites. Moreover, the 
explicit context hash is now part of the PCM path. This is useful in D102488 
, where the context hash can change due to 
command-line pruning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103516

Files:
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -163,6 +163,12 @@
 "'-fmodule-file=', '-o', '-fmodule-map-file='."),
 llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt BuildDir(
+"build-dir",
+llvm::cl::desc("With '-generate-modules-path-args', put the PCM files in"
+   "the provided directory instead of the module cache."),
+llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -357,7 +363,22 @@
 
 private:
   StringRef lookupPCMPath(ModuleID MID) {
-return Modules[IndexedModuleID{MID, 0}].ImplicitModulePCMPath;
+auto PCMPath = PCMPaths.insert({IndexedModuleID{MID, 0}, ""});
+if (PCMPath.second)
+  PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID));
+return PCMPath.first->second;
+  }
+
+  /// Construct a path where to put the explicitly built PCM.
+  std::string constructPCMPath(const ModuleDeps &MD) const {
+StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
+
+SmallString<256> ExplicitPCMPath(
+!BuildDir.empty()
+? BuildDir
+: MD.Invocation.getHeaderSearchOpts().ModuleCachePath);
+llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
+return std::string(ExplicitPCMPath);
   }
 
   const ModuleDeps &lookupModuleDeps(ModuleID MID) {
@@ -395,6 +416,8 @@
   std::mutex Lock;
   std::unordered_map
   Modules;
+  std::unordered_map
+  PCMPaths;
   std::vector Inputs;
 };
 
Index: clang/test/ClangScanDeps/modules-full.cpp
===
--- clang/test/ClangScanDeps/modules-full.cpp
+++ clang/test/ClangScanDeps/modules-full.cpp
@@ -20,6 +20,12 @@
 // RUN:   -generate-modules-path-args -mode preprocess-minimized-sources >> %t.result
 // RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK,CHECK-ABS %s
 //
+// RUN: echo %t.dir > %t.result
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format experimental-full \
+// RUN:   -generate-modules-path-args -build-dir %t.dir/build \
+// RUN:   -mode preprocess-minimized-sources >> %t.result
+// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK,CHECK-ABS-BUILD %s
+//
 // RUN: echo %t.dir > %t_clangcl.result
 // RUN: clang-scan-deps -compilation-database %t_clangcl.cdb -j 4 -format experimental-full \
 // RUN:   -mode preprocess-minimized-sources >> %t_clangcl.result
@@ -45,9 +51,11 @@
 // CHECK-NEXT: "-cc1"
 // CHECK-NO-ABS-NOT:   "-fmodule-map-file={{.*}}"
 // CHECK-ABS:  "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
+// CHECK-ABS-BUILD:"-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK:  "-emit-module"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS:  "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm"
+// CHECK-ABS-BUILD:"-fmodule-file=[[PREFIX]]/build/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK:  "-fmodule-name=header1"
 // CHECK:  "-fno-implicit-modules"
 // CHECK:],
@@ -105,8 +113,10 @@
 // CHECK-NEXT: "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm"
+// CHECK-ABS-BUILD-NEXT: "-fmodule-file=[[PREFIX]]/build/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NO-ABS-NOT:   "-fmodule-map-file={{.*}}"
 // CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
+// CHECK-ABS-BUILD-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.

[PATCH] D102488: [clang][deps] Prune unused header search paths

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:374
+  /// (potentially modified) context hash.
+  std::string constructPCMPath(const ModuleDeps &MD) const {
+const std::string &ImplicitPCMPath = MD.ImplicitModulePCMPath;

This change will be reverted in favor of D103516.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102488

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

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

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

steakhal added a comment.

In D103511#2793291 , @aaron.ballman 
wrote:

> I think the C++ Core Guideline wording is... confusing. The rule title is 
> `C.21: If you define or =delete any copy, move, or destructor function, 
> define or =delete them all`, which I take literally. Using `= default` 
> defines the function, so:

Okay, then I'm going to abandon this in a few days if nothing interesting shows 
up.
Thanks for the quick response!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103511

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

[PATCH] D103082: [AArch64][SVE] Improve codegen for dupq SVE ACLE intrinsics

[Bradley Smith via Phabricator via cfe-commits]

bsmith updated this revision to Diff 349241.
bsmith retitled this revision from "[AArch64][SVE] Optimize svbool dupq ACLE 
intrinsic to fixed predicate patterns" to "[AArch64][SVE] Improve codegen for 
dupq SVE ACLE intrinsics".
bsmith edited the summary of this revision.
bsmith added a comment.

- Rework approach to use llvm.experimental.vector.insert instead of introducing 
a new LLVM intrinsic
- Also apply changes to all non-svbool variants.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103082

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-cmpne.ll

Index: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-cmpne.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-cmpne.ll
@@ -0,0 +1,397 @@
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; DUPQ b8
+
+define  @dupq_b_0() #0 {
+; CHECK-LABEL: @dupq_b_0(
+; CHECK: ret  zeroinitializer
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv16i8.v16i8( undef,
+<16 x i8> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv16i8( %1,  %3,  %4)
+  ret  %5
+}
+
+define  @dupq_b_d() #0 {
+; CHECK-LABEL: @dupq_b_d(
+; CHECK: %1 = call  @llvm.aarch64.sve.ptrue.nxv2i1(i32 31)
+; CHECK-NEXT: %2 = call  @llvm.aarch64.sve.convert.to.svbool.nxv2i1( %1)
+; CHECK-NEXT: ret  %2
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv16i8.v16i8( undef,
+<16 x i8> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv16i8( %1,  %3,  %4)
+  ret  %5
+}
+
+define  @dupq_b_w() #0 {
+; CHECK-LABEL: @dupq_b_w(
+; CHECK: %1 = call  @llvm.aarch64.sve.ptrue.nxv4i1(i32 31)
+; CHECK-NEXT: %2 = call  @llvm.aarch64.sve.convert.to.svbool.nxv4i1( %1)
+; CHECK-NEXT: ret  %2
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv16i8.v16i8( undef,
+<16 x i8> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv16i8( %1,  %3,  %4)
+  ret  %5
+}
+
+define  @dupq_b_h() #0 {
+; CHECK-LABEL: @dupq_b_h(
+; CHECK: %1 = call  @llvm.aarch64.sve.ptrue.nxv8i1(i32 31)
+; CHECK-NEXT: %2 = call  @llvm.aarch64.sve.convert.to.svbool.nxv8i1( %1)
+; CHECK-NEXT: ret  %2
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv16i8.v16i8( undef,
+<16 x i8> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv16i8( %1,  %3,  %4)
+  ret  %5
+}
+
+define  @dupq_b_b() #0 {
+; CHECK-LABEL: @dupq_b_b(
+; CHECK: %1 = call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+; CHECK-NEXT: ret  %1
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv16i8.v16i8( undef,
+<16 x i8> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv16i8( %1,  %3,  %4)
+  ret  %5
+}
+
+; DUPQ b16
+
+define  @dupq_h_0() #0 {
+; CHECK-LABEL: @dupq_h_0(
+; CHECK: ret  zeroinitializer
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv8i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv8i16.v8i16( undef,
+<8 x i16> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv8i16( %2 , i64 0)
+  %4 = tail call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
+  %5 = tail call  @llvm.aarch64.sve.cmpne.wide.nxv8i16( %1,  %3,  %4)
+  ret  %5
+}
+
+define  @dupq_h_d() #0 {
+; CHECK-LABEL: @dupq_h_d(
+; CHECK: %1 = call  @llvm.aarch64.sve.ptrue.nxv2i1(i32 31)
+; CHECK-NEXT: %2 = call  @llvm.aarch64.sve.convert.to.svbool.nxv2i1( %1)
+; CHECK-NEXT: %3 = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %2)
+; CHECK-NEXT: ret  %3
+  %1 = tail call  @llvm.aarch64.sve.ptrue.nxv8i1(i32 31)
+  %2 = tail call  @llvm.experimental.vector.insert.nxv8i16.v8i16( undef,
+<8 x i16> , i64 0)
+  %3 = tail call  @llvm.aarch64.sve.dupq.lane.nxv8i16( %2 , i64 0)
+  %4 = tail call  @llvm.aarch

[PATCH] D103519: [clang][deps] Support object files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: mgorny.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a project uses PCH with explicit modules, the build will look like this:

1. scan PCH dependencies
2. explicitly build PCH
3. scan TU dependencies
4. explicitly build TU

Step 2 produces an object file for the PCH, which the dependency scanner needs 
to read in step 3.

This patch adds support for object files to the dependency scanner.

The `clang-scan-deps` invocation in the attached test would fail without this 
change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103519

Files:
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
  clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
  clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: cp %S/Inputs/modules-pch/* %t
+
+// Explicitly build the PCH:
+//
+// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch
+
+// Scan dependencies of the TU:
+//
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb_tu.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_tu.json -format experimental-full \
+// RUN:   -generate-modules-path-args -build-dir %t/build
Index: clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/tu.c
@@ -0,0 +1,3 @@
+// tu.c
+
+#include "mod_tu.h"
Index: clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
@@ -0,0 +1 @@
+// pch.h
Index: clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
@@ -0,0 +1,3 @@
+module ModTU {
+header "mod_tu.h"
+}
Index: clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu.h
@@ -0,0 +1 @@
+// mod_tu.h
Index: clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu.json
@@ -0,0 +1,7 @@
+[
+  {
+"directory": "DIR",
+"command": "clang -fsyntax-only DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -153,7 +154,13 @@
 DependencyScanningService &Service)
 : Format(Service.getFormat()) {
   DiagOpts = new DiagnosticOptions();
+
   PCHContainerOps = std::make_shared();
+  PCHContainerOps->registerReader(
+  std::make_unique());
+  PCHContainerOps->registerWriter(
+  std::make_unique());
+
   RealFS = llvm::vfs::createPhysicalFileSystem();
   if (Service.canSkipExcludedPPRanges())
 PPSkipMappings =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "llvm/Support/TargetSelect.h"
 
 using namespace clang;
 using namespace tooling;
@@ -16,4

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

[Sam McCall via Phabricator via cfe-commits]

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

Nice! Thanks for the reminder on this one, I lost track of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101741

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

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added a comment.

Nice!


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

https://reviews.llvm.org/D103195

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Have you tried running this over any large C++ code bases to see how often the 
diagnostics trigger and whether there are false positives? If not, you should 
probably do so -- one concern I have is with a private virtual destructor; I 
could imagine people using that for a class that is intended to be created but 
not destroyed (like a singleton).




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:99
+CheckFactories.registerCheck(
+"cppcoreguidelines-virtual-base-class-destructor");
   }

I think this may be a bit confusing of a name for the check -- this suggests to 
me it's about virtual base classes and their destructors, but the check is 
really more about defining a destructor properly in a class used as a base 
class with a vtable. However, this check follows the enforcement from the rule 
rather than the rule wording itself -- it doesn't care whether the class is 
ever used as a base class or not, it only cares whether the class contains a 
virtual function. How about: `cppcoreguidelines-virtual-class-destructor`? 
(Probably worth it to rename the check at the same time to keep the public 
check name and the internal check implementation names consistent.)



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:28
+  cxxRecordDecl(
+  anyOf(isStruct(), isClass()),
+  anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),

I believe you can remove this without changing the behavior -- unions can't 
have virtual member functions anyway.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:80
+  Loc = StructOrClass.getEndLoc();
+  DestructorString.append("public:");
+  AppendLineBreak = true;





Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:89-93
+  DestructorString.append("\n")
+  .append("virtual ~")
+  .append(StructOrClass.getName().str())
+  .append("() = default;")
+  .append(AppendLineBreak ? "\n" : "");

This should be using an `llvm::Twine` 
(https://llvm.org/docs/ProgrammersManual.html#the-twine-class).



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:117-121
+  std::string()
+  .append(Visibility)
+  .append(":\n")
+  .append(Visibility == "public" && !Destructor.isVirtual() ? "virtual 
"
+: "");

This function should also be using a Twine for much of this -- basically, Twine 
helps you build up a string from various components (some `std::string`, some 
`StringRef`, some `const char *`, etc) in an efficient manner that only does 
memory allocation when needing the full string contents.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:123-125
+  if (Visibility == "protected" && Destructor.isVirtualAsWritten()) {
+OriginalDestructor = eraseKeyword(OriginalDestructor, "virtual ");
+  }





Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:135-141
+  if (Destructor.isExplicitlyDefaulted()) {
+EndLocation =
+utils::lexer::findNextTerminator(Destructor.getEndLoc(), SM, LangOpts)
+.getLocWithOffset(1);
+  } else {
+EndLocation = Destructor.getEndLoc().getLocWithOffset(1);
+  }





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:6-8
+Finds base classes and structs whose destructor is neither public and virtual
+nor protected and non-virtual. A base class's destructor should be specified in
+one of these ways to prevent undefined behaviour.





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:24-50
+  struct Foo {  // NOK, protected destructor should not be virtual
+virtual void f();
+  protected:
+virtual ~Foo(){};
+  };
+
+  class Bar {// NOK, public destructor should be virtual

There are a bunch of trailing semicolons in the example code that can be 
removed.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp:6
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {

Thank you for the comment about the fix not being applied!


Repository:
  rG LLVM Github Monorepo

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

https://revie

[PATCH] D102175: [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:541
   Orig.constMethod();
+  UnnecessaryCopy.constMethod();
 }

If this line weren't here, then we'd delete line 537, in which case `Ref` 
itself becomes unused and so line 536 should be deleted as well. Do you handle 
this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102175

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

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102180

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Returning to the discussion raised in D102696 
, I'd like to share my vision.
I think we can use much easier approach to use valid constraints at any point 
of time.
The main idea is lazy-reasoning of the ranges.
This approach:

- **doesn't** need to use **canonicalization**;
- **doesn't** need to **update all** the constraints on **each** 
`setConstraint` call (using brute-force or any other structure traversals);
- **add** an additional bindings for **every** simple operand on **each** 
//assignment// or //initialization//.
- **remove** an invalid bindings for **every** simple operand on **each** 
//assignment// or //initialization//.
- **recursively** get the **range** when and only when needed (lazy-reasoning).

Example:

  a = b + 1;
  c = a - 5;
  if (b != 42) return 0;
  if(c == 38) return 1;
  return 0;

`a = b + 1;`

1. LHS `a` lookup. LHS not found.
2. Add binding `$a = $b + 1`
3. Traverse through RHS `b + 1`.
4. `b` lookup. `b` not found.
5. Built a new expression `a - 1` for `b`.
6. Add binding for `b`: `$b = $a - 1`
7. `1` is a constant. Skip.

`c = a - 1;`

1. LHS `c` lookup. LHS not found.
2. Add binding `$c = $a - 1`
3. Traverse through RHS `a - 1`.
4. `a` lookup. `a` found.
5. If LHS not found and `a` found, then skip.
6. `1` is a constant. Skip.

`if (b != 42) return;`

1. Get the range of `$b`. Direct range not found. Add `$b` to the list of 
visited symbols.
2. Substitute bindings. From `$b != 42` to `$a - 1 == 42`.
3. Traverse through `$a - 1`.
4. Get the range of `$a`. Direct range not found. Add `$a` to the list of 
visited symbols.
5. `1` is a constant. Skip.
6. Substitute bindings. From `$a - 1 == 42` to `$b + 1 - 1 == 42`.
7. Traverse through `$b + 1 - 1`.
8. `$b` is in the list of already visited symbols. No way to find a range.
9. Create a range from the type [MIN, MAX].
10. Perform comparison and produce a constraint for `b` [42, 42].

`if(c == 38) return;`

1. Get the range of `$c`. Direct range not found. Add `$c` to the list of 
visited symbols.
2. Substitute bindings. From `$c == 38` to `$a - 1 == 38`.
3. Traverse through `$a - 1`.
4. Get the range of `$a`. Direct range not found. Add `$a` to the list of 
visited symbols.
5. `1` is a constant. Skip.
6. Substitute bindings. From `$a - 1 == 38` to `$b + 1 - 5 == 38`.
7. Traverse through `$b + 1 - 5`.
8. Get the range of `$b`. Direct range **found** [42, 42].
9. `1` is a constant. Skip.
10. `5` is a constant. Skip.
11. Calculate a new range for `$c` [38, 38].
12. Condition is //true//.

`return 1`

P.S. I've just started thinking of the way to solve this problem. For now it's 
just a sketch of where I'm going.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D101554: [clangd] Add support for the `defaultLibrary` semantic token modifier

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 349259.
dgoldman marked an inline comment as done.
dgoldman added a comment.

Add `isDefaultLibrary(const Type *)` and support auto/decl types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101554

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -69,13 +69,16 @@
 std::vector>
 AdditionalFiles = {},
-uint32_t ModifierMask = -1) {
+uint32_t ModifierMask = -1,
+std::vector AdditionalArgs = {}) {
   Annotations Test(Code);
   TestTU TU;
   TU.Code = std::string(Test.code());
 
   TU.ExtraArgs.push_back("-std=c++20");
   TU.ExtraArgs.push_back("-xobjective-c++");
+  TU.ExtraArgs.insert(std::end(TU.ExtraArgs), std::begin(AdditionalArgs),
+  std::end(AdditionalArgs));
 
   for (auto File : AdditionalFiles)
 TU.AdditionalFiles.insert({File.first, std::string(File.second)});
@@ -102,9 +105,9 @@
   struct {
   } $Variable_decl[[S]];
   void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
 auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
@@ -320,8 +323,8 @@
   $Class_deduced[[decltype]](auto) $Variable_decl[[AF2]] = $Class[[Foo]]();
   $Class_deduced[[auto]] *$Variable_decl[[AFP]] = &$Variable[[AF]];
   $Enum_deduced[[auto]] &$Variable_decl[[AER]] = $Variable[[AE]];
-  $Primitive_deduced[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
-  $Primitive_deduced[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
+  $Primitive_deduced_defaultLibrary[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
+  $Primitive_deduced_defaultLibrary[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
   auto $Variable_decl[[Fun]] = []()->void{};
 )cpp",
   R"cpp(
@@ -746,6 +749,24 @@
 #define DEFINE_Y DEFINE(Y)
   )cpp"}},
  ~ScopeModifierMask);
+
+  checkHighlightings(R"cpp(
+#include "SYSObject.h"
+@interface $Class_defaultLibrary[[SYSObject]] ($Namespace_decl[[UserCategory]])
+@property(nonatomic, readonly) int $Field_decl_readonly[[user_property]];
+@end
+int $Function_decl[[somethingUsingSystemSymbols]]() {
+  $Class_defaultLibrary[[SYSObject]] *$LocalVariable_decl[[obj]] = [$Class_defaultLibrary[[SYSObject]] $StaticMethod_static_defaultLibrary[[new]]];
+  return $LocalVariable[[obj]].$Field_defaultLibrary[[value]] + $LocalVariable[[obj]].$Field_readonly[[user_property]];
+}
+  )cpp",
+ {{"SystemSDK/SYSObject.h", R"cpp(
+@interface SYSObject
+@property(nonatomic, assign) int value;
++ (instancetype)new;
+@end
+  )cpp"}},
+ ~ScopeModifierMask, {"-isystemSystemSDK/"});
 }
 
 TEST(SemanticHighlighting, ScopeModifiers) {
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025
+# CHECK-NEXT:  2049
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025
+# CHECK-NEXT:  2049
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025,
+# CHECK-NEXT:  2049,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-

[PATCH] D103524: [clang][deps] Handle precompiled headers' AST files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `PreprocessOnlyAction` doesn't support loading the AST file of a 
precompiled header. This is problematic for dependency scanning, since the 
`#include` manufactured for the PCH is treated as textual. This means the PCH 
contents get scanned with each TU, which is redundant. Moreover, dependencies 
of the PCH end up being considered dependency of the TU.

To handle AST file of PCH properly, this patch creates new `FrontendAction` 
that behaves the same way `PreprocessorOnlyAction` does, but treats the 
manufactured PCH `#include` as a normal compilation would (by not claiming it 
only uses a preprocessor and creating the default AST consumer).

The AST file is now reported as a file dependency of the TU.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103524

Files:
  clang/include/clang/Frontend/FrontendActions.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -4,10 +4,56 @@
 // Explicitly build the PCH:
 //
 // RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch
+// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch -fno-implicit-modules -fno-implicit-module-maps
 
 // Scan dependencies of the TU:
 //
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb_tu.json
+// RUN: echo -%t > %t/result_tu.json
 // RUN: clang-scan-deps -compilation-database %t/cdb_tu.json -format experimental-full \
-// RUN:   -generate-modules-path-args -build-dir %t/build
+// RUN:   -generate-modules-path-args -build-dir %t/build -mode preprocess >> %t/result_tu.json
+// RUN: cat %t/result_tu.json | FileCheck %s -check-prefix=CHECK-TU
+//
+// CHECK-TU:  -[[PREFIX:.*]]
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "modules": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "clang-module-deps": [],
+// CHECK-TU-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-TU-NEXT:   "command-line": [
+// CHECK-TU-NEXT: "-cc1",
+// CHECK-TU:  "-emit-module",
+// CHECK-TU:  "-fmodule-name=ModTU",
+// CHECK-TU:  "-fno-implicit-modules",
+// CHECK-TU:],
+// CHECK-TU-NEXT:   "context-hash": "[[HASH_MOD_TU:.*]]",
+// CHECK-TU-NEXT:   "file-deps": [
+// CHECK-TU-NEXT: "[[PREFIX]]/mod_tu.h",
+// CHECK-TU-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "name": "ModTU"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "translation-units": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "clang-context-hash": "[[HASH_TU:.*]]",
+// CHECK-TU-NEXT:   "clang-module-deps": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "context-hash": "[[HASH_MOD_TU]]",
+// CHECK-TU-NEXT:   "module-name": "ModTU"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "command-line": [
+// CHECK-TU-NEXT: "-fno-implicit-modules",
+// CHECK-TU-NEXT: "-fno-implicit-module-maps",
+// CHECK-TU-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm",
+// CHECK-TU-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "file-deps": [
+// CHECK-TU-NEXT: "[[PREFIX]]/tu.c",
+// CHECK-TU-NEXT: "[[PREFIX]]/pch.h.gch"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "input-file": "[[PREFIX]]/tu.c"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ]
+// CHECK-TU-NEXT: }
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -156,6 +156,9 @@
   MDC.MainFile = std::string(
   Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
 
+  if (!Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
+MDC.FileDeps.push_back(Instance.getPreprocessorOpts().ImplicitPCHInclude);
+
   for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
 
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong marked 3 inline comments as done.
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1559
+  // absolute minimum.
+  LLVM_NODISCARD ProgramStateRef simplifyEquivalenceClass(
+  ProgramStateRef State, EquivalenceClass Class, SymbolSet ClassMembers) {

vsavchenko wrote:
> Maybe it should be a `simplify` method of the class itself?
Yeah, makes sense.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1568-1573
+EquivalenceClass ClassOfSimplifiedSym =
+EquivalenceClass::find(State, SimplifiedMemberSym);
+// We are about to add the newly simplified symbol to the existing
+// equivalence class, but they are known to be non-equal. This is a
+// contradiction.
+if (DisequalClasses.contains(ClassOfSimplifiedSym))

vsavchenko wrote:
> I think we can add a method `isDisequalTo` or just use `areEqual` in a this 
> way:
> are equal?
>   [Yes] -> nothing to do here
>   [No]  -> return nullptr
>   [Don't know] -> merge
Good point, I've added a new overload to the static `areEqual` and added a 
method `isEqualTo` that uses `areEqual`.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1566-1569
+ConstraintRangeTy Constraints = State->get();
+for (std::pair ClassConstraint : Constraints) {
+  EquivalenceClass Class = ClassConstraint.first;
+  SymbolSet ClassMembers = Class.getClassMembers(State);

vsavchenko wrote:
> martong wrote:
> > vsavchenko wrote:
> > > You don't actually use constraints here, so (let me write it in python) 
> > > instead of:
> > > ```
> > > [update(classMap[class]) for class, constraint in constraints.items()]
> > > ```
> > > you can use
> > > ```
> > > [update(members) for class, members in classMap.items()]
> > > ```
> > Actually, trivial equivalence classes (those that have only one symbol 
> > member) are not stored in the State. Thus, we must skim through the 
> > constraints as well in order to be able to simplify symbols in the 
> > constraints.
> > In short, we have to iterate both collections.
> > 
> Ah, I see.  Then I would say that your previous solution is more readable (if 
> we keep `simplify`, of course).
My previous solution might be more readable, though, that's not working.
Actually, I think I failed to explain properly why do we have to iterate both 
collections.

* We have to iterate the ConstraintMap because trivial constraints are not 
stored in the State but we want to simplify symbols in the constraints. So, if 
we were to iterate over only the ClassMap then the simplest test-case would 
fail:
```
int test_rhs_further_constrained(int x, int y) {
  if (x + y != 0)
return 0;
  if (y != 0)
return 0;
  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}  
  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} FAIL
  return 0;
}
```

* We have to iterate the ClassMap in order to update all equivalence classes 
that we store in the State. Consider the example you brought up before:
```
void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
  if (a + b != c)
return;
  if (a != d)
return;
  if (b != 0)
return;
  // Keep the symbols and the constraints! alive.
  (void)(a * b * c * d);
  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
  return;
}
```
Before we start to simulate `b==0`,  we have only these equivalence classes in 
the State: E1{`a+b`, `c`} and E2{`a`, `d`}. And we have these constraints: 
SymExpr(`a+b==c`) -> out-of [0, 0], SymExpr(`a==d`) -> out-of [0, 0].
Now, when we evaluate `b==0`in setConstraint when iterating the ConstraintMap 
then SymExpr(`a+b==c`) becomes SymExpr(`a==c`). But the equality classes are 
not updated. And we can update them if we scan through the ClassMap. 

Another alternative solution could be to re-trigger the `track` mechanism when 
we iterate over the ConstraintMap, but `track` seemed to be an exclusive 
interface towards the higher abstraction Range**d**ConstraintManager. On the 
other hand, reusing the `track` mechanism could result better performance than 
doing another iteration on the ClassMap. Do you think it would be a better 
approach? And how could we reuse the `track` mechanism without getting confused 
with the `Adjustment` stuff?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 349261.
martong marked 3 inline comments as done.
martong added a comment.

- Add isEqualTo and simplify members to EquivalenceClass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/find-binop-constraints.cpp

Index: clang/test/Analysis/find-binop-constraints.cpp
===
--- /dev/null
+++ clang/test/Analysis/find-binop-constraints.cpp
@@ -0,0 +1,145 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_legacy_behavior(int x, int y) {
+  if (y != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return y / (x + y);  // expected-warning{{Division by zero}}
+}
+
+int test_rhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (x != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_and_rhs_further_constrained(int x, int y) {
+  if (x % y != 1)
+return 0;
+  if (x != 1)
+return 0;
+  if (y != 2)
+return 0;
+  clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_commutativity(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(y + x == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_binop_when_height_is_2_r(int a, int x, int y, int z) {
+  switch (a) {
+  case 1: {
+if (x + y + z != 0)
+  return 0;
+if (z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 2: {
+if (x + y + z != 0)
+  return 0;
+if (y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 3: {
+if (x + y + z != 0)
+  return 0;
+if (x != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 4: {
+if (x + y + z != 0)
+  return 0;
+if (x + y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 5: {
+if (z != 0)
+  return 0;
+if (x + y + z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+if (y != 0)
+  return 0;
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+break;
+  }
+
+  }
+  return 0;
+}
+
+void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a != d)
+return;
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
+  return;
+}
+
+void test_contradiction(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a == c)
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // Bring in the contradiction.
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_warnIfReached(); // no-warning, i.e. UNREACHABLE
+  return;
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/Small

[PATCH] D101554: [clangd] Add support for the `defaultLibrary` semantic token modifier

[David Goldman via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:436
+  if (HB.inDefaultLibrary(D->getLocation()))
+return HighlightingModifier::DefaultLibrary;
   const DeclContext *DC = D->getDeclContext();

sammccall wrote:
> dgoldman wrote:
> > sammccall wrote:
> > > dgoldman wrote:
> > > > sammccall wrote:
> > > > > kadircet wrote:
> > > > > > I don't use semantic highlighting enough to judge whether it is 
> > > > > > more useful to say `DefaultLibrary` than `{Class,Function}Scope`. 
> > > > > > (i.e. having the check here vs at the end of the function as a 
> > > > > > fallback). Can you provide some rationale about the decision 
> > > > > > (probably as comments in code)?
> > > > > > 
> > > > > > By looking at the testcase, I suppose you are trying to indicate 
> > > > > > "overriden"/"extended" attribute of a symbol, which makes sense but 
> > > > > > would be nice to know if there's more. I am just worried that this 
> > > > > > is very obj-c specific and won't really generalize to c/c++. As 
> > > > > > most of the system headers only provide platform specific structs 
> > > > > > (e.g. posix) and they are almost never extended. So it might be 
> > > > > > confusing for user to see different colors on some memberexprs.
> > > > > I think default-library isn't a scope modifier at all, it should be 
> > > > > independent.
> > > > > 
> > > > > e.g. std::string is defaultlLibrary | globalScope, while 
> > > > > std::string::push_back() is defaultLibrary | classScope.
> > > > I can break it up if you'd like - just let me know what you had in 
> > > > mind. I can change the scopeModifier function's name and then have it 
> > > > return a list of modifiers or I can just add a new function which all 
> > > > existing callers of scopeModifier will also need to call.
> > > > 
> > > > And what we're really trying to do here is color the system/SDK symbols 
> > > > differently - this is definitely useful for iOS dev since Apple has a 
> > > > very large SDK, but if it's not useful for C++ maybe we could just make 
> > > > this configurable?
> > > > just let me know what you had in mind
> > > 
> > > I think this should follow `isStatic()` etc: just a boolean function that 
> > > gets called in the minimum number of places needed.
> > > 
> > > scopeModifier is slightly different pattern because there are a bunch of 
> > > mutually-exclusive modifiers. defaultLibrary is (mostly) independent of 
> > > other modifiers.
> > > 
> > > > if it's not useful for C++ maybe we could just make this configurable
> > > 
> > > No need really - sending more modifiers is cheap (one string at startup 
> > > and one bit in an integer we're sending anyway).
> > > It's up to clients to style the ones they care about, and at least in 
> > > VSCode this can be customized per-lang I think. Important thing is that 
> > > they don't interact with other modifiers.
> > Swapped over, currently don't check on deduced types but we could add our 
> > isDefaultLibrary check there as well
> I think it'd be useful to do this for consistency, just VisitDecltypeTypeLoc 
> and VisitDeclaratorDecl need to be changed I think.
> 
> We'd want a version of isDefaultLibrary that works on Type. We're only going 
> to see canonical types so I think we need to:
>  - unwrap pointers (getPointeeOrArrayElementType)
>  - if BuiltinType -> return true
>  - for types: find the decl and dispatch to isDefaultLibrary(Decl)
> 
> to find the decl it'd be nice to use targetDecl() but also sufficient to 
> handle TagType + ObjCInterfaceType for now I think.
Done, I think this is fine for a v1 - we don't see much auto/decltype use in 
objc, can improve it later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101554

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

[clang] 61c65d8 - Fix comments in test cuda-kernel-call.cu

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-06-02T10:21:26-04:00
New Revision: 61c65d8e4a29610bd1ab498eba8f892ffc0af023

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

LOG: Fix comments in test cuda-kernel-call.cu

Added: 


Modified: 
clang/test/Parser/cuda-kernel-call.cu

Removed: 




diff  --git a/clang/test/Parser/cuda-kernel-call.cu 
b/clang/test/Parser/cuda-kernel-call.cu
index ef708e3a4b96..f0ff7680143f 100644
--- a/clang/test/Parser/cuda-kernel-call.cu
+++ b/clang/test/Parser/cuda-kernel-call.cu
@@ -10,8 +10,6 @@ void foo(void) {
 
   foo<<<>>>();  // expected-error {{expected expression}}
 
-  // The following two are parse errors because -std=c++11 is not enabled.
-
   S>> s;
   S>> s1;
   (void)(&f>>==0);



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

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:907
+  /// \param C   Context of the AST.
+  /// \param Loc Location of the 'full' identifier.
+  static OMPFullClause *Create(const ASTContext &C, SourceLocation StartLoc,

Fix params descriptions



Comment at: clang/include/clang/AST/StmtOpenMP.h:463-478
+  static const SpecificClause *getSingleClause(ArrayRef Clauses) {
+auto ClausesOfKind = getClausesOfKind(Clauses);
 
-if (Clauses.begin() != Clauses.end()) {
-  assert(std::next(Clauses.begin()) == Clauses.end() &&
+if (ClausesOfKind.begin() != ClausesOfKind.end()) {
+  assert(std::next(ClausesOfKind.begin()) == ClausesOfKind.end() &&
  "There are at least 2 clauses of the specified kind");
+  return *ClausesOfKind.begin();

Better to commit it it separately as NFC.



Comment at: clang/lib/AST/StmtOpenMP.cpp:382
+   Stmt *PreInits) {
+  OMPUnrollDirective *Dir = createDirective(
+  C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc);

`auto *`



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2594-2598
+  if (S.hasClausesOfKind())
+LoopStack.setUnrollState(LoopAttributes::Full);
+
+  auto *PartialClause = S.getSingleClause();
+  if (PartialClause) {

Can we have `full` and `partial` at the same time? If no, emit partial in 
`else` substatement for `full` clause.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:8984-8985
+  DependentPreInits = Dir->getPreInits();
+} else
+  llvm_unreachable("Unexpected loop transformation");
 if (!DependentPreInits)

Enclose in braces



Comment at: clang/lib/Sema/SemaOpenMP.cpp:10139-10142
+/// Find and diagnose mutually exclusive clause kinds.
+static bool checkMutuallyExclusiveClauses(
+Sema &S, ArrayRef Clauses,
+ArrayRef MutuallyExclusiveClauses) {

Would be good if this change is committed separately as NFC.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12604
+  });
+  assert(OriginalInits.back().empty());
+  OriginalInits.pop_back();

assert message?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12821
+/// Determine whether an expression is constant without emitting diagnostic.
+static bool isConstantExpression(Sema &SemaRef, Expr *E) {
+  struct ConstTripcountDiagnoser : public Sema::VerifyICEDiagnoser {

Can you use `VerifyPositiveIntegerConstantInClause` function instead?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12955-12956
+  SourceLocation FactorLoc;
+  if (auto *FactorConst =
+  cast_or_null(PartialClause->getFactor())) {
+Factor = FactorConst->getResultAsAPSInt().getZExtValue();

Better to use `PartialClause->getFactor()->isIntegerConstantExpr(Ctx)` or 
something similar.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12964
+  assert(Factor > 0 && "Expected positive unroll factor");
+  auto makeFactorExpr = [this, Factor, IVTy, FactorLoc]() {
+return IntegerLiteral::Create(

Why do you need it? Just use original `PartialClause->getFactor()`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12985
+  buildVarDecl(*this, {}, IVTy, OuterIVName, nullptr, OrigVar);
+  auto makeOuterRef = [this, OuterIVDecl, IVTy, OrigVarLoc]() {
+return buildDeclRefExpr(*this, OuterIVDecl, IVTy, OrigVarLoc);

`MakeOuterRef`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:12993
+  InnerIVDecl->setDeclName(&PP.getIdentifierTable().get(InnerIVName));
+  auto makeInnerRef = [this, InnerIVDecl, IVTy, OrigVarLoc]() {
+return buildDeclRefExpr(*this, InnerIVDecl, IVTy, OrigVarLoc);

`MakeInnerRef`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:13000
+  CaptureVars CopyTransformer(*this);
+  auto makeNumIterations = [&CopyTransformer, &LoopHelper]() -> Expr * {
+return AssertSuccess(

`MakeNumIterations`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99459

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

[clang-tools-extra] 2f951ca - [clangd] Add support for the `defaultLibrary` semantic token modifier

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2021-06-02T10:24:29-04:00
New Revision: 2f951ca98b7a12fed7ac4ebf790a0fbabc02d80b

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

LOG: [clangd] Add support for the `defaultLibrary` semantic token modifier

This allows us to differentiate symbols from the system (e.g. system
includes or sysroot) differently than symbols defined in the user's
project, which can be used by editors to display them differently.

This is currently based on `FileCharacteristic`, but we can
consider alternatives such as `Sysroot` and file paths in the future.

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 12ccdff82d028..bb192596f8c52 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -246,6 +246,29 @@ bool isDependent(const Decl *D) {
   return false;
 }
 
+/// Returns true if `Decl` is considered to be from a default/system library.
+/// This currently checks the systemness of the file by include type, although
+/// 
diff erent heuristics may be used in the future (e.g. sysroot paths).
+bool isDefaultLibrary(const Decl *D) {
+  SourceLocation Loc = D->getLocation();
+  if (!Loc.isValid())
+return false;
+  return D->getASTContext().getSourceManager().isInSystemHeader(Loc);
+}
+
+bool isDefaultLibrary(const Type *T) {
+  if (!T)
+return false;
+  const Type *Underlying = T->getPointeeOrArrayElementType();
+  if (Underlying->isBuiltinType())
+return true;
+  if (auto *TD = dyn_cast(Underlying))
+return isDefaultLibrary(TD->getDecl());
+  if (auto *TD = Underlying->getAsTagDecl())
+return isDefaultLibrary(TD);
+  return false;
+}
+
 // For a macro usage `DUMP(foo)`, we want:
 //  - DUMP --> "macro"
 //  - foo --> "variable".
@@ -473,6 +496,8 @@ class CollectExtraHighlightings
   .addModifier(HighlightingModifier::Deduced);
   if (auto Mod = scopeModifier(L.getTypePtr()))
 Tok.addModifier(*Mod);
+  if (isDefaultLibrary(L.getTypePtr()))
+Tok.addModifier(HighlightingModifier::DefaultLibrary);
 }
 return true;
   }
@@ -485,8 +510,11 @@ class CollectExtraHighlightings
  H.getResolver())) {
   auto &Tok = H.addToken(D->getTypeSpecStartLoc(), *K)
   .addModifier(HighlightingModifier::Deduced);
-  if (auto Mod = scopeModifier(AT->getDeducedType().getTypePtrOrNull()))
+  const Type *Deduced = AT->getDeducedType().getTypePtrOrNull();
+  if (auto Mod = scopeModifier(Deduced))
 Tok.addModifier(*Mod);
+  if (isDefaultLibrary(Deduced))
+Tok.addModifier(HighlightingModifier::DefaultLibrary);
 }
 return true;
   }
@@ -494,7 +522,7 @@ class CollectExtraHighlightings
   // We handle objective-C selectors specially, because one reference can
   // cover several non-contiguous tokens.
   void highlightObjCSelector(const ArrayRef &Locs, bool Decl,
- bool Class) {
+ bool Class, bool DefaultLibrary) {
 HighlightingKind Kind =
 Class ? HighlightingKind::StaticMethod : HighlightingKind::Method;
 for (SourceLocation Part : Locs) {
@@ -504,20 +532,27 @@ class CollectExtraHighlightings
 Tok.addModifier(HighlightingModifier::Declaration);
   if (Class)
 Tok.addModifier(HighlightingModifier::Static);
+  if (DefaultLibrary)
+Tok.addModifier(HighlightingModifier::DefaultLibrary);
 }
   }
 
   bool VisitObjCMethodDecl(ObjCMethodDecl *OMD) {
 llvm::SmallVector Locs;
 OMD->getSelectorLocs(Locs);
-highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod());
+highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod(),
+  isDefaultLibrary(OMD));
 return true;
   }
 
   bool VisitObjCMessageExpr(ObjCMessageExpr *OME) {
 llvm::SmallVector Locs;
 OME->getSelectorLocs(Locs);
-highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage());
+bool DefaultLibrary = false;
+if (ObjCMethodDecl *OMD = OME->getMethodDecl())
+  DefaultLibrary = isDefaultLibrary(OMD);
+highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage(),
+  DefaultLibrary);
 return true;
   }
 
@@ -643,6 +678,8 @@ std::vector 
getSemanticHighlightings(Pa

[PATCH] D101554: [clangd] Add support for the `defaultLibrary` semantic token modifier

[David Goldman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dgoldman marked an inline comment as done.
Closed by commit rG2f951ca98b7a: [clangd] Add support for the `defaultLibrary` 
semantic token modifier (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101554

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -69,13 +69,16 @@
 std::vector>
 AdditionalFiles = {},
-uint32_t ModifierMask = -1) {
+uint32_t ModifierMask = -1,
+std::vector AdditionalArgs = {}) {
   Annotations Test(Code);
   TestTU TU;
   TU.Code = std::string(Test.code());
 
   TU.ExtraArgs.push_back("-std=c++20");
   TU.ExtraArgs.push_back("-xobjective-c++");
+  TU.ExtraArgs.insert(std::end(TU.ExtraArgs), std::begin(AdditionalArgs),
+  std::end(AdditionalArgs));
 
   for (auto File : AdditionalFiles)
 TU.AdditionalFiles.insert({File.first, std::string(File.second)});
@@ -102,9 +105,9 @@
   struct {
   } $Variable_decl[[S]];
   void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
 auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
@@ -320,8 +323,8 @@
   $Class_deduced[[decltype]](auto) $Variable_decl[[AF2]] = $Class[[Foo]]();
   $Class_deduced[[auto]] *$Variable_decl[[AFP]] = &$Variable[[AF]];
   $Enum_deduced[[auto]] &$Variable_decl[[AER]] = $Variable[[AE]];
-  $Primitive_deduced[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
-  $Primitive_deduced[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
+  $Primitive_deduced_defaultLibrary[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
+  $Primitive_deduced_defaultLibrary[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
   auto $Variable_decl[[Fun]] = []()->void{};
 )cpp",
   R"cpp(
@@ -746,6 +749,24 @@
 #define DEFINE_Y DEFINE(Y)
   )cpp"}},
  ~ScopeModifierMask);
+
+  checkHighlightings(R"cpp(
+#include "SYSObject.h"
+@interface $Class_defaultLibrary[[SYSObject]] ($Namespace_decl[[UserCategory]])
+@property(nonatomic, readonly) int $Field_decl_readonly[[user_property]];
+@end
+int $Function_decl[[somethingUsingSystemSymbols]]() {
+  $Class_defaultLibrary[[SYSObject]] *$LocalVariable_decl[[obj]] = [$Class_defaultLibrary[[SYSObject]] $StaticMethod_static_defaultLibrary[[new]]];
+  return $LocalVariable[[obj]].$Field_defaultLibrary[[value]] + $LocalVariable[[obj]].$Field_readonly[[user_property]];
+}
+  )cpp",
+ {{"SystemSDK/SYSObject.h", R"cpp(
+@interface SYSObject
+@property(nonatomic, assign) int value;
++ (instancetype)new;
+@end
+  )cpp"}},
+ ~ScopeModifierMask, {"-isystemSystemSDK/"});
 }
 
 TEST(SemanticHighlighting, ScopeModifiers) {
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025
+# CHECK-NEXT:  2049
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025
+# CHECK-NEXT:  2049
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  1025,
+# CHECK-

[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:97
+  // Search the whole function body for decl statements of the initialization
+  // variable not just the current block statement.
   auto Matches =

Maybe a bit clearer:
// Search the whole function body, not just the current block statement, for 
decl statements of the initialization variable.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:98-101
   auto Matches =
   match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar
 .bind("declStmt")),
+Body, Context);

Consider inspecting the `DeclContext`s instead, which should be much more 
efficient than searching the entire block.  Pass the `FunctionDecl` as an 
argument instead of `Body`, since it is a `DeclContext`.  e.g. `const 
DeclContext &Fun`

Then, either
1. Call `Fun.containsDecl(InitializingVar)`, or
2. Search through the contexts yourself; something like:
```
DeclContext* DC = InitializingVar->getDeclContext(); 
while (DC != nullptr && DC != &Fun)
  DC = DC->getLexicalParent();
if (DC == nullptr)
  // The reference or pointer is not initialized anywhere witin the function. We
  // assume its pointee is not modified then.
  return true;
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:530
+
+  auto Labmda = []() {
+ExpensiveToCopyType Orig;

typo: Lambda


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103021

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

[clang] 369c648 - [clang] Implement the using_if_exists attribute

[Louis Dionne via cfe-commits]

Author: Erik Pilkington
Date: 2021-06-02T10:30:24-04:00
New Revision: 369c64839946d89cf5697550b6feeea031b2f270

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

LOG: [clang] Implement the using_if_exists attribute

This attribute applies to a using declaration, and permits importing a
declaration without knowing if that declaration exists. This is useful
for libc++ C wrapper headers that re-export declarations in std::, in
cases where the base C library doesn't provide all declarations.

This attribute was proposed in 
http://lists.llvm.org/pipermail/cfe-dev/2020-June/066038.html.

rdar://69313357

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

Added: 
clang/test/Parser/using-if-exists-attr.cpp
clang/test/SemaCXX/using-if-exists.cpp

Modified: 
clang/include/clang/AST/DeclCXX.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DeclNodes.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/AST/DeclBase.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTCommon.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/SemaCXX/attr-deprecated.cpp
clang/tools/libclang/CIndex.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c9efc4b454968..5c7cdd67e3d32 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -3776,6 +3776,28 @@ class UnresolvedUsingTypenameDecl
   static bool classofKind(Kind K) { return K == UnresolvedUsingTypename; }
 };
 
+/// This node is generated when a using-declaration that was annotated with
+/// __attribute__((using_if_exists)) failed to resolve to a known declaration.
+/// In that case, Sema builds a UsingShadowDecl whose target is an instance of
+/// this declaration, adding it to the current scope. Referring to this
+/// declaration in any way is an error.
+class UnresolvedUsingIfExistsDecl final : public NamedDecl {
+  UnresolvedUsingIfExistsDecl(DeclContext *DC, SourceLocation Loc,
+  DeclarationName Name);
+
+  void anchor() override;
+
+public:
+  static UnresolvedUsingIfExistsDecl *Create(ASTContext &Ctx, DeclContext *DC,
+ SourceLocation Loc,
+ DeclarationName Name);
+  static UnresolvedUsingIfExistsDecl *CreateDeserialized(ASTContext &Ctx,
+ unsigned ID);
+
+  static bool classof(const Decl *D) { return classofKind(D->getKind()); }
+  static bool classofKind(Kind K) { return K == Decl::UnresolvedUsingIfExists; 
}
+};
+
 /// Represents a C++11 static_assert declaration.
 class StaticAssertDecl : public Decl {
   llvm::PointerIntPair AssertExprAndFailed;

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 0623a5c8d5ea8..ab49b39307be8 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1846,6 +1846,8 @@ DEF_TRAVERSE_DECL(UnresolvedUsingTypenameDecl, {
   // source.
 })
 
+DEF_TRAVERSE_DECL(UnresolvedUsingIfExistsDecl, {})
+
 DEF_TRAVERSE_DECL(EnumDecl, {
   TRY_TO(TraverseDeclTemplateParameterLists(D));
 

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 5bfcec7329070..714621087cb77 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3750,6 +3750,14 @@ def NoBuiltin : Attr {
   let Documentation = [NoBuiltinDocs];
 }
 
+def UsingIfExists : InheritableAttr {
+  let Spellings = [Clang<"using_if_exists", 0>];
+  let Subjects = SubjectList<[Using,
+  UnresolvedUsingTypename,
+  UnresolvedUsingValue], ErrorDiag>;
+  let Documentation = [UsingIfExistsDocs];
+}
+
 // FIXME: This attribute is not inheritable, it will not be propagated to
 // redecls. [[clang::lifetimebound]] has the same problems. This should be
 // fixed in TableGen (by probably adding a new inheritable flag).

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index cdbbb38573dab..6594e77d0beef 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.t

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG369c64839946: [clang] Implement the using_if_exists 
attribute (authored by erik.pilkington, committed by ldionne).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90188

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Parser/using-if-exists-attr.cpp
  clang/test/SemaCXX/attr-deprecated.cpp
  clang/test/SemaCXX/using-if-exists.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6466,6 +6466,7 @@
   case Decl::Concept:
   case Decl::LifetimeExtendedTemporary:
   case Decl::RequiresExprBody:
+  case Decl::UnresolvedUsingIfExists:
 return C;
 
   // Declaration kinds that don't make any sense here, but are
Index: clang/test/SemaCXX/using-if-exists.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/using-if-exists.cpp
@@ -0,0 +1,226 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %s -verify
+
+#define UIE __attribute__((using_if_exists))
+
+namespace test_basic {
+namespace NS {}
+
+using NS::x UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+x usex();// expected-error{{reference to unresolved using declaration}}
+
+using NotNS::x UIE; // expected-error{{use of undeclared identifier 'NotNS'}}
+
+using NS::NotNS::x UIE; // expected-error{{no member named 'NotNS' in namespace 'test_basic::NS'}}
+} // namespace test_basic
+
+namespace test_redecl {
+namespace NS {}
+
+using NS::x UIE;
+using NS::x UIE;
+
+namespace NS1 {}
+namespace NS2 {}
+namespace NS3 {
+int A(); // expected-note{{target of using declaration}}
+struct B {}; // expected-note{{target of using declaration}}
+int C(); // expected-note{{conflicting declaration}}
+struct D {}; // expected-note{{conflicting declaration}}
+} // namespace NS3
+
+using NS1::A UIE;
+using NS2::A UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}} expected-note{{conflicting declaration}}
+using NS3::A UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+int i = A();  // expected-error{{reference to unresolved using declaration}}
+
+using NS1::B UIE;
+using NS2::B UIE; // expected-note{{conflicting declaration}} expected-note{{using declaration annotated with 'using_if_exists' here}}
+using NS3::B UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+B myB;// expected-error{{reference to unresolved using declaration}}
+
+using NS3::C UIE;
+using NS2::C UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+int j = C();
+
+using NS3::D UIE;
+using NS2::D UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+D myD;
+} // namespace test_redecl
+
+namespace test_dependent {
+template 
+struct S : B {
+  using B::mf UIE;  // expected-note 3 {{using declaration annotated with 'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+};
+
+struct BaseEmpty {
+};
+struct BaseNonEmpty {
+  void mf();
+  typedef int mt;
+};
+
+template 
+struct UseCtor : Base {
+  using Base::Base UIE; // expected-error{{'using_if_exists' attribute cannot be applied to an inheriting constructor}}
+};
+struct BaseCtor {};
+
+void f() {
+  S empty;
+  S nonempty;
+  empty.mf(); // expected-error {{reference to unresolved using declaration}}
+  nonempty.mf();
+  (&empty)->mf(); // expected-error {{reference to unresolved using declaration}}
+  (&nonempty)->mf();
+
+  S::mt y; // expected-error {{reference to unresolved using declaration}}
+  S::mt z;
+
+  S::mf(); // expected-error {{reference to unresolved using declaration}}
+
+  UseCtor usector;
+}
+
+template 
+struct Implicit : B {
+  using B::mf UIE;  // expected-note {{using declar

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103317#2793557 , @ASDenysPetrov 
wrote:

> Returning to the discussion raised in D102696 
> , I'd like to share my vision.
> I think we can use much easier approach to use valid constraints at any point 
> of time.
> The main idea is lazy-reasoning of the ranges.
> This approach:
>
> - **doesn't** need to use **canonicalization**;
> - **doesn't** need to **update all** the constraints on **each** 
> `setConstraint` call (using brute-force or any other structure traversals);
> - **add** an additional bindings for **every** simple operand on **each** 
> //assignment// or //initialization//.
> - **remove** an invalid bindings for **every** simple operand on **each** 
> //assignment// or //initialization//.
> - **recursively** get the **range** when and only when needed 
> (lazy-reasoning).
>
> Example:
>
>   a = b + 1;
>   c = a - 5;
>   if (b != 42) return 0;
>   if(c == 38) return 1;
>   return 0;
>
> `a = b + 1;`
>
> 1. LHS `a` lookup. LHS not found.
> 2. Add binding `$a = $b + 1`
> 3. Traverse through RHS `b + 1`.
> 4. `b` lookup. `b` not found.
> 5. Built a new expression `a - 1` for `b`.
> 6. Add binding for `b`: `$b = $a - 1`
> 7. `1` is a constant. Skip.
>
> `c = a - 5;`
>
> 1. LHS `c` lookup. LHS not found.
> 2. Add binding `$c = $a - 5`
> 3. Traverse through RHS `a - 5`.
> 4. `a` lookup. `a` found.
> 5. If LHS not found and `a` found, then skip.
> 6. `5` is a constant. Skip.
>
> `if (b != 42) return;`
>
> 1. Get the range of `$b`. Direct range not found. Add `$b` to the list of 
> visited symbols.
> 2. Substitute bindings. From `$b != 42` to `$a - 1 == 42`.
> 3. Traverse through `$a - 1`.
> 4. Get the range of `$a`. Direct range not found. Add `$a` to the list of 
> visited symbols.
> 5. `1` is a constant. Skip.
> 6. Substitute bindings. From `$a - 1 == 42` to `$b + 1 - 1 == 42`.
> 7. Traverse through `$b + 1 - 1`.
> 8. `$b` is in the list of already visited symbols. No way to find a range.
> 9. Create a range from the type [MIN, MAX].
> 10. Perform comparison and produce a constraint for `b` [42, 42].
>
> `if(c == 38) return;`
>
> 1. Get the range of `$c`. Direct range not found. Add `$c` to the list of 
> visited symbols.
> 2. Substitute bindings. From `$c == 38` to `$a - 5 == 38`.
> 3. Traverse through `$a - 5`.
> 4. Get the range of `$a`. Direct range not found. Add `$a` to the list of 
> visited symbols.
> 5. `5` is a constant. Skip.
> 6. Substitute bindings. From `$a - 5 == 38` to `$b + 1 - 5 == 38`.
> 7. Traverse through `$b + 1 - 5`.
> 8. Get the range of `$b`. Direct range **found** [42, 42].
> 9. `1` is a constant. Skip.
> 10. Calculate(simplificate the expression etc.) a new range for `$c` [38, 38].
> 11. Condition is //true//.
>
> `return 1`
>
> P.S. I've just started thinking of the way to solve this problem. For now 
> it's just a sketch of where I'm going.

Great to know that you are also thinking about this problem!
The more the merrier!

I have a couple of notes about your suggestion.
First is minor, `a = b + 1` and `c = a + 5` won't actually produce any 
constraints. Starting from those statements if you request a symbol for `a` you 
will get `b + 1` and `b + 6` for `c`.
Of course, you can change your code so that we make an assumption that `a == b 
+ 1` and `c == a + 5` and those would be constraints.  That's why I said that 
it's minor.

Now, for the biggest concern.  You cover in a very detailed manner all the 
traversals and caches for visited symbols, but have **Substitute bindings** as 
a sub-step , which is HUGE and absolutely non-trivial.
In your examples, you have a linear chain of related symbolic expressions, i.e. 
something like this `x -> y + 2`, `y -> z + 3`, `z -> 4` so that we have `x -> 
9` after three substitutions. But the problem it is generally not one-to-one 
relationship, so `x -> y1 + 1`, `x -> y2 + 2`, ... , `x -> yN + N`.  Imagine 
that each `yK` is involved with another `M` constraints. So, you would need to 
check `x -> y1 -> z11`, if it's not working - check `x -> y1 -> z12` and so on. 
 When you run out of options for `z1K`, you need to backtrack and try `x -> y2 
-> z21`, etc.  As you can see, this is BRUTE FORCE.

And it couldn't be easier theoretically. So, instead of trying that many 
options every time we need to judge about, we take a bit simpler shortcut.  
It's far from being complete, but honestly we're not pursuing completeness.  
For my example above, whenever we find out that `zKM -> 5`, we simplify 
everything that involves `zKM` and get `yK -> 10` and get `x -> 15`.  It is far 
from being perfect, I don't like that we need to go through all symbols and try 
to simplify every single symbol, and it would be good to optimize this scenario 
(mapping symbols to all symbolic expressions that contain them, for example), 
but essentially it is cheaper in the long run.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  htt

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14921
+  verifyFormat("unsigned int   *a;\n"
+   "int*b;\n"
"unsigned int Const *c;\n"

curdeius wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > I seem to remember in the past there was a comment from @djasper in the 
> > > past that he felt you shouldn't align the * like this
> > > 
> > > I think it depends on your viewpoint as to whether the * goes with the 
> > > type or the variable, this could be a religious debate, I don't think you 
> > > can just assume everyone agrees with this, the fact you are changing unit 
> > > tests just leaves me with alarm bells going off...
> > > 
> > > I'm not comfortable with you changing unit tests. This implies that you 
> > > think its a bug, but I think the fact the test exists (Beyonce rule), 
> > > means someone may have wanted to assert that behaviour. 
> > > 
> > >  Should we have an option to control this?
> > As far as I understand it `PAS_Right` says it should stick with the 
> > variable, and thus align this way, if we align declarations.
> > 
> > I think this test was this way because of the `FIXME`.
> I agree with @HazardyKnusperkeks. But I agree that it might be a contentious 
> subject.
The unit tests were wrong IMO and were just hacked that way so that the tests 
don't fail.
Hence, it was correct to fix the tests, because this feature now works.
Normally such tests should be coded as "expected failure" to make this clear, 
but this was not the case.

PAS_Right says it should stick with the variable, not with the type.
See https://www.kernel.org/doc/html/v4.10/process/coding-style.html#spaces for 
an example.



Comment at: clang/unittests/Format/FormatTest.cpp:15046-15048
 "  int const i   = 1;\n"
-"  int * j   = 2;\n"
+"  int  *j   = 2;\n"
 "  int   big = 1;\n"

curdeius wrote:
> curdeius wrote:
> > HazardyKnusperkeks wrote:
> > > But maybe than this should be aligned as that?
> > Hmm, that's a subjective choice here indeed.
> > We might either 1) choose one or another (I'm slightly in favor of aligning 
> > variable names, as it's done currently), or  2) add a config option.
> > For 1), it would be wise to see large projects with a similar style and 
> > what they do (and whethere there's some consensus).
> > Anyone aware of such projects?
> That makes me think. How would be this formatted:
> ```
> int **lotsOfpointers;
> int   i;
> ```
> 
> @gergap, could you add such a test or point me to an existing one that tests 
> the same thing, please?
> That sort of test might fail if pointers/references weren't accounted for in 
> the length of the type name itself.
It's a religious question, like everything related to styles. From my 
experience, aligning at the variable name (excluding type modifiers) is the 
common case. It just looks nicer aligned this way.

```
int   a;
int  *b;
int **c;
int   foobar;
```

It also makes more sense from the logical point of view. First we align the 
variable names then the pointers are placed right most (PAS_Right), left most 
(PAS_Left) or in the middle (PAS_Middle).
The middle one is a more complicated question, because even if the pointers are 
not attached to type or variable there is still the question of the pointer 
alignment. I didn't change anything here and just kept the logic as-is, which 
means pointers are left-aligned with one space between type and pointer

@curdeius I added a new test for this which I will upload shortly.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103245

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

[PATCH] D103087: [clang-tidy] performances-unnecessary-* checks: Extend isOnlyUsedAsConst to expressions and catch const methods returning non-const references/pointers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

I have some concerns about the cost of this checks as it used matching over 
entire contexts quite extensively.  At this point, the facilities involved seem 
quite close to doing dataflow analysis and I wonder if you might be better off 
with a very different implementation. Regardless, have you done any perfomance 
testing to see the impact on real code?




Comment at: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h:30
 
+// Returns true if expression is only used in a const compatible fashion.
+bool isOnlyUsedAsConst(const Expr &E, const Stmt &Stmt, ASTContext &Context);

please expand this comment. It's not obvious (to me) what it means for a 
variable to be used in a "const compatible fashion".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103087

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap updated this revision to Diff 349269.
gergap added a comment.

add new test for checking pointer alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103245

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14881,6 +14881,7 @@
   FormatStyle Alignment = getLLVMStyle();
   Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  Alignment.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
Alignment);
@@ -14916,8 +14917,8 @@
   verifyFormat("int  oneTwoThree{0}; // comment\n"
"unsigned oneTwo; // comment",
Alignment);
-  verifyFormat("unsigned int *  a;\n"
-   "int *   b;\n"
+  verifyFormat("unsigned int   *a;\n"
+   "int*b;\n"
"unsigned int Const *c;\n"
"unsigned int const *d;\n"
"unsigned int Const &e;\n"
@@ -15039,9 +15040,11 @@
"  doublebar();\n"
"};\n",
Alignment);
+
+  // PAS_Right
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"
-"  int * j   = 2;\n"
+"  int  *j   = 2;\n"
 "  int   big = 1;\n"
 "\n"
 "  unsigned oneTwoThree = 123;\n"
@@ -15062,6 +15065,161 @@
"int ll=1;\n"
"}",
Alignment));
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int **j   = 2, ***k;\n"
+"  int  &k   = i;\n"
+"  int &&l   = i + j;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int **j=2,***k;\n"
+   "int &k=i;\n"
+   "int &&l=i+j;\n"
+   " int big  =  1;\n"
+   "\n"
+   "unsigned oneTwoThree  =123;\n"
+   "int oneTwo = 12;\n"
+   "  method();\n"
+   "float k= 2;\n"
+   "int ll=1;\n"
+   "}",
+   Alignment));
+  // variables are aligned at their name, pointers are at the right most
+  // position
+  verifyFormat("int   *a;\n"
+   "int  **b;\n"
+   "int ***c;\n"
+   "intfoobar;\n",
+   Alignment);
+
+  // PAS_Left
+  FormatStyle AlignmentLeft = Alignment;
+  AlignmentLeft.PointerAlignment = FormatStyle::PAS_Left;
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int*  j   = 2;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int *j=2;\n"
+   " int big  =  1;\n"
+   "\n"
+   "unsigned oneTwoThree  =123;\n"
+   "int oneTwo = 12;\n"
+   "  method();\n"
+   "float k= 2;\n"
+   "int ll=1;\n"
+   "}",
+   AlignmentLeft));
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int** j   = 2;\n"
+"  int&  k   = i;\n"
+"  int&& l   = i + j;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int **j=2;\n"
+   "int &k=i;\n"
+   "int &&l=i+j;\n"
+   " int big  =  1;\n"
+   "\n"
+   "uns

[PATCH] D103526: [clang][deps] Mark modular deps to file deps if they come from PCH

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a translation unit uses a PCH and imports the same modules, we'd prefer to 
resolve to those modules instead of inventing new modules and reporting them as 
modular dependencies. Since the PCH modules have already been built, report 
them as file dependencies instead and nudge the compiler to reuse them when 
deciding whether to build a new module.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103526

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu_with_common.json
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_1.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_2.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu_with_common.h
  clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
  clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/tu_with_common.c
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -1,10 +1,124 @@
 // RUN: rm -rf %t && mkdir %t
 // RUN: cp %S/Inputs/modules-pch/* %t
 
+// Scan dependencies of the PCH:
+//
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb_pch.json
+// RUN: echo -%t > %t/result_pch.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json -format experimental-full \
+// RUN:   -generate-modules-path-args -build-dir %t/build -mode preprocess >> %t/result_pch.json
+// RUN: cat %t/result_pch.json | FileCheck %s -check-prefix=CHECK-PCH
+//
+// CHECK-PCH:  -[[PREFIX:.*]]
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "modules": [
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModCommon1"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_1:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_common_1.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "name": "ModCommon1"
+// CHECK-PCH-NEXT: },
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModCommon2"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_2:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_common_2.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "name": "ModCommon2"
+// CHECK-PCH-NEXT: },
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_2]]",
+// CHECK-PCH-NEXT:   "module-name": "ModCommon2"
+// CHECK-PCH-NEXT: }
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModPCH"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_PCH:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_pch.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "name": "ModPCH"
+// CHECK-PCH-NEXT: }
+// CHECK-PCH-NEXT:  

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic updated this revision to Diff 349274.
rsanthir.quic added a comment.

Updated Release Notes and rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102782

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Frontend/backend-stack-usage-diagnostic.c


Index: clang/test/Frontend/backend-stack-usage-diagnostic.c
===
--- /dev/null
+++ clang/test/Frontend/backend-stack-usage-diagnostic.c
@@ -0,0 +1,24 @@
+// RUN: %clang -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wno-stack-usage -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wstack-usage=0 -Wno-stack-usage -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// RUN: %clang -Wstack-usage=0 -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// RUN: %clang -Wstack-usage=3 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wstack-usage=100 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// WARN: warning: stack frame size of {{[0-9]+}} bytes in function 
'test_square'
+// IGNORE-NOT:  stack frame size of {{[0-9]+}} bytes in function 'test_square'
+int test_square(int num) {
+  return num * num;
+}
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4826,12 +4826,14 @@
   D.Diag(diag::err_aix_default_altivec_abi);
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
-StringRef v = A->getValue();
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back(Args.MakeArgString("-warn-stack-size=" + v));
-A->claim();
-  }
+  if (Args.hasFlag(options::OPT_Wframe_larger_than_EQ,
+   options::OPT_Wno_frame_larger_than, false))
+if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
+  StringRef v = A->getValue();
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back(Args.MakeArgString("-warn-stack-size=" + v));
+  A->claim();
+}
 
   if (!Args.hasFlag(options::OPT_fjump_tables, options::OPT_fno_jump_tables,
 true))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2577,6 +2577,11 @@
 def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, 
Group;
 def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias;
 def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, 
Group, Flags<[NoXarchOption]>;
+def Wno_frame_larger_than : Joined<["-"], "Wno-frame-larger-than">, 
Group, Flags<[NoXarchOption]>;
+def Wstack_stack_usage_EQ : Joined<["-"], "Wstack-usage=">, 
Flags<[NoXarchOption]>,
+  Alias;
+def Wno_stack_stack_usage : Joined<["-"], "Wno-stack-usage">, 
Flags<[NoXarchOption]>,
+  Alias;
 
 def : Flag<["-"], "fterminated-vtables">, Alias;
 defm threadsafe_statics : BoolFOption<"threadsafe-statics",
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -76,6 +76,9 @@
   file contains frame size information for each function defined in the source
   file.
 
+- ``-Wstack-usage=`` warn if stack usage of user functions might
+  exceed .
+
 Deprecated Compiler Flags
 -
 


Index: clang/test/Frontend/backend-stack-usage-diagnostic.c
===
--- /dev/null
+++ clang/test/Frontend/backend-stack-usage-diagnostic.c
@@ -0,0 +1,24 @@
+// RUN: %clang -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wno-stack-usage -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wstack-usage=0 -Wno-stack-usage -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// RUN: %clang -Wstack-usage=0 -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// RUN: %clang -Wstack-usage=3 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+
+// RUN: %clang -Wstack-usage=100 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// WARN: warning: stack frame size of {{[0-9]+}} bytes in function 'test_square'
+// IGNORE-NOT:  stack frame size of {{[0-9]+}} by

[PATCH] D103131: support debug info for alias variable

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D103131#2792364 , @dblaikie wrote:

> I will say, as the person who implemented DW_TAG_imported_declaration support 
> in Clang - it's a bit not great/unfortunate (in part because we currently 
> have to emit them for all using declarations (because clang doesn't track 
> some of the usedness we would need to more selectively emit them), and then 
> we can't reap them in LTO or other optimizations if they are effectively 
> unused) - but such is the way of things.

Yeah, we've been carrying a downstream patch to turn off a lot of 'using' 
declarations, and it has been on my wishlist for a long time to get used-ness 
info in there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103131

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

[PATCH] D103527: [Clang][RISCV] Implement vlseg and vlsegff.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai created this revision.
HsiangKai added reviewers: frasercrmck, craig.topper, rogfer01.
Herald added subscribers: StephenFan, vkmr, dexonsmith, evandro, luismarques, 
apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
HsiangKai requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Implement the new Zvlsseg intrinsic interface proposed in 
https://github.com/riscv/rvv-intrinsic-doc/issues/95


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103527

Files:
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vlseg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vlseg.c
  clang/utils/TableGen/RISCVVEmitter.cpp

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

[PATCH] D103039: [AST] fully-qualify template args of outer types in getFullyQualifiedType

[Sterling Augustine via Phabricator via cfe-commits]

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

This is a long-standing bug that needed to be fixed. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103039

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D103317#2793658 , @vsavchenko 
wrote:

> But the problem it is generally not one-to-one relationship, so `x -> y1 + 
> 1`, `x -> y2 + 2`, ... , `x -> yN + N`.

In my approach it can't be more then one binding for one symbol. Like:
`x = y + z;` produces `$x = $y + $z`, `$y = $x - $z`, `$z = $x - $y`.
Then if `z = a;`, all above should be invalidated (removed), because we can't 
rely on those expressions any more. And new ones should be added `$z = $a` and 
`$a = $z`.

> As you can see, this is BRUTE FORCE.

I would say this is a `linked list` with stop-markers, not blindly passing 
through all the bindings. And I don't talk about implementation details. For 
now we just need to elaborate an approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D102822: [Clang][CodeGen] Set the size of llvm.lifetime to unknown for scalable types.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102822

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

Hello! There are still some false positives, for example this one is breaking 
the libc++ CI: 
https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848

Would it be possible to either fix this quickly or revert the change until the 
issue has been fixed? Our pre-commit CI is going to be stalled until this is 
fixed. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103317#2793781 , @ASDenysPetrov 
wrote:

> In D103317#2793658 , @vsavchenko 
> wrote:
>
>> But the problem it is generally not one-to-one relationship, so `x -> y1 + 
>> 1`, `x -> y2 + 2`, ... , `x -> yN + N`.
>
> In my approach it can't be more then one binding for one symbol. Like:
> `x = y + z;` produces `$x = $y + $z`, `$y = $x - $z`, `$z = $x - $y`.

Hmm, Okay, but what about situations if you have: `a = a1 + a2` and `a = a3 + 
a4 + a5` are you going to throw away one of these constraints? And if so, how 
do you want to select which one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for adding the tests. 👍




Comment at: clang/unittests/Format/FormatTest.cpp:15095
+   Alignment));
+  // variables are aligned at their name, pointers are at the right most
+  // position

Nit: Comments should start with a capital letter and end with a fullstop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103245

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

[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 349295.
RedDocMD added a comment.

trackExpressionValue recieving callback, passing it to ReturnVisitor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103434

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -320,6 +320,22 @@
   return P;
 }
 
+//===--===//
+// Implementation of StoppableBugReporterVisitor
+//===--===//
+
+StoppableBugReporterVisitor::~StoppableBugReporterVisitor() {
+  assert(Stopped && "Stoppable visitor not stopped in its lifetime");
+}
+
+void StoppableBugReporterVisitor::Stop(const ExplodedNode *Curr,
+   BugReporterContext &BRC,
+   PathSensitiveBugReport &BR) {
+  assert(!Stopped && "Stop() can be called only once in visitor's lifetime");
+  Stopped = true;
+  OnStop(Curr, BRC, BR);
+}
+
 //===--===//
 // Implementation of NoStoreFuncVisitor.
 //===--===//
@@ -883,7 +899,7 @@
 ///
 /// This visitor is intended to be used when another visitor discovers that an
 /// interesting value comes from an inlined function call.
-class ReturnVisitor : public BugReporterVisitor {
+class ReturnVisitor : public StoppableBugReporterVisitor {
   const StackFrameContext *CalleeSFC;
   enum {
 Initial,
@@ -895,12 +911,14 @@
   bool ShouldInvalidate = true;
   AnalyzerOptions& Options;
   bugreporter::TrackingKind TKind;
+  VisitorCallback Callback;
 
 public:
   ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
-AnalyzerOptions &Options, bugreporter::TrackingKind TKind)
-  : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
-Options(Options), TKind(TKind) {}
+AnalyzerOptions &Options, bugreporter::TrackingKind TKind,
+VisitorCallback &&Callback)
+  : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed), Options(Options),
+TKind(TKind), Callback(Callback) {}
 
   static void *getTag() {
 static int Tag = 0;
@@ -913,6 +931,13 @@
 ID.AddBoolean(EnableNullFPSuppression);
   }
 
+  void OnStop(const ExplodedNode *Curr, BugReporterContext &BRC,
+  PathSensitiveBugReport &BR) const override {
+if (Callback) {
+  Callback(Curr, BRC, BR);
+}
+  }
+
   /// Adds a ReturnVisitor if the given statement represents a call that was
   /// inlined.
   ///
@@ -923,7 +948,8 @@
   static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
 PathSensitiveBugReport &BR,
 bool InEnableNullFPSuppression,
-bugreporter::TrackingKind TKind) {
+bugreporter::TrackingKind TKind,
+VisitorCallback Callback) {
 if (!CallEvent::isCallStmt(S))
   return;
 
@@ -994,9 +1020,9 @@
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-BR.addVisitor(std::make_unique(CalleeContext,
-   EnableNullFPSuppression,
-   Options, TKind));
+BR.addVisitor(
+std::make_unique(CalleeContext, EnableNullFPSuppression,
+Options, TKind, std::move(Callback)));
   }
 
   PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N,
@@ -1123,6 +1149,10 @@
 else
   BR.markInteresting(CalleeSFC);
 
+// Since we know that no other notes will be emitted subsequently by this
+// visitor.
+Stop(N, BRC, BR);
+
 return EventPiece;
   }
 
@@ -1966,7 +1996,8 @@
const Expr *E,
PathSensitiveBugReport &report,
bugreporter::TrackingKind TKind,
-   bool EnableNullFPSuppression) {
+   bool EnableNullFPSuppression,
+   VisitorCallback Callback) {
 
   if (!E || !InputNode)
 return false;
@@ -2067,7 +2098,7 @@
   SVal V = LVState->getSValAsScalarOrLoc(Inner, LVNode->getLocationContext());
 
   ReturnVisitor::addVisitorIfNecessary(
-LVNode, Inner, report, EnableNullFPSuppression, TKind

[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

The following things are now done:

- `trackExpressionValue` must receive a callback
- The callback must be passed to a visitor (`ReturnVisitor` for now)

I still don't know a good way to test the following:

- Tests to verify that `ReturnVisitor` actually does what we intend it to do 
(call the callback at the right place).

@vsavchenko, @NoQ, @xazax.hun, @teemperor can you give some suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103434

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D103317#2793797 , @vsavchenko 
wrote:

> Hmm, Okay, but what about situations if you have: `a = a1 + a2` and `a = a3 + 
> a4 + a5` are you going to throw away one of these constraints? And if so, how 
> do you want to select which one?

Are you talking about comparison or assignment? Both assignments can't be valid 
at the same time, and latter replaces bindings of former. In case of 
comparisons, they both can be valid.
But we should keep in mind that **assignment** is a //write// operation which 
replaces and invalidates previous bindings, and comparison a //read// 
operation. It can add new bindings but can not remove old ones.
This what I haven't dig deep enough yet. Let's do this together how we can 
handle that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D100581#2793775 , @ldionne wrote:

> Hello! There are still some false positives, for example this one is breaking 
> the libc++ CI: 
> https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848
>
> Would it be possible to either fix this quickly or revert the change until 
> the issue has been fixed? Our pre-commit CI is going to be stalled until this 
> is fixed. Thanks!

Looks like a true positive in libc++ ( 
https://github.com/llvm/llvm-project/blob/main/libcxx/include/regex ) - the 
"__j" variable is initialized, then incremented, but never read (except to 
increment). Is that a bug in libc++? Was __j meant to be used for something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Fred Grim via Phabricator via cfe-commits]

feg208 updated this revision to Diff 349299.
feg208 added a comment.

I am of the view that this is complete. Or at a minimum all the tests that have
been requested pass and I would be surprised if there were gross errors or other
major things I have missed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103317#2793911 , @ASDenysPetrov 
wrote:

> In D103317#2793797 , @vsavchenko 
> wrote:
>
>> Hmm, Okay, but what about situations if you have: `a = a1 + a2` and `a = a3 
>> + a4 + a5` are you going to throw away one of these constraints? And if so, 
>> how do you want to select which one?
>
> Are you talking about comparison or assignment? Both assignments can't be 
> valid at the same time, and latter replaces bindings of former. In case of 
> comparisons, they both can be valid.
> But we should keep in mind that **assignment** is a //write// operation which 
> replaces and invalidates previous bindings, and **comparison** is a //read// 
> operation. It can add new bindings but can not remove old ones.
> This is what I haven't dig deep enough yet. Let's do this together how we can 
> handle that.

I replied to you earlier that assignments are not producing constraints.  The 
analyzer has some sort of SSA (not really, but anyways), so every time we 
reassign the variable it actually becomes a different symbol. So, from this 
perspective `DeclRefExpr` `x` and symbol `x` are two different things:

  int x = foo(); // after this DeclRefExpr and VarDecl x are associated with 
conjured symbol #1 (aka conj#1)
  int c = x + 10; // c is associated with `$conj#1 + 10`
  x = a + 1; // DeclRefExpr x is now associated with `$a + 1`
  x = c + x; // and now it is `conj#1 + 11`

There is no symbolic assignment in the static analyzer.  Symbols are values, 
values don't change.  We can only obtain more information of what particular 
symbolic value represents.

I hope it makes it more clear.

As for equality, we already track equivalence classes of symbols and they can 
have more than 2 symbols in them.  Actually, this whole data structure mostly 
makes sense for cases when the class has more than 2 elements in it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102782

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

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:657-667
+  // Certain kinds unfortunately need to be side-stepped for canonical type
+  // matching.
+  if (LType->getAs() || RType->getAs()) {
+// Unfortunately, the canonical type of a function pointer becomes the
+// same even if exactly one is "noexcept" and the other isn't, making us
+// give a false positive report irrespective of implicit conversions.
+LLVM_DEBUG(llvm::dbgs()

aaron.ballman wrote:
> whisperity wrote:
> > @aaron.ballman Getting ahead of the curve here. I understand this is ugly. 
> > Unfortunately, no matter how much I read the standard, I don't get anything 
> > of "canonical type" mentioned, it seems to me this concept is something 
> > inherent to the model of Clang.
> > 
> > Basically why this is here: imagine a `void (*)() noexcept` and a `void 
> > (*)()`. One's `noexcept`, the other isn't. Inside the AST, this is a 
> > `ParenType` of a `PointerType` to a `FunctionProtoType`. There exists a 
> > //one-way// implicit conversion from the `noexcept` to the non-noexcept 
> > ("noexceptness can be discarded", similarly to how "constness can be 
> > gained")
> > Unfortunately, because this is a one-way implicit conversion, it won't 
> > return on the code path earlier for implicit conversions.
> > 
> > Now because of this, the recursive descent in our code will reach the point 
> > when the two innermost `FunctionProtoType`s are in our hands. As a fallback 
> > case, we simply ask Clang "Hey, do //you// think these two are the same?". 
> > And for some weird reason, Clang will say "Yes."... While one of them is a 
> > `noexcept` function and the other one isn't.
> > 
> > I do not know the innards of the AST well enough to even have a glimpse of 
> > whether or not this is a bug. So that's the reason why I went ahead and 
> > implemented this side-stepping logic. Basically, as the comment says, 
> > it'lll **force** the information of "No matter what you do, do NOT consider 
> > these mixable!" back up the recursion chain, and handle it appropriately 
> > later.
> > Unfortunately, no matter how much I read the standard, I don't get anything 
> > of "canonical type" mentioned, it seems to me this concept is something 
> > inherent to the model of Clang.
> 
> It is more of a Clang-centric concept. Basically, a canonical type is one 
> that's had all of the typedefs stripped off it.
> 
> > Now because of this, the recursive descent in our code will reach the point 
> > when the two innermost FunctionProtoTypes are in our hands. As a fallback 
> > case, we simply ask Clang "Hey, do you think these two are the same?". And 
> > for some weird reason, Clang will say "Yes."... While one of them is a 
> > noexcept function and the other one isn't.
> 
> I think a confounding factor in this area is that `noexcept` did not used to 
> be part of the function type until one day it started being a part of the 
> function type. So my guess is that this is fallout from this sort of thing: 
> https://godbolt.org/z/EYfj8z (which may be worth keeping in mind when working 
> on the check).
About `noexcept`: we've faced a similar problem in the `ASTImporter` library. 
We could not import a noexcept function's definition if we already had one 
without the noexcept specifier. 

Thus, in `ASTStructuralEquivalence.cpp` we do differentiate the function types 
based on their noexcept specifier (and we even check the noexcept expression).:
```
TEST_F(StructuralEquivalenceFunctionTest, Noexcept) {
  auto t = makeNamedDecls("void foo();",
  "void foo() noexcept;", Lang_CXX11);
  EXPECT_FALSE(testStructuralMatch(t));
}
TEST_F(StructuralEquivalenceFunctionTest, NoexceptNonMatch) {
  auto t = makeNamedDecls("void foo() noexcept(false);",
  "void foo() noexcept(true);", Lang_CXX11);
  EXPECT_FALSE(testStructuralMatch(t));
}
```

May be in these special cases it would worth to reuse the 
ASTStructuralEquivalenceContext class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041

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

[PATCH] D102849: [flang][driver] Add support for the "-init-only" option

[Stuart Ellis via Phabricator via cfe-commits]

stuartellis updated this revision to Diff 349307.
stuartellis added a comment.

Address review comments
Renaming InitializationOnly to InitOnly
Add missing line in init-only.f90
Updating commit message with init-only/test-io comparison


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102849

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/init-only.f90

Index: flang/test/Driver/init-only.f90
===
--- /dev/null
+++ flang/test/Driver/init-only.f90
@@ -0,0 +1,7 @@
+! Verify that -init-only flag generates a diagnostic as expected
+
+! REQUIRES: new-flang-driver
+
+! RUN: %flang_fc1 -init-only 2>&1 | FileCheck %s
+
+! CHECK: warning: Use `-init-only` for testing purposes only
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -104,6 +104,7 @@
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
 ! HELP-FC1-NEXT: -help  Display available options
+! HELP-FC1-NEXT: -init-only Only execute frontend initialization
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of include search paths
 ! HELP-FC1-NEXT: -module-dir   Put MODULE files in 
 ! HELP-FC1-NEXT: -nocpp Disable predefined and command line preprocessor macros
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -73,6 +73,9 @@
   case GetSymbolsSources:
 return std::make_unique();
 break;
+  case InitOnly:
+return std::make_unique();
+break;
   default:
 break;
 // TODO:
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -447,3 +447,11 @@
   clang::DiagnosticsEngine::Error, "code-generation is not available yet");
   ci.diagnostics().Report(DiagID);
 }
+
+void InitOnlyAction::ExecuteAction() {
+  CompilerInstance &ci = this->instance();
+  unsigned DiagID =
+  ci.diagnostics().getCustomDiagID(clang::DiagnosticsEngine::Warning,
+  "Use `-init-only` for testing purposes only");
+  ci.diagnostics().Report(DiagID);
+}
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -162,9 +162,12 @@
 case clang::driver::options::OPT_fget_definition:
   opts.programAction_ = GetDefinition;
   break;
+case clang::driver::options::OPT_init_only:
+  opts.programAction_ = InitOnly;
+  break;
 
   // TODO:
-  // case calng::driver::options::OPT_emit_llvm:
+  // case clang::driver::options::OPT_emit_llvm:
   // case clang::driver::options::OPT_emit_llvm_only:
   // case clang::driver::options::OPT_emit_codegen_only:
   // case clang::driver::options::OPT_emit_module:
Index: flang/include/flang/Frontend/FrontendOptions.h
===
--- flang/include/flang/Frontend/FrontendOptions.h
+++ flang/include/flang/Frontend/FrontendOptions.h
@@ -71,7 +71,10 @@
   GetDefinition,
 
   /// Parse, run semantics and then dump symbol sources map
-  GetSymbolsSources
+  GetSymbolsSources,
+
+  /// Only execute frontend initialization
+  InitOnly
 
   /// TODO: RunPreprocessor, EmitLLVM, EmitLLVMOnly,
   /// EmitCodeGenOnly, EmitAssembly, (...)
Index: flang/include/flang/Frontend/FrontendActions.h
===
--- flang/include/flang/Frontend/FrontendActions.h
+++ flang/include/flang/Frontend/FrontendActions.h
@@ -38,6 +38,10 @@
   void ExecuteAction() override;
 };
 
+class InitOnlyAction : public FrontendAction {
+  void ExecuteAction() override;
+};
+
 //===--===//
 // Prescan Actions
 //===--===//
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5224,8 +5224,6 @@
   HelpText<

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Zhaomo Yang via Phabricator via cfe-commits]

zhaomo added a comment.

Thanks folks!


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

https://reviews.llvm.org/D103195

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D100581#2793926 , @dblaikie wrote:

> In D100581#2793775 , @ldionne wrote:
>
>> Hello! There are still some false positives, for example this one is 
>> breaking the libc++ CI: 
>> https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848
>>
>> Would it be possible to either fix this quickly or revert the change until 
>> the issue has been fixed? Our pre-commit CI is going to be stalled until 
>> this is fixed. Thanks!
>
> Looks like a true positive in libc++ ( 
> https://github.com/llvm/llvm-project/blob/main/libcxx/include/regex ) - the 
> "__j" variable is initialized, then incremented, but never read (except to 
> increment). Is that a bug in libc++? Was __j meant to be used for something?

Oh, you're right! I was tripped by the fact that we did in fact "use" `__j` (in 
the sense that we do `__j += ...`), but indeed we never read the result. I'll 
work on fixing that issue. I'm not sure whether it's a bug or not yet, that 
code was modified 11 years ago, but I'll do my research.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.



> ! In D103317#2793797 , @vsavchenko 
> wrote:



> I replied to you earlier that assignments are not producing constraints.  The 
> analyzer has some sort of SSA (not really, but anyways), so every time we 
> reassign the variable it actually becomes a different symbol. So, from this 
> perspective `DeclRefExpr` `x` and symbol `x` are two different things:
>
>   int x = foo(); // after this DeclRefExpr and VarDecl x are associated with 
> conjured symbol #1 (aka conj#1)
>   int c = x + 10; // c is associated with `$conj#1 + 10`
>   x = a + 1; // DeclRefExpr x is now associated with `$a + 1`
>   x = c + x; // and now it is `conj#1 + 11`
>
> There is no symbolic assignment in the static analyzer.  Symbols are values, 
> values don't change.  We can only obtain more information of what particular 
> symbolic value represents.
>
> I hope it makes it more clear.
>
> As for equality, we already track equivalence classes of symbols and they can 
> have more than 2 symbols in them.  Actually, this whole data structure mostly 
> makes sense for cases when the class has more than 2 elements in it.

Thanks for the clarifications.

I'm trying to say that we need to keep our attention on 
assignments/initializations as well if we want to solve this problem 
efficiently. I didn't say that assignments produce constraints. Of course they 
do not :-). I want we produce more //bindings(associations)// on 
//assignments// to use them as mapping for further substitutions and traversing 
to get the right range.

As for the equivalence classes. I think we should go away from special cases. I 
mean what about lower-then classes or greater-or-equal classes. I think you got 
what I mean. I'm trying to find a general approach which can cover almost all 
individual improvements around CSA codebase.

I'm started working on this when saw your discussion. Just what you also 
consider the direction I'm moving. A lot of corner-examples, like you gave me, 
are welcome. Thank you for the feedbacks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[clang] d0e1593 - Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Yitzhak Mandelbaum via cfe-commits]

Author: Zhaomo Yang
Date: 2021-06-02T17:28:14Z
New Revision: d0e159334f9d1285ec35cf71465358c47141618c

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

LOG: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

This patch adds support for matching gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL 
and EXPECT_CALL macros.

Reviewed By: ymandel, hokein

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

Added: 


Modified: 
clang/include/clang/ASTMatchers/GtestMatchers.h
clang/lib/ASTMatchers/GtestMatchers.cpp
clang/unittests/ASTMatchers/GtestMatchersTest.cpp

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/GtestMatchers.h 
b/clang/include/clang/ASTMatchers/GtestMatchers.h
index 4f8addcf744ae..e19d91a674f2e 100644
--- a/clang/include/clang/ASTMatchers/GtestMatchers.h
+++ b/clang/include/clang/ASTMatchers/GtestMatchers.h
@@ -16,6 +16,7 @@
 
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace ast_matchers {
@@ -30,14 +31,55 @@ enum class GtestCmp {
   Lt,
 };
 
-/// Matcher for gtest's ASSERT_... macros.
+/// This enum indicates whether the mock method in the matched ON_CALL or
+/// EXPECT_CALL macro has arguments. For example, `None` can be used to match
+/// `ON_CALL(mock, TwoParamMethod)` whereas `Some` can be used to match
+/// `ON_CALL(mock, TwoParamMethod(m1, m2))`.
+enum class MockArgs {
+  None,
+  Some,
+};
+
+/// Matcher for gtest's ASSERT comparison macros including ASSERT_EQ, 
ASSERT_NE,
+/// ASSERT_GE, ASSERT_GT, ASSERT_LE and ASSERT_LT.
 internal::BindableMatcher gtestAssert(GtestCmp Cmp, StatementMatcher 
Left,
 StatementMatcher Right);
 
-/// Matcher for gtest's EXPECT_... macros.
+/// Matcher for gtest's ASSERT_THAT macro.
+internal::BindableMatcher gtestAssertThat(StatementMatcher Actual,
+StatementMatcher Matcher);
+
+/// Matcher for gtest's EXPECT comparison macros including EXPECT_EQ, 
EXPECT_NE,
+/// EXPECT_GE, EXPECT_GT, EXPECT_LE and EXPECT_LT.
 internal::BindableMatcher gtestExpect(GtestCmp Cmp, StatementMatcher 
Left,
 StatementMatcher Right);
 
+/// Matcher for gtest's EXPECT_THAT macro.
+internal::BindableMatcher gtestExpectThat(StatementMatcher Actual,
+StatementMatcher Matcher);
+
+/// Matcher for gtest's EXPECT_CALL macro. `MockObject` matches the mock
+/// object and `MockMethodName` is the name of the method invoked on the mock
+/// object.
+internal::BindableMatcher gtestExpectCall(StatementMatcher MockObject,
+llvm::StringRef MockMethodName,
+MockArgs Args);
+
+/// Matcher for gtest's EXPECT_CALL macro. `MockCall` matches the whole mock
+/// member method call. This API is more flexible but requires more knowledge 
of
+/// the AST structure of EXPECT_CALL macros.
+internal::BindableMatcher gtestExpectCall(StatementMatcher MockCall,
+MockArgs Args);
+
+/// Like the first `gtestExpectCall` overload but for `ON_CALL`.
+internal::BindableMatcher gtestOnCall(StatementMatcher MockObject,
+llvm::StringRef MockMethodName,
+MockArgs Args);
+
+/// Like the second `gtestExpectCall` overload but for `ON_CALL`.
+internal::BindableMatcher gtestOnCall(StatementMatcher MockCall,
+MockArgs Args);
+
 } // namespace ast_matchers
 } // namespace clang
 

diff  --git a/clang/lib/ASTMatchers/GtestMatchers.cpp 
b/clang/lib/ASTMatchers/GtestMatchers.cpp
index 0e587c0c3b9f6..79f69c63c10f0 100644
--- a/clang/lib/ASTMatchers/GtestMatchers.cpp
+++ b/clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -5,76 +5,105 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
+//
+// This file implements several matchers for popular gtest macros. In general,
+// AST matchers cannot match calls to macros. However, we can simulate such
+// matches if the macro definition has identifiable elements that themselves 
can
+// be matched. In that case, we can match on those elements and then check that
+// the match occurs within an expansion of the desired macro. The more uncommon
+// the identified elements, the more efficient this process will be.
+//
+//===--===//
 
 #include "clang/ASTMatchers/GtestMatchers.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #in

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0e159334f9d: Add matchers for gtest's ASSERT_THAT, 
EXPECT_THAT, ON_CALL and EXPECT_CALL (authored by zhaomo, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103195

Files:
  clang/include/clang/ASTMatchers/GtestMatchers.h
  clang/lib/ASTMatchers/GtestMatchers.cpp
  clang/unittests/ASTMatchers/GtestMatchersTest.cpp

Index: clang/unittests/ASTMatchers/GtestMatchersTest.cpp
===
--- clang/unittests/ASTMatchers/GtestMatchersTest.cpp
+++ clang/unittests/ASTMatchers/GtestMatchersTest.cpp
@@ -42,6 +42,14 @@
 #define EXPECT_PRED_FORMAT2(pred_format, v1, v2) \
 GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_NONFATAL_FAILURE_)
 
+#define GTEST_PRED_FORMAT1_(pred_format, v1, on_failure) \
+  GTEST_ASSERT_(pred_format(#v1, v1), on_failure)
+
+#define EXPECT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_NONFATAL_FAILURE_)
+#define ASSERT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_FATAL_FAILURE_)
+
 #define EXPECT_EQ(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define EXPECT_NE(val1, val2) \
@@ -55,11 +63,29 @@
 #define EXPECT_LT(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::CmpHelperLT, val1, val2)
 
+#define ASSERT_THAT(value, matcher) \
+  ASSERT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+#define EXPECT_THAT(value, matcher) \
+  EXPECT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+
 #define ASSERT_EQ(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define ASSERT_NE(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)
 
+#define GMOCK_ON_CALL_IMPL_(mock_expr, Setter, call)\
+  ((mock_expr).gmock_##call)(::testing::internal::GetWithoutMatchers(), \
+ nullptr)   \
+  .Setter(nullptr, 0, #mock_expr, #call)
+
+#define ON_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalDefaultActionSetAt, call)
+
+#define EXPECT_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalExpectedAt, call)
+
   namespace testing {
   namespace internal {
   class EqHelper {
@@ -96,8 +122,77 @@
   const T2& val2) {
 return 0;
   }
+
+  // For implementing ASSERT_THAT() and EXPECT_THAT().  The template
+  // argument M must be a type that can be converted to a matcher.
+  template 
+  class PredicateFormatterFromMatcher {
+   public:
+explicit PredicateFormatterFromMatcher(M m) : matcher_(m) {}
+
+// This template () operator allows a PredicateFormatterFromMatcher
+// object to act as a predicate-formatter suitable for using with
+// Google Test's EXPECT_PRED_FORMAT1() macro.
+template 
+int operator()(const char* value_text, const T& x) const {
+  return 0;
+}
+
+   private:
+const M matcher_;
+  };
+
+  template 
+  inline PredicateFormatterFromMatcher MakePredicateFormatterFromMatcher(
+  M matcher) {
+return PredicateFormatterFromMatcher(matcher);
+  }
+
+  bool GetWithoutMatchers() { return false; }
+
+  template 
+  class MockSpec {
+   public:
+MockSpec() {}
+
+bool InternalDefaultActionSetAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+bool InternalExpectedAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+MockSpec operator()(bool, void*) {
+  return *this;
+}
+  };  // class MockSpec
+
   }  // namespace internal
+
+  template 
+  int StrEq(T val) {
+return 0;
+  }
+  template 
+  int Eq(T val) {
+return 0;
+  }
+
   }  // namespace testing
+
+  class Mock {
+public:
+Mock() {}
+testing::internal::MockSpec gmock_TwoArgsMethod(int, int) {
+  return testing::internal::MockSpec();
+}
+testing::internal::MockSpec gmock_TwoArgsMethod(bool, void*) {
+  return testing::internal::MockSpec();
+}
+  };  // class Mock
 )cc";
 
 static std::string wrapGtest(llvm::StringRef Input) {
@@ -187,5 +282,137 @@
   matches(wrapGtest(Input), gtestExpect(GtestCmp::Gt, expr(), expr(;
 }
 
+TEST(GtestExpectTest, ThatShouldMatchAssertThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { ASSERT_THAT(2, Eq(2)); }
+  )cc";
+  EXPECT_TRUE(matches(
+  wrapGtest(Input),
+  gtestAssertThat(
+  expr(), callExpr(callee(functionDecl(hasName("::testing::Eq")));
+}
+
+TEST(GtestExpectTest, ThatShouldMatchExpectThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { EXPECT_T

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: ahatanak.
rjmccall added a comment.

Hmm.  CC'ing Akira.  Akira, do you know why we're building a fake FunctionDecl 
as part of emitting these helper functions?  Is it something we can avoid?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Fred Grim via Phabricator via cfe-commits]

feg208 updated this revision to Diff 349324.
feg208 added a comment.

oops left some debugging messages in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ "that really, in any ju

[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

[Adam Czachorowski via Phabricator via cfe-commits]

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

In --check mode we do not run code completion because it is too slow,
especially on larger files. With the introducation of --check-lines we
can narrow down the scope and thus we can afford to do code completion.

We vlog() the top completion result, but that's not really the point.
The most value will come from being able to reproduce crashes that occur
during code completion and require preamble build or index (and thus are
more difficult to reproduce with -code-complete-at).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103538

Files:
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   const bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
   }
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,10 +193,15 @@
 
   // Run AST-based features at each token in the file.
   void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine) {
+  llvm::function_ref ShouldCheckLine,
+  const bool EnableCodeCompletion) {
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
 auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
+
+CodeCompleteOptions CCOpts = Opts.CodeComplete;
+CCOpts.Index = &Index;
+
 for (const auto &Tok : SpelledTokens) {
   unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
   unsigned End = Start + Tok.length();
@@ -228,8 +233,12 @@
   auto Hover = getHover(*AST, Pos, Style, &Index);
   vlog("hover: {0}", Hover.hasValue());
 
-  // FIXME: it'd be nice to include code completion, but it's too slow.
-  // Maybe in combination with a line restriction?
+  if (EnableCodeCompletion) {
+Position EndPos = offsetToPosition(Inputs.Contents, End);
+auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
+vlog("code completion: {0}",
+ CC.Completions.empty() ? "" : CC.Completions[0].Name);
+  }
 }
   }
 };
@@ -238,7 +247,8 @@
 
 bool check(llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   const bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
   llvm::Optional Contents;
   if (File.empty()) {
@@ -262,7 +272,7 @@
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine);
+  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@
 // Implemented in Check.cpp.
 bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   const bool EnableCodeCompletion);
 
 namespace {
 
@@ -929,7 +930,11 @@
   uint32_t Line = Pos.line + 1; // Position::line is 0-based.
   return Line >= Begin && Line <= End;
 };
-return check(Path, ShouldCheckLine, TFS, Opts)
+// Fo

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

[Chris Lattner via Phabricator via cfe-commits]

lattner added inline comments.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600
+  if (RHS.data() == getEmptyKey().data())
+return LHS.data() == getEmptyKey().data();
+  if (RHS.data() == getTombstoneKey().data())
+return LHS.data() == getTombstoneKey().data();
+  return LHS == RHS;
+}

nikic wrote:
> lattner wrote:
> > craig.topper wrote:
> > > lattner wrote:
> > > > I'm pretty sure this method can just be "return LHS == RHS;"  The 
> > > > tombstone/empty comparisons should work without special cases.
> > > Doesn't operator== on ArrayRef compare the elements of the arrays? So 
> > > wouldn't that dereference the invalid pointers used by tombstone/empty?
> > Yes, implemented in terms of std::equal.  However, both of these cases have 
> > zero elements, so the "pointer" is never dereferenced.
> As the length is zero, wouldn't the empty key, the tombstone key and an empty 
> ArrayRef all be considered equal, as the data pointer is never compared?
Yes, you're right, this won't work.  Good catch, thanks :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103491

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

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

[Sam McCall via Phabricator via cfe-commits]

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

Thanks, and sorry for the long journey to get here :-)




Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1542
Semaphore &Sem) {
+  if (!Path.empty())
+LastActiveFile = Path.str();

this works, but for some reason my brain finds this both cuter and easier to 
parse
```
if (Path.empty())
  Path = LastActiveFile;
else
  LastActiveFile = Path.str();
// and capture Path as before
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103476

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

[PATCH] D103028: [clang][ARM] Remove arm2/3/6/7m CPU names

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103028

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

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD marked 7 inline comments as done.
RedDocMD added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:258
+  const Decl *D = DS->getSingleDecl();
+  assert(D && "DeclStmt should have at least one Decl");
+  const auto *VD = llvm::dyn_cast(D);

RedDocMD wrote:
> NoQ wrote:
> > That's not what the assert is saying; the assert is saying that the 
> > `DeclStmt` has //exactly// one `Decl`. It basically forbids code like
> > ```
> > int x = 1, y = 2;
> > ```
> > . You may wonder why don't you crash all over the place. That's because 
> > Clang CFG [[ 
> > https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/lib/Analysis/CFG.cpp#L2826
> >  | creates its own ]] `DeclStmt`s that aren't normally present in the AST, 
> > that always have exactly one declaration. This is necessary because there 
> > may be non-trivial control flow between these declarations (due to, say, 
> > presence of operator `?:` in the initializer) so they have to be 
> > represented as different elements (possibly even in different blocks) in 
> > the CFG.
> So I guess the tests at lines `317` and `378` of `smart-ptr-text-output.cpp` 
> work because of the CFG messing with the AST? So should I remove the assert?
@NoQ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

[Sam McCall via Phabricator via cfe-commits]

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

changes is in fact optional, indicated by the `?` in `changes?: { [uri: 
DocumentUri]: TextEdit[]; };`.

But the spec requires *some* field to be set, and this is the only one we 
support, so it's not optional in practice for us.




Comment at: clang-tools-extra/clangd/Protocol.cpp:811
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
+  if (WE.changes.empty())
 return llvm::json::Object{};

I'd suggest deleting this special case.
We no longer have two distinct states of our WorkspaceEdit struct to represent 
`{changes:{}}` and `{}`.

`{changes:{}}` is a well-defined empty edit, even if that never makes sense to 
actually send.
`{}` conforms to the typescript type definition of the spec, but doesn't 
actually define an edit per the spec text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103449

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

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 349332.
RedDocMD added a comment.

Moved visitor entirely to SmartPtrChecker.cpp, other refactors, better naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -306,10 +306,81 @@
 };
 
 void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
-  A *RP = P.get();
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
   if (!RP) { // expected-note {{Assuming 'RP' is null}}
 // expected-note@-1 {{Taking true branch}}
 P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void multpleDeclsWithGet(std::unique_ptr P) {
+  A *dummy1 = nullptr, *RP = P.get(), *dummy2; // expected-note {{Obtained null inner pointer from 'P'}}
+  if (!RP) {   // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void multipleGetsShouldNotAllHaveNotes(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  A *dummy1 = P.get();
+  A *dummy2 = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldNotAlwaysLeaveANote() {
+  std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  A *a = P.get();
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void getShouldNotLeaveANoteAfterReset(std::unique_ptr P) {
+  A *a = P.get();
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  P->foo();  // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get();
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithWhileLoop(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  while (!RP) {// expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithForLoop(std::unique_ptr P) {
+  for (A *RP = P.get(); !RP;) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Obtained null inner pointer from 'P'}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveNoteOnChaining(std::unique_ptr P) {
+  A *praw = P.get(), *other; // expected-note {{Obtained null inner pointer from 'P'}}
+  other = praw;  // expected-note {{Obtained null value here}}
+  if (!other) {  // expected-note {{Assuming 'other' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -76,11 +76,18 @@
   {{"release"}, &SmartPtrModeling::handleRelease},
   {{"swap", 1}, &

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.

Sorry about delay.




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+  case Config::ExternalIndexSpec::None:
+break;
   case Config::ExternalIndexSpec::Server:

I think you hit llvm_unreachable here - is this an invariant enforced by the 
caller, or did you mean to return nullptr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

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

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

Important question from @vsavchenko:

> I have two major questions about this implementation:
>
> - Why don't we need an actual check for `IfStmt`? Won't it trigger on `bool 
> unused = !pointer;`? And if so it won't mean **constrained**.
> - Why do we only care about implicit pointer-to-bool conversion? What about 
> situations like `pointer == nullptr`, `NULL != pointer`, 
> `__builtin_expect(pointer, 0)`, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D99540: [clangd] Preserve diags between tweak enumeration and execution

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry I've lost my context - did we decide to move forward with this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99540

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

[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, charco.
leonardchan added a project: Sanitizers.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This allows for hwasan to be built targetting fuchsia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103543

Files:
  compiler-rt/cmake/config-ix.cmake


Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -711,7 +711,7 @@
 endif()
 
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND HWASAN_SUPPORTED_ARCH AND
-OS_NAME MATCHES "Linux|Android")
+OS_NAME MATCHES "Linux|Android|Fuchsia")
   set(COMPILER_RT_HAS_HWASAN TRUE)
 else()
   set(COMPILER_RT_HAS_HWASAN FALSE)


Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -711,7 +711,7 @@
 endif()
 
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND HWASAN_SUPPORTED_ARCH AND
-OS_NAME MATCHES "Linux|Android")
+OS_NAME MATCHES "Linux|Android|Fuchsia")
   set(COMPILER_RT_HAS_HWASAN TRUE)
 else()
   set(COMPILER_RT_HAS_HWASAN FALSE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, charco.
leonardchan added a project: Sanitizers.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This disables use of hwasan interceptors which we do not use on Fuchsia. This 
explicitly sets the macro for defining the hwasan versions of `new/delete`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103544

Files:
  compiler-rt/lib/hwasan/CMakeLists.txt


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -41,7 +41,11 @@
   )
 
 set(HWASAN_DEFINITIONS)
-append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 
HWASAN_DEFINITIONS)
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
 
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -41,7 +41,11 @@
   )
 
 set(HWASAN_DEFINITIONS)
-append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
 
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

[Reid Kleckner via Phabricator via cfe-commits]

rnk added subscribers: aeubanks, hans, rsmith.
rnk added a comment.

+@rsmith @hans @aeubanks

I think this would be an unfortunate code size and startup time regression for 
Itanium C++ inline variables. The existing code was written under the 
assumption that vague linkage (GVA_DiscardableODR) implies no initialization 
ordering, but I think you've found a valid counterexample.

> specifically when init_array is not used

Can you elaborate on why that is? Here's what I remember, and what I guess is 
happening. ELF has two initializer schemes: .init_array and .ctors. They are 
essentially equivalent, they are both arrays of function pointers, but one is 
called in reverse order and the other is called in forward order. The compiler 
knows which scheme is in use and it is controlled by -fuse-init-array.

The LLVM IR langref says that llvm.global_ctors are called in ascending 
priority order, and the order between initializers is not defined. That's not 
very helpful and often doesn't reflect reality. I wonder if we could do two 
things, perhaps separately:

1. emit llvm.global_ctors so that they are called in order of appearance in the 
IR array (is this not already true?)
2. define the order of initialization in LangRef

With the first change in place, we can be confident that so long as all 
includers of the `StaticsClass` in the test emit both initializers in the 
correct order, no matter which initializers prevail, they will be called in the 
correct order. Of course, this could break users relying on the existing 
ordering, nevermind that it is explicitly documented as undefined.

The second change is only a documentation change, but I would like to find a 
way to justify what LLVM does in GlobalOpt. GlobalOpt can effectively fold a 
dynamic initializer to constant memory in certain cases. That can only ever 
have the effect of making an initializer run earlier. As long as no 
initializers depend on observing uninitialized globals, that should be safe. 
The guarantee that we want to provide is that, for each initializer, all 
initializers prior to it in the global_ctors array will have run when it runs. 
Initializers that appear later may run earlier. Hopefully that covers both the 
usual way that .init_array sections are linked together and the way globalopt 
optimizes dynamic initialization.


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

https://reviews.llvm.org/D103495

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

[PATCH] D103547: Don't delete the module you're inspecting

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: steven_wu, arphaman.
beanz requested review of this revision.
Herald added a project: clang.

Prior to this patch when you used `clang -module-file-info` clang would delete 
the module on completion because the module was treated as an output file.

This fixes the issue so you don't need to invoke cc1 directly to get module 
file information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103547

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Modules/module_file_info.m


Index: clang/test/Modules/module_file_info.m
===
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -7,8 +7,8 @@
 // RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s 
--check-prefix=RAW
 
 // RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj 
-fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F 
%S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s 
--check-prefix=OBJ
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s 
--check-prefix=OBJ
 
 // RAW:   Module format: raw
 // OBJ:   Module format: obj
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||


Index: clang/test/Modules/module_file_info.m
===
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -7,8 +7,8 @@
 // RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s --check-prefix=RAW
 
 // RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefix=OBJ
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefix=OBJ
 
 // RAW:   Module format: raw
 // OBJ:   Module format: obj
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I don't know know why these fake FunctionDecls are needed, but it looks like 
it's okay to avoid creating them. I see a few debug info tests failing if 
`GlobalDecl()` instead of a fake function is passed to `StartFunction`, but it 
looks like the test check strings should be changed.

AFAICT `CodeGenFunction::GenerateCopyHelperFunction` has been creating these 
fake FunctionDecls from the beginning and subsequent patches didn't fix it:

https://github.com/llvm/llvm-project/commit/0c74327715af823930cb583490d315f64ef8de4e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()

It might be better to force the value of COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
to OFF instead.

But I'll leave the cmake details to Petr.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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

[PATCH] D103549: [POC] Put annotation strings into debuginfo.

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added a project: debug-info.
Herald added subscribers: dexonsmith, pengfei, JDevlieghere, hiraditya, 
kristof.beyls, mgorny.
yonghong-song requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This is a Proof-Of-Concept patch and intends to seek suggestions
from llvm community on how to put an attribute with arbitrary
string into the final debuginfo in the object file.

The Use Cases
=

In BPF ecosystem, BTF is the debuginfo used for validation
and additional information.

  https://www.kernel.org/doc/html/latest/bpf/btf.html

Currently, BTF in vmlinux (x86_64, aarch64, etc.) are
generated by using pahole to convert dwarf to BTF and
vmlinux BTF is used to validate bpf program compliance,
e.g., bpf program signature must match kernel function
signature for certain tracing programs. Beyond signature
checking, the following are use cases which will further
help verifier.

1. annotation of "user"/"rcu" etc for function arguments, structure fields and 
global/static variables. Kernel currently uses `address_space` attributes for 
`sparse` tool. But we could like to carry this information to debuginfo. 
Previous attempt https://reviews.llvm.org/D69393 tries to use `address_space` 
which is halted as it needs to touch a lot of other llvm places.
2. annotation of functions. Currently, kernel tries to group them with separate 
logic, e.g., foo() __attribute__("property1", "property2") since the above 
attribute is not supported, kernel has to do some magic like global 
btf_property1: btf type id for foo, ... global btf_property2: btf type id for 
foo, ... this is really error prone as the function definition may be under 
some configs and the `global btf_property1 ...` may not even in the same source 
file as the function. Such a disconnect between function definition and 
function attributes already caused numerous issues.

  We also want to annotate functions with certain pre-conditions (e.g., a 
socket lock has been hold), as bpf programs has started to call kernel 
functions. Such annotations should be really directly applied to the function 
definition to avoid any potential later mismatch issues.
3. annotation of structures, e.g., if somehow this structure fields may have 
been randomized, verifier should know it as it cannot trust debuginfo structure 
layout any more.

Sorry for tense explanation of use cases. The main takeaway
is we want to annotate structure/field/func/argument/variable
with *arbitrary* strings and want such strings to be preserved
in final dwarf (or BTF) output.

An Example
==

In this patch, I hacked clang Frontend to put annotations
in debuginfo and hacked llvm/CodeGen to "output" these
annotations into BTF. The target architecture is x86.
Note that I didn't really output these attributes to BTF yet.
I would like to seek llvm community advise first.

Below is an example to show what the source code looks like.
I am using "annotate" attribute as it accepts arbitrary strings.

  $ cat t1.c
  /* a pointer pointing to user memory */
  #define __user __attribute__((annotate("user")))
  /* a pointer protected by rcu */
  #define __rcu __attribute__((annotate("rcu")))
  /* the struct has some special property */
  #define __special_struct __attribute__((annotate("special_struct")))
  /* sock_lock is held for the function */
  #define __sock_lock_held __attribute((annotate("sock_lock_held")))
  /* the hash table element type is socket */
  #define __special_info __attribute__((annotate("elem_type:socket")))
  
  struct hlist_node;
  struct hlist_head {
struct hlist_node *prev;
struct hlist_node *next;
  } __special_struct;
  struct hlist {
 struct hlist_head head __special_info;
  };
  
  extern void bar(struct hlist *);
  int foo(struct hlist *h,  int *a __user, int *b __rcu) __sock_lock_held {
bar(h);
return *a + *b;
  }
  $ clang --target x86_64 -O2 -c -g t1.c
  TODO (BTF2Debug.cpp): Add func arg 'a' annotation 'user' to .BTF section
  TODO (BTF2Debug.cpp): Add func arg 'b' annotation 'rcu' to .BTF section
  TODO (BTF2Debug.cpp): Add subroutine 'foo' annotation 'sock_lock_held' to 
.BTF section
  TODO (BTF2Debug.cpp): Add field 'head' annotation 'elem_type:socket' to .BTF 
section
  TODO (BTF2Debug.cpp): Add struct 'hlist_head' annotation 'special_struct' to 
.BTF section
  $

What Is Next


First, using "annotate" attribute is not the best choice as I generated
extra globals and IRs. Maybe a different clang specific attribute?

Second, in the above example, I tried to put these attributes in BTF
as I researched and didn't find a way to put these attributes in dwarf.
Do we have a way to put it into dwarf? That works for us too.
Otherwise, we can let x86/arm64 etc. generates BTF (with a flag of course)
which will have these attribute information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103549

F