[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 253408.
f00kat added a comment.

lint fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76996

Files:
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/test/Analysis/placement-new.cpp

Index: clang/test/Analysis/placement-new.cpp
===
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -155,3 +155,109 @@
   (void)dp;
 }
 } // namespace testHierarchy
+
+namespace testConcretePointers {
+void f1() {
+// ok (200 % 8 == 0).
+::new ((size_t*)200) long;
+}
+void f2() {
+// bad (100 % 8 == 4).
+::new ((size_t*)100) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f3() {
+::new (reinterpret_cast(200)) long;
+}
+
+void f4() {
+::new (reinterpret_cast(100)) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f5() {
+::new ((size_t*)(200 + 0)) long;
+}
+
+void f6() {
+::new ((size_t*)(100 + 2)) long; // expected-warning{{Address is aligned to 6 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+} // namespace testConcretePointers
+
+namespace testArrayTypesAllocation {
+void f1() {
+struct S {
+short a;
+};
+
+// bad (not enough space).
+const unsigned N = 32;
+alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
+::new (buffer1) S[N]; // expected-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires more space for internal needs}} expected-note 1 {{}}
+}
+
+void f2() {
+struct S {
+short a;
+};
+
+// maybe ok but we need to warn.
+const unsigned N = 32;
+alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
+::new (buffer2) S[N]; // expected-warning{{Possibly not enough 68 bytes for array allocation which requires 64 bytes. Current overhead requires the size of 4 bytes}} expected-note 1 {{}}
+}
+} // namespace testArrayTypesAllocation
+
+namespace testStructAlign {
+void f1() {
+struct A {
+char a[9];
+} Ai; // expected-note {{'Ai' initialized here}}
+
+// bad (struct A is aligned to char).
+::new (&Ai.a) long;  // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f2() {
+struct B {
+char a;
+long b;
+} Bi;
+
+// ok (struct B is aligned to long).
+::new (&Bi.a) long;
+}
+
+void f3() {
+struct C {
+char a[8];
+alignas(2) char b;
+} Ci; // expected-note {{'Ci' initialized here}}
+
+// bad (struct C is aligned to 2).
+::new (&Ci.a) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4() {
+struct D {
+char a;
+char b;
+struct {
+long c;
+};
+} Di;
+
+// ok (struct D is aligned to long).
+::new (&Di.a) long;
+}
+
+void f5() {
+struct alignas(alignof(long)) E {
+char a;
+char b;
+} Ei;
+
+// ok (struct E is aligned to long).
+::new (&Ei.a) long;
+}
+} // namespace testStructAlign
+
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -25,22 +25,31 @@
   void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
 
 private:
+  bool checkPlaceCapacityIsSufficient(const CXXNewExpr *NE,
+  CheckerContext &C) const;
+
+  bool checkPlaceIsAlignedProperly(const CXXNewExpr *NE,
+   CheckerContext &C) const;
+
   // Returns the size of the target in a placement new expression.
   // E.g. in "new (&s) long" it returns the size of `long`.
-  SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, ProgramStateRef State,
-CheckerContext &C) const;
+  SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, CheckerContext &C,
+bool &IsArray) const;
   // Returns the size of the place in a placement new expression.
   // E.g. in "new (&s) long" it returns the size of `s`.
-  SVal getExtentSizeOfPlace(const Expr *NE, ProgramStateRef State,
-CheckerContext &C) const;
-  BugType BT{this, "Insufficient storage for placement new",
- categories::MemoryError};
+  SVal getExtentSizeOfPlace(const CXXNewExpr *NE, CheckerContext &C) const;
+  BugType SBT{this, "Insufficient storage for placement new",
+  categories::MemoryError};
+  BugTyp

[PATCH] D77010: [OpenMP] set_bits iterator yields unsigned elements, no reference (NFC).

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: jdoerfert, rnk.
Herald added subscribers: llvm-commits, guansong, hiraditya.
Herald added a project: LLVM.

BitVector::set_bits() returns an iterator range yielding unsinged
elements, which always will be copied while const & gives the impression
that there will be no copy. Newer version of clang complain:

  warning: loop variable 'SetBitsIt' is always a copy because the range of type 
'iterator_range' (aka 
'iterator_range >') does not 
return a reference [-Wrange-loop-analysis]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77010

Files:
  llvm/lib/Frontend/OpenMP/OMPContext.cpp


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -79,8 +79,8 @@
   LLVM_DEBUG({
 dbgs() << "[" << DEBUG_TYPE
<< "] New OpenMP context with the following properties:\n";
-for (const auto &SetBitsIt : ActiveTraits.set_bits()) {
-  TraitProperty Property = TraitProperty(SetBitsIt);
+for (unsigned Bit : ActiveTraits.set_bits()) {
+  TraitProperty Property = TraitProperty(Bit);
   dbgs() << "\t " << getOpenMPContextTraitPropertyFullName(Property)
  << "\n";
 }
@@ -127,8 +127,8 @@
   // relation is not required to be strict.
   if (VMI0.RequiredTraits.count() >= VMI1.RequiredTraits.count())
 return false;
-  for (const auto &SetBitsIt : VMI0.RequiredTraits.set_bits())
-if (!VMI1.RequiredTraits.test(SetBitsIt))
+  for (unsigned Bit : VMI0.RequiredTraits.set_bits())
+if (!VMI1.RequiredTraits.test(Bit))
   return false;
   if (!isSubset(VMI0.ConstructTraits, VMI1.ConstructTraits))
 return false;
@@ -139,8 +139,8 @@
 const VariantMatchInfo &VMI, const OMPContext &Ctx,
 SmallVectorImpl *ConstructMatches) {
 
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
 if (!IsActiveTrait) {
@@ -191,8 +191,8 @@
   APInt Score(64, 1);
 
   unsigned NoConstructTraits = VMI.ConstructTraits.size();
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 // If there is a user score attached, use it.
 if (VMI.ScoreMap.count(Property)) {
   const APInt &UserScore = VMI.ScoreMap.lookup(Property);


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -79,8 +79,8 @@
   LLVM_DEBUG({
 dbgs() << "[" << DEBUG_TYPE
<< "] New OpenMP context with the following properties:\n";
-for (const auto &SetBitsIt : ActiveTraits.set_bits()) {
-  TraitProperty Property = TraitProperty(SetBitsIt);
+for (unsigned Bit : ActiveTraits.set_bits()) {
+  TraitProperty Property = TraitProperty(Bit);
   dbgs() << "\t " << getOpenMPContextTraitPropertyFullName(Property)
  << "\n";
 }
@@ -127,8 +127,8 @@
   // relation is not required to be strict.
   if (VMI0.RequiredTraits.count() >= VMI1.RequiredTraits.count())
 return false;
-  for (const auto &SetBitsIt : VMI0.RequiredTraits.set_bits())
-if (!VMI1.RequiredTraits.test(SetBitsIt))
+  for (unsigned Bit : VMI0.RequiredTraits.set_bits())
+if (!VMI1.RequiredTraits.test(Bit))
   return false;
   if (!isSubset(VMI0.ConstructTraits, VMI1.ConstructTraits))
 return false;
@@ -139,8 +139,8 @@
 const VariantMatchInfo &VMI, const OMPContext &Ctx,
 SmallVectorImpl *ConstructMatches) {
 
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
 if (!IsActiveTrait) {
@@ -191,8 +191,8 @@
   APInt Score(64, 1);
 
   unsigned NoConstructTraits = VMI.ConstructTraits.size();
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 // If there is a user score attached, use it.
 if (VMI.ScoreMap.count(Property)) {
   const APInt &UserScore = VMI.ScoreMap.lookup(Property);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 253412.
ztamas added a comment.

Rebase, TODO comment, remove unrelated change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar &operator=(const Bar &other) {
+  if (this != &other) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by 
value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,14 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  // TODO: Solve this on the matcher level, so we don't need to use two 
different
+  // matchers for the same thing.
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar &operator=(const Bar &other) {
+  if (this != &other) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,14 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  // TODO: Solve this on the matcher level, so we don't need to use two different
+  // matchers for the same thing.
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

In D76990#1948286 , @njames93 wrote:

> I'm not entirely sure this is where the fix needs to be for this. The test 
> case code is whacky as hell, but from what I can see clang should always emit 
> a `BinaryOperator` for dependent type operators. No idea why it is spewing 
> out a `CXXOperatorCallExpr` instead. Would need someone with more knowledge 
> on Sema to confirm(or deny) this though.


I agree, it seems suspicious that a BinaryOperator matcher does not work in 
this case. However, I'm working on this level of the code, I'm looking at the 
matchers like an API, what I'm just using in the clang-tidy code, without 
changing them. So that's why I added this workaround. I don't know how much 
time it would take to fix this issue in the matcher code or whether it will be 
fixed in the near future or not, but until then I think it useful to have this 
workaround in the caller code, so this clang-tidy check works better. I added a 
TODO comment, so it's more visible that we have an issue here, so it's more 
probable somebody will fix that working with the matchers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990



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

[PATCH] D76594: [clang][AST] Support AST files larger than 512M

[Dmitry Polukhin via Phabricator via cfe-commits]

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

Rebase, all tests passes locally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1892,6 +1892,7 @@
   // Write out the source location entry table. We skip the first
   // entry, which is always the same dummy entry.
   std::vector SLocEntryOffsets;
+  uint64_t SLocEntryOffsetsBase = Stream.GetCurrentBitNo();
   RecordData PreloadSLocs;
   SLocEntryOffsets.reserve(SourceMgr.local_sloc_entry_size() - 1);
   for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size();
@@ -1902,7 +1903,9 @@
 assert(&SourceMgr.getSLocEntry(FID) == SLoc);
 
 // Record the offset of this source-location entry.
-SLocEntryOffsets.push_back(Stream.GetCurrentBitNo());
+uint64_t Offset = Stream.GetCurrentBitNo() - SLocEntryOffsetsBase;
+assert((Offset >> 32) == 0 && "SLocEntry offset too large");
+SLocEntryOffsets.push_back(Offset);
 
 // Figure out which record code to use.
 unsigned Code;
@@ -2010,12 +2013,14 @@
   Abbrev->Add(BitCodeAbbrevOp(SOURCE_LOCATION_OFFSETS));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 16)); // # of slocs
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 16)); // total size
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // base offset
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // offsets
   unsigned SLocOffsetsAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
   {
 RecordData::value_type Record[] = {
 SOURCE_LOCATION_OFFSETS, SLocEntryOffsets.size(),
-SourceMgr.getNextLocalOffset() - 1 /* skip dummy */};
+SourceMgr.getNextLocalOffset() - 1 /* skip dummy */,
+SLocEntryOffsetsBase};
 Stream.EmitRecordWithBlob(SLocOffsetsAbbrev, Record,
   bytes(SLocEntryOffsets));
   }
@@ -2092,9 +2097,11 @@
 /// Writes the block containing the serialized form of the
 /// preprocessor.
 void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
+  uint64_t MacroOffsetsBase = Stream.GetCurrentBitNo();
+
   PreprocessingRecord *PPRec = PP.getPreprocessingRecord();
   if (PPRec)
-WritePreprocessorDetail(*PPRec);
+WritePreprocessorDetail(*PPRec, MacroOffsetsBase);
 
   RecordData Record;
   RecordData ModuleMacroRecord;
@@ -2155,7 +2162,8 @@
   // identifier they belong to.
   for (const IdentifierInfo *Name : MacroIdentifiers) {
 MacroDirective *MD = PP.getLocalMacroDirectiveHistory(Name);
-auto StartOffset = Stream.GetCurrentBitNo();
+uint64_t StartOffset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
+assert((StartOffset >> 32) == 0 && "Macro identifiers offset too large");
 
 // Emit the macro directives in reverse source order.
 for (; MD; MD = MD->getPrevious()) {
@@ -2228,14 +2236,12 @@
 
 // Record the local offset of this macro.
 unsigned Index = ID - FirstMacroID;
-if (Index == MacroOffsets.size())
-  MacroOffsets.push_back(Stream.GetCurrentBitNo());
-else {
-  if (Index > MacroOffsets.size())
-MacroOffsets.resize(Index + 1);
+if (Index >= MacroOffsets.size())
+  MacroOffsets.resize(Index + 1);
 
-  MacroOffsets[Index] = Stream.GetCurrentBitNo();
-}
+uint64_t Offset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
+assert((Offset >> 32) == 0 && "Macro offset too large");
+MacroOffsets[Index] = Offset;
 
 AddIdentifierRef(Name, Record);
 AddSourceLocation(MI->getDefinitionLoc(), Record);
@@ -2286,17 +2292,20 @@
   Abbrev->Add(BitCodeAbbrevOp(MACRO_OFFSET));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # of macros
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // first ID
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32));   // base offset
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
 
   unsigned MacroOffsetAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
   {
 RecordData::value_type Record[] = {MACRO_OFFSET, MacroOffsets.size(),
-   FirstMacroID - NUM_PREDEF_MACRO_IDS};
+   FirstMacroID - NUM_PREDEF_MACRO_IDS,
+   MacroOffsetsBase};
 Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
 }
 
-void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec) {
+void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
+ 

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added reviewers: martong, NoQ.
Herald added subscribers: ASDenysPetrov, Charusso, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun.
Herald added a project: clang.

This check was causing a crash in a test case where the 0th argument was
uninitialized ('Assertion `T::isKind(*this)' at line SVals.h:104). This
was happening since the argument was actually undefined, but the castAs
assumes the value is DefinedOrUnknownSVal.

The fix appears to be simply to check for an undefined value and skip
the check allowing the uninitalized value checker to detect the error.

I included a test case that I verified to produce the negative case
prior to the fix, and passes with the fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77012

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions.c


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,16 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+
+void test_fread_uninitialized(void)
+{
+void *ptr;
+size_t sz;
+size_t nmem;
+FILE *fp;
+(void) fread(ptr, sz, nmem, fp); // expected-warning {{1st function call 
argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,16 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+
+void test_fread_uninitialized(void)
+{
+void *ptr;
+size_t sz;
+size_t nmem;
+FILE *fp;
+(void) fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77010: [OpenMP] set_bits iterator yields unsigned elements, no reference (NFC).

[Reid Kleckner via Phabricator via cfe-commits]

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

Lgtm, thanks, I was seeing this locally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77010



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

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 253419.
vabridgers added a comment.

fix pre-merge lint checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77012

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions.c


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void) fread(ptr, sz, nmem, fp); // expected-warning {{1st function call 
argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void) fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: Szelethus.
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Thanks!

@Szelethus can we make this checker depend on undefined value checker (probably 
CallAndMessage) so that uninitialized arguments were handled first? In fact, 
can we make a silent assumption that everything depends on `core`? If so we 
could eliminate all checks for undefined values in PreStmt and PreCall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77012



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

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

That was fast! Looks alright.




Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:25
 public:
   void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
 

Before i forget: Ideally @martong should have subscribed to [[ 
https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CheckerDocumentation.html#a7fdb3b5ff726f4c5e782cef0d59c01ad
 | `checkNewAllocator` ]] because it fires before the construct-expression 
whereas this callback fires after construct-expression which is too late as the 
UB we're trying to catch has occured much earlier.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:165-166
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const FieldRegion *TheFieldRegion = MRegion->getAs())
+  MRegion = TheFieldRegion->getBaseRegion();
+

You're saying that `A` is a struct and `a` is of type `A` and `&a` is 
sufficiently aligned then for //every// field `f` in the struct `&a.f` is 
sufficiently aligned. I'm not sure it's actually the case.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:196-197
+}
+  } else if (Optional TheConcreteInt =
+ PlaceVal.getAs()) {
+uint64_t PlaceAlign = *TheConcreteInt.getValue().getValue().getRawData();

I don't think you'll ever see this case in a real-world program. Even if you 
would, i doubt we'll behave as expected, because we have [[ 
https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0/clang/lib/StaticAnalyzer/Core/Store.cpp#L456
 | certain hacks ]] in place that mess up arithmetic on concrete pointers. I 
appreciate your thinking but i suggest removing this section for now as it'll 
probably cause more false positives than true positives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76996



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

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D76990#1948516 , @ztamas wrote:

> I agree, it seems suspicious that a BinaryOperator matcher does not work in 
> this case. However, I'm working on this level of the code, I'm looking at the 
> matchers like an API, what I'm just using in the clang-tidy code, without 
> changing them. So that's why I added this workaround. I don't know how much 
> time it would take to fix this issue in the matcher code or whether it will 
> be fixed in the near future or not, but until then I think it useful to have 
> this workaround in the caller code, so this clang-tidy check works better. I 
> added a TODO comment, so it's more visible that we have an issue here, so 
> it's more probable somebody will fix that working with the matchers.


It's not the matchers fault, that's working as intended. its the AST that clang 
has generated for that source code that looks suspicious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990



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

[PATCH] D77013: [AMDGPU] Add options -mamdgpu-ieee -mno-amdgpu-ieee

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: arsenm, b-sumner, rjmccall.
Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, nhaehnle, wdng, 
jvesely, kzhuravl.

AMDGPU backend need to know whether IEEE754-2008 NaN compliant instructions
need to be emitted for a function, which is conveyed by a function attribute
"amdgpu-ieee". "amdgpu-ieee"="false" turns this off. Without this function 
attribute
backend assumes it is on for compute functions.

-mamdgpu-ieee and -mno-amdgpu-ieee are added to Clang to control this function 
attribute.
By default it is on. If other options affecting NaN handling is set (e.g. 
-menable-no-nans,
-cl-fast-relaxed-math, etc.), it is on if IEEE NaN compliant handling is 
required, otherwise
it is off. If -mamdgpu-ieee or -mno-amdgpu-ieee is explicitly provided, the 
explicit value
takes precedence.


https://reviews.llvm.org/D77013

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenOpenCL/amdgpu-ieee.cl


Index: clang/test/CodeGenOpenCL/amdgpu-ieee.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/amdgpu-ieee.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   | FileCheck -check-prefixes=COM,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -mamdgpu-ieee | FileCheck -check-prefixes=COM,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -mno-amdgpu-ieee | FileCheck -check-prefixes=COM,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -menable-no-nans | FileCheck -check-prefixes=COM,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -menable-no-nans -mamdgpu-ieee | FileCheck -check-prefixes=COM,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -cl-fast-relaxed-math | FileCheck -check-prefixes=COM,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -cl-fast-relaxed-math -mamdgpu-ieee \
+// RUN: | FileCheck -check-prefixes=COM,ON %s
+
+kernel void kern() {
+// COM: define amdgpu_kernel void @kern() [[ATTRS:#[0-9]+]]
+}
+
+// ON-NOT: attributes [[ATTRS]] = {{.*}} "amdgpu-ieee"
+// OFF: attributes [[ATTRS]] = {{.*}} "amdgpu-ieee"="false"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1430,6 +1430,11 @@
   std::string(Args.getLastArgValue(OPT_fsymbol_partition_EQ));
 
   Opts.ForceAAPCSBitfieldLoad = Args.hasArg(OPT_ForceAAPCSBitfieldLoad);
+
+  Opts.EmitIEEENaNCompliantInsts =
+  Args.hasFlag(options::OPT_mamdgpu_ieee, options::OPT_mno_amdgpu_ieee,
+   !Opts.NoNaNsFPMath);
+
   return Success;
 }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8512,6 +8512,9 @@
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
+
+  if (!getABIInfo().getCodeGenOpts().EmitIEEENaNCompliantInsts)
+F->addFnAttr("amdgpu-ieee", "false");
 }
 
 unsigned AMDGPUTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2402,6 +2402,11 @@
   HelpText<"Generate additional code for specified  of debugger ABI 
(AMDGPU only)">,
   MetaVarName<"">;
 
+def mamdgpu_ieee : Flag<["-"], "mamdgpu-ieee">, Flags<[CC1Option]>,
+  Group, HelpText<"Enable IEEE754-2008 NaN compliance in supported 
AMDGPU instructions">;
+def mno_amdgpu_ieee : Flag<["-"], "mno-amdgpu-ieee">, Flags<[CC1Option]>,
+  Group;
+
 def mcode_object_v3 : Flag<["-"], "mcode-object-v3">, 
Group,
   HelpText<"Enable code object v3 (AMDGPU only)">;
 def mno_code_object_v3 : Flag<["-"], "mno-code-object-v3">, 
Group,
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -396,6 +396,9 @@
 /// Whether to not follow the AAPCS that enforce at least one read before 
storing to a volatile bitfield
 CODEGENOPT(ForceAAPCSBitfieldLoad, 1, 0)
 
+/// Whether to emit IEEE754-2008 NaN compliant instructions if available 
(AMDGPU Only)
+CODEGENOPT(EmitIEEENaNCompliantInsts, 1, 1)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT


Index: clang/test/CodeGenOpenCL/amdgpu-ieee.cl
===

[PATCH] D77010: [OpenMP] set_bits iterator yields unsigned elements, no reference (NFC).

[Florian Hahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG99913ef3d14f: [OpenMP] set_bits iterator yields unsigned 
elements, no reference (NFC). (authored by fhahn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77010

Files:
  llvm/lib/Frontend/OpenMP/OMPContext.cpp


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -79,8 +79,8 @@
   LLVM_DEBUG({
 dbgs() << "[" << DEBUG_TYPE
<< "] New OpenMP context with the following properties:\n";
-for (const auto &SetBitsIt : ActiveTraits.set_bits()) {
-  TraitProperty Property = TraitProperty(SetBitsIt);
+for (unsigned Bit : ActiveTraits.set_bits()) {
+  TraitProperty Property = TraitProperty(Bit);
   dbgs() << "\t " << getOpenMPContextTraitPropertyFullName(Property)
  << "\n";
 }
@@ -127,8 +127,8 @@
   // relation is not required to be strict.
   if (VMI0.RequiredTraits.count() >= VMI1.RequiredTraits.count())
 return false;
-  for (const auto &SetBitsIt : VMI0.RequiredTraits.set_bits())
-if (!VMI1.RequiredTraits.test(SetBitsIt))
+  for (unsigned Bit : VMI0.RequiredTraits.set_bits())
+if (!VMI1.RequiredTraits.test(Bit))
   return false;
   if (!isSubset(VMI0.ConstructTraits, VMI1.ConstructTraits))
 return false;
@@ -139,8 +139,8 @@
 const VariantMatchInfo &VMI, const OMPContext &Ctx,
 SmallVectorImpl *ConstructMatches) {
 
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
 if (!IsActiveTrait) {
@@ -191,8 +191,8 @@
   APInt Score(64, 1);
 
   unsigned NoConstructTraits = VMI.ConstructTraits.size();
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 // If there is a user score attached, use it.
 if (VMI.ScoreMap.count(Property)) {
   const APInt &UserScore = VMI.ScoreMap.lookup(Property);


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -79,8 +79,8 @@
   LLVM_DEBUG({
 dbgs() << "[" << DEBUG_TYPE
<< "] New OpenMP context with the following properties:\n";
-for (const auto &SetBitsIt : ActiveTraits.set_bits()) {
-  TraitProperty Property = TraitProperty(SetBitsIt);
+for (unsigned Bit : ActiveTraits.set_bits()) {
+  TraitProperty Property = TraitProperty(Bit);
   dbgs() << "\t " << getOpenMPContextTraitPropertyFullName(Property)
  << "\n";
 }
@@ -127,8 +127,8 @@
   // relation is not required to be strict.
   if (VMI0.RequiredTraits.count() >= VMI1.RequiredTraits.count())
 return false;
-  for (const auto &SetBitsIt : VMI0.RequiredTraits.set_bits())
-if (!VMI1.RequiredTraits.test(SetBitsIt))
+  for (unsigned Bit : VMI0.RequiredTraits.set_bits())
+if (!VMI1.RequiredTraits.test(Bit))
   return false;
   if (!isSubset(VMI0.ConstructTraits, VMI1.ConstructTraits))
 return false;
@@ -139,8 +139,8 @@
 const VariantMatchInfo &VMI, const OMPContext &Ctx,
 SmallVectorImpl *ConstructMatches) {
 
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
 if (!IsActiveTrait) {
@@ -191,8 +191,8 @@
   APInt Score(64, 1);
 
   unsigned NoConstructTraits = VMI.ConstructTraits.size();
-  for (const auto &SetBitsIt : VMI.RequiredTraits.set_bits()) {
-TraitProperty Property = TraitProperty(SetBitsIt);
+  for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
+TraitProperty Property = TraitProperty(Bit);
 // If there is a user score attached, use it.
 if (VMI.ScoreMap.count(Property)) {
   const APInt &UserScore = VMI.ScoreMap.lookup(Property);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 253434.
vabridgers added a comment.

fix pre-merge lint checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77012

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions.c


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call 
argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 253438.
ztamas added a comment.

Remove false TODO comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar &operator=(const Bar &other) {
+  if (this != &other) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by 
value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar &operator=(const Bar &other) {
+  if (this != &other) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77022: Use IgnoreImpCasts() instead of reimplementing it.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: dcoughlin.
Herald added subscribers: martong, Charusso.

No intended behavior change.


https://reviews.llvm.org/D77022

Files:
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -508,13 +508,7 @@
 /// return expressions of ObjC types when the return type of the function or
 /// method is non-null but the express is not.
 static const Expr *lookThroughImplicitCasts(const Expr *E) {
-  assert(E);
-
-  while (auto *ICE = dyn_cast(E)) {
-E = ICE->getSubExpr();
-  }
-
-  return E;
+  return E->IgnoreImpCasts();
 }
 
 /// This method check when nullable pointer or null value is returned from a


Index: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -508,13 +508,7 @@
 /// return expressions of ObjC types when the return type of the function or
 /// method is non-null but the express is not.
 static const Expr *lookThroughImplicitCasts(const Expr *E) {
-  assert(E);
-
-  while (auto *ICE = dyn_cast(E)) {
-E = ICE->getSubExpr();
-  }
-
-  return E;
+  return E->IgnoreImpCasts();
 }
 
 /// This method check when nullable pointer or null value is returned from a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512
+  return E->IgnoreImpCasts();
 }
 

I think it would make sense to remove the helper-function completely. (Being 
used 2 times.)


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

https://reviews.llvm.org/D77022



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

[clang] fe0723d - Fix -Wdocumentation warning. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-03-29T19:51:37+01:00
New Revision: fe0723dc9d45acfa4511961b208b7817b09297ec

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

LOG: Fix -Wdocumentation warning. NFC.

gcc was misinterpreting the template code snippet as html.

Added: 


Modified: 
clang/include/clang/AST/PrettyPrinter.h

Removed: 




diff  --git a/clang/include/clang/AST/PrettyPrinter.h 
b/clang/include/clang/AST/PrettyPrinter.h
index f42a6c0c1442..21e5ca94f6c4 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -184,7 +184,8 @@ struct PrintingPolicy {
   /// with zero parameters.
   unsigned UseVoidForZeroParams : 1;
 
-  /// Whether nested templates must be closed like a > rather than 
a>.
+  /// Whether nested templates must be closed like 'a >' rather than
+  /// 'a>'.
   unsigned SplitTemplateClosers : 1;
 
   /// Provide a 'terse' output.



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

[PATCH] D77023: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77023

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this 
function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the 
message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -836,8 +834,9 @@
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 
'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -124,7 +124,6 @@
 : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
   IsWarningAsError(IsWarningAsError) {}
 
-
 class ClangTidyContext::CachedGlobList {
 public:
   CachedGlobList(StringRef Globs) : Globs(Globs) {}
@@ -710,9 +709,9 @@
 const tooling::DiagnosticMessage &M1 = LHS.Message;
 const tooling::DiagnosticMessage &M2 = RHS.Message;
 
-return
-  std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, M1.Message) <
-  std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, M2.Message);
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);
   }
 };
 struct EqualClangTidyError {


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -836,8 +834,9 @@
   auto Index = buildIn

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 253446.
nridge added a comment.

Address last review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -272,7 +272,11 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
+  #define BAD2 BAD
+  double h = BAD2;  // NOLINT
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -310,14 +310,13 @@
   // Check for suppression comment. Skip the check for diagnostics not
   // in the main file, because we don't want that function to query the
   // source buffer for preamble files. For the same reason, we ask
-  // shouldSuppressDiagnostic not to follow macro expansions, since
-  // those might take us into a preamble file as well.
+  // shouldSuppressDiagnostic to avoid I/O.
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ /*AllowIO=*/false)) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -216,15 +216,12 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If `AllowIO` is false, the function does not attempt to read source files
+/// from disk which are not already mapped into memory; such files are treated
+/// as not containing a suppression comment.
 bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
   const Diagnostic &Info, ClangTidyContext &Context,
-  bool CheckMacroExpansion = true);
+  bool AllowIO = true);
 
 /// A diagnostic consumer that turns each \c Diagnostic into a
 /// \c SourceManager-independent \c ClangTidyError.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -296,57 +296,54 @@
   return true;
 }
 
+llvm::Optional getBuffer(const SourceManager &SM, FileID File,
+bool AllowIO) {
+  // This is similar to the implementation of SourceManager::getBufferData(),
+  // but uses ContentCache::getRawBuffer() rather than getBuffer() if
+  // AllowIO=false, to avoid triggering file I/O if the file contents aren't
+  // already mapped.
+  bool CharDataInvalid = false;
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(File, &CharDataInvalid);
+  if (CharDataInvalid || !Entry.isFile())
+return llvm::None;
+  const SrcMgr::ContentCache *Cache = Entry.getFile().getContentCache();
+  const llvm::MemoryBuffer *Buffer =
+  AllowIO ? Cache->getBuffer(SM.getDiagnostics(), SM.getFileManager(),
+ SourceLocation(), &CharDataInvalid)
+  : Cache->getRawBuffer();
+  if (!Buffer || CharDataInval

[clang-tools-extra] 15f1fe1 - clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-03-29T15:19:09-04:00
New Revision: 15f1fe1506f5e860409fddd8e62ed5508855ff79

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

LOG: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and 
DiagnosticsTets.cpp

Subscribers: jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 414da2857cff..10d5bdb3b3af 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -124,7 +124,6 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
 : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
   IsWarningAsError(IsWarningAsError) {}
 
-
 class ClangTidyContext::CachedGlobList {
 public:
   CachedGlobList(StringRef Globs) : Globs(Globs) {}
@@ -710,9 +709,9 @@ struct LessClangTidyError {
 const tooling::DiagnosticMessage &M1 = LHS.Message;
 const tooling::DiagnosticMessage &M2 = RHS.Message;
 
-return
-  std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, M1.Message) <
-  std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, M2.Message);
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);
   }
 };
 struct EqualClangTidyError {

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index eb4fdc98fb0b..d72f722921a8 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@ MATCHER_P3(Fix, Range, Replacement, Message,
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@ TEST(DiagnosticsTest, ClangTidy) {
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this 
function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the 
message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -836,8 +834,9 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 
'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {



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

[clang-tools-extra] b9d9968 - [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-03-29T15:19:13-04:00
New Revision: b9d9968f63ab8f24b300c69be11eadda3d405ac5

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

LOG: [clangd] Handle clang-tidy suppression comments for diagnostics inside 
macro expansions

Summary:
Not handling this was a side-effect of being overly cautious when trying
to avoid reading files for which clangd doesn't have the source mapped.

Fixes https://github.com/clangd/clangd/issues/266

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet,
usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 10d5bdb3b3af..d30552734679 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -296,57 +296,54 @@ static bool IsNOLINTFound(StringRef NolintDirectiveText, 
StringRef Line,
   return true;
 }
 
+llvm::Optional getBuffer(const SourceManager &SM, FileID File,
+bool AllowIO) {
+  // This is similar to the implementation of SourceManager::getBufferData(),
+  // but uses ContentCache::getRawBuffer() rather than getBuffer() if
+  // AllowIO=false, to avoid triggering file I/O if the file contents aren't
+  // already mapped.
+  bool CharDataInvalid = false;
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(File, &CharDataInvalid);
+  if (CharDataInvalid || !Entry.isFile())
+return llvm::None;
+  const SrcMgr::ContentCache *Cache = Entry.getFile().getContentCache();
+  const llvm::MemoryBuffer *Buffer =
+  AllowIO ? Cache->getBuffer(SM.getDiagnostics(), SM.getFileManager(),
+ SourceLocation(), &CharDataInvalid)
+  : Cache->getRawBuffer();
+  if (!Buffer || CharDataInvalid)
+return llvm::None;
+  return Buffer->getBuffer();
+}
+
 static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc,
unsigned DiagID,
-   const ClangTidyContext &Context) {
-  bool Invalid;
-  const char *CharacterData = SM.getCharacterData(Loc, &Invalid);
-  if (Invalid)
+   const ClangTidyContext &Context,
+   bool AllowIO) {
+  FileID File;
+  unsigned Offset;
+  std::tie(File, Offset) = SM.getDecomposedSpellingLoc(Loc);
+  llvm::Optional Buffer = getBuffer(SM, File, AllowIO);
+  if (!Buffer)
 return false;
 
   // Check if there's a NOLINT on this line.
-  const char *P = CharacterData;
-  while (*P != '\0' && *P != '\r' && *P != '\n')
-++P;
-  StringRef RestOfLine(CharacterData, P - CharacterData + 1);
+  StringRef RestOfLine = Buffer->substr(Offset).split('\n').first;
   if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context))
 return true;
 
   // Check if there's a NOLINTNEXTLINE on the previous line.
-  const char *BufBegin =
-  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), 
&Invalid);
-  if (Invalid || P == BufBegin)
-return false;
-
-  // Scan backwards over the current line.
-  P = CharacterData;
-  while (P != BufBegin && *P != '\n')
---P;
-
-  // If we reached the begin of the file there is no line before it.
-  if (P == BufBegin)
-return false;
-
-  // Skip over the newline.
-  --P;
-  const char *LineEnd = P;
-
-  // Now we're on the previous line. Skip to the beginning of it.
-  while (P != BufBegin && *P != '\n')
---P;
-
-  RestOfLine = StringRef(P, LineEnd - P + 1);
-  if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context))
-return true;
-
-  return false;
+  StringRef PrevLine =
+  Buffer->substr(0, Offset).rsplit('\n').first.rsplit('\n').second;
+  return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context);
 }
 
 static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
   SourceLocation Loc, unsigned DiagID,
-  const ClangTidyContext &Context) {
+  const ClangTidyContext &Context,
+  bool AllowIO) {
   while (true) {
-if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
+if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context, AllowIO))
   return true;
 if (!Loc.isMacroID())
   

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9d9968f63ab: [clangd] Handle clang-tidy suppression 
comments for diagnostics inside macro… (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -272,7 +272,11 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
+  #define BAD2 BAD
+  double h = BAD2;  // NOLINT
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -310,14 +310,13 @@
   // Check for suppression comment. Skip the check for diagnostics not
   // in the main file, because we don't want that function to query the
   // source buffer for preamble files. For the same reason, we ask
-  // shouldSuppressDiagnostic not to follow macro expansions, since
-  // those might take us into a preamble file as well.
+  // shouldSuppressDiagnostic to avoid I/O.
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ /*AllowIO=*/false)) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -216,15 +216,12 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If `AllowIO` is false, the function does not attempt to read source files
+/// from disk which are not already mapped into memory; such files are treated
+/// as not containing a suppression comment.
 bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
   const Diagnostic &Info, ClangTidyContext &Context,
-  bool CheckMacroExpansion = true);
+  bool AllowIO = true);
 
 /// A diagnostic consumer that turns each \c Diagnostic into a
 /// \c SourceManager-independent \c ClangTidyError.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -296,57 +296,54 @@
   return true;
 }
 
+llvm::Optional getBuffer(const SourceManager &SM, FileID File,
+bool AllowIO) {
+  // This is similar to the implementation of SourceManager::getBufferData(),
+  // but uses ContentCache::getRawBuffer() rather than getBuffer() if
+  // AllowIO=false, to avoid triggering file I/O if the file contents aren't
+  // already mapped.
+  bool CharDataInvalid = false;
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(File, &CharDataInvalid);
+  if (CharDataInvalid || !Entry.isFile())
+return llvm::None;
+  const SrcMgr::ContentCache *Cache = Entry.getFile().getContentCache();
+  const llvm::MemoryBuffer *Buffer =
+  AllowIO ? Cache->getBuffer(SM.getDiagnostics(), SM.getFileManager(),
+

[PATCH] D77023: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG15f1fe1506f5: clang-format fixes in 
ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77023

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this 
function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the 
message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -836,8 +834,9 @@
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 
'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -124,7 +124,6 @@
 : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
   IsWarningAsError(IsWarningAsError) {}
 
-
 class ClangTidyContext::CachedGlobList {
 public:
   CachedGlobList(StringRef Globs) : Globs(Globs) {}
@@ -710,9 +709,9 @@
 const tooling::DiagnosticMessage &M1 = LHS.Message;
 const tooling::DiagnosticMessage &M2 = RHS.Message;
 
-return
-  std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, M1.Message) <
-  std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, M2.Message);
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);
   }
 };
 struct EqualClangTidyError {


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the message.
+  WithFix(FixMessage(
+  "use a trailing retu

[clang] a5458bb - Don't claim template names that name non-templates are undeclared.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-29T13:15:30-07:00
New Revision: a5458bb0d6b1c35c7dcca4f339e77c40f5fc5e06

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

LOG: Don't claim template names that name non-templates are undeclared.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/pseudo-destructors.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b48f92f38939..cdc0d9688c08 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4916,7 +4916,8 @@ def note_using_value_decl_missing_typename : Note<
   "add 'typename' to treat this using declaration as a type">;
 
 def err_template_kw_refers_to_non_template : Error<
-  "%0 following the 'template' keyword does not refer to a template">;
+  "%0%select{| following the 'template' keyword}1 "
+  "does not refer to a template">;
 def note_template_kw_refers_to_non_template : Note<
   "declared as a non-template here">;
 def err_template_kw_refers_to_dependent_non_template : Error<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b4b2611481db..ecda3f4692a5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6999,6 +6999,27 @@ class Sema final {
bool AllowFunctionTemplates = true,
bool AllowDependent = true);
 
+  enum TemplateNameIsRequiredTag { TemplateNameIsRequired };
+  /// Whether and why a template name is required in this lookup.
+  class RequiredTemplateKind {
+  public:
+/// Template name is required if TemplateKWLoc is valid.
+RequiredTemplateKind(SourceLocation TemplateKWLoc = SourceLocation())
+: TemplateKW(TemplateKWLoc) {}
+/// Template name is unconditionally required.
+RequiredTemplateKind(TemplateNameIsRequiredTag) : TemplateKW() {}
+
+SourceLocation getTemplateKeywordLoc() const {
+  return TemplateKW.getValueOr(SourceLocation());
+}
+bool hasTemplateKeyword() const { return 
getTemplateKeywordLoc().isValid(); }
+bool isRequired() const { return TemplateKW != SourceLocation(); }
+explicit operator bool() const { return isRequired(); }
+
+  private:
+llvm::Optional TemplateKW;
+  };
+
   enum class AssumedTemplateKind {
 /// This is not assumed to be a template name.
 None,
@@ -7008,12 +7029,11 @@ class Sema final {
 /// functions (but no function templates).
 FoundFunctions,
   };
-  bool LookupTemplateName(LookupResult &R, Scope *S, CXXScopeSpec &SS,
-  QualType ObjectType, bool EnteringContext,
-  bool &MemberOfUnknownSpecialization,
-  SourceLocation TemplateKWLoc = SourceLocation(),
-  AssumedTemplateKind *ATK = nullptr,
-  bool Disambiguation = false);
+  bool LookupTemplateName(
+  LookupResult &R, Scope *S, CXXScopeSpec &SS, QualType ObjectType,
+  bool EnteringContext, bool &MemberOfUnknownSpecialization,
+  RequiredTemplateKind RequiredTemplate = SourceLocation(),
+  AssumedTemplateKind *ATK = nullptr, bool AllowTypoCorrection = true);
 
   TemplateNameKind isTemplateName(Scope *S,
   CXXScopeSpec &SS,

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1e2ebc5037bc..342262522e61 100755
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -205,7 +205,8 @@ TemplateNameKind Sema::isTemplateName(Scope *S,
   LookupResult R(*this, TName, Name.getBeginLoc(), LookupOrdinaryName);
   if (LookupTemplateName(R, S, SS, ObjectType, EnteringContext,
  MemberOfUnknownSpecialization, SourceLocation(),
- &AssumedTemplate, Disambiguation))
+ &AssumedTemplate,
+ /*AllowTypoCorrection=*/!Disambiguation))
 return TNK_Non_template;
 
   if (AssumedTemplate != AssumedTemplateKind::None) {
@@ -371,9 +372,9 @@ bool Sema::LookupTemplateName(LookupResult &Found,
   QualType ObjectType,
   bool EnteringContext,
   bool &MemberOfUnknownSpecialization,
-  SourceLocation TemplateKWLoc,
+  RequiredTemplateKind RequiredTemplate,
   AssumedTemplateKind *ATK,
-  bool Disambiguation) {
+  bool AllowTypoCorrection) {
   if (ATK

[clang] 6e0afb5 - [ARMMVE] Create fewer temporary SmallVectors

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-29T22:31:26+02:00
New Revision: 6e0afb5f108fe0570268978f4b32a0c8b4a43ee1

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

LOG: [ARMMVE] Create fewer temporary SmallVectors

Shrinks clang by 40k.

Added: 


Modified: 
clang/utils/TableGen/MveEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/MveEmitter.cpp 
b/clang/utils/TableGen/MveEmitter.cpp
index f53c6036766a..36aa7534aafa 100644
--- a/clang/utils/TableGen/MveEmitter.cpp
+++ b/clang/utils/TableGen/MveEmitter.cpp
@@ -739,7 +739,7 @@ class IRIntrinsicResult : public Result {
 "Intrinsic::ID", "Intrinsic::" + IntrinsicID);
 OS << "Builder.CreateCall(CGM.getIntrinsic(" << IntNo;
 if (!ParamTypes.empty()) {
-  OS << ", llvm::SmallVector {";
+  OS << ", {";
   const char *Sep = "";
   for (auto T : ParamTypes) {
 OS << Sep << ParamAlloc.allocParam("llvm::Type *", T->llvmName());
@@ -747,7 +747,7 @@ class IRIntrinsicResult : public Result {
   }
   OS << "}";
 }
-OS << "), llvm::SmallVector {";
+OS << "), {";
 const char *Sep = "";
 for (auto Arg : Args) {
   OS << Sep << Arg->asValue();



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

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Fair!




Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512
+  return E->IgnoreImpCasts();
 }
 

Charusso wrote:
> I think it would make sense to remove the helper-function completely. (Being 
> used 2 times.)
Yup.


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

https://reviews.llvm.org/D77022



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

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 253463.
atmnpatel added a comment.

Added error based on OpenMP version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591

Files:
  clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst
  clang-tools-extra/test/clang-tidy/checkers/openmp-use-default-none.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/driver.c
  clang/test/OpenMP/parallel_default_messages.cpp
  clang/test/OpenMP/parallel_for_default_messages.cpp
  clang/test/OpenMP/parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/parallel_master_default_messages.cpp
  clang/test/OpenMP/parallel_sections_default_messages.cpp
  clang/test/OpenMP/target_parallel_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/target_teams_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_default_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/task_default_messages.cpp
  clang/test/OpenMP/task_messages.cpp
  clang/test/OpenMP/teams_default_messages.cpp
  clang/test/OpenMP/teams_distribute_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_default_messages.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -381,6 +381,7 @@
 
 __OMP_DEFAULT_KIND(none)
 __OMP_DEFAULT_KIND(shared)
+__OMP_DEFAULT_KIND(firstprivate)
 __OMP_DEFAULT_KIND(unknown)
 
 #undef __OMP_DEFAULT_KIND
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1843,11 +1843,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(MatchFinderAPI, matchesDynamic) {
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2812,11 +2812,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(matchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isNoneKind) {
@@ -2852,10 +2859,17 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isSharedKind) {
@@ -2891,10 +2905,63 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+}
+
+TEST(OMPDefaultClause, isFirstPrivateKind) {
+  auto Matcher = ompExecutableDirective(
+  hasAnyClause(ompDefaultClause(isFirstPrivateKind(;
+
+  const

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 253470.
atmnpatel added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591

Files:
  clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst
  clang-tools-extra/test/clang-tidy/checkers/openmp-use-default-none.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/driver.c
  clang/test/OpenMP/parallel_default_messages.cpp
  clang/test/OpenMP/parallel_for_default_messages.cpp
  clang/test/OpenMP/parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/parallel_master_default_messages.cpp
  clang/test/OpenMP/parallel_sections_default_messages.cpp
  clang/test/OpenMP/target_parallel_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/target_teams_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_default_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/task_default_messages.cpp
  clang/test/OpenMP/task_messages.cpp
  clang/test/OpenMP/teams_default_messages.cpp
  clang/test/OpenMP/teams_distribute_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_default_messages.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -382,6 +382,7 @@
 
 __OMP_DEFAULT_KIND(none)
 __OMP_DEFAULT_KIND(shared)
+__OMP_DEFAULT_KIND(firstprivate)
 __OMP_DEFAULT_KIND(unknown)
 
 #undef __OMP_DEFAULT_KIND
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1843,11 +1843,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(MatchFinderAPI, matchesDynamic) {
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2805,11 +2805,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(matchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isNoneKind) {
@@ -2845,10 +2852,17 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isSharedKind) {
@@ -2884,10 +2898,63 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+}
+
+TEST(OMPDefaultClause, isFirstPrivateKind) {
+  auto Matcher = ompExecutableDirective(
+  hasAnyClause(ompDefaultClause(isFirstPrivateKind(;
+
+  const std::string Source0 = R"(
+

[clang] 9a7eda1 - PR45350: Handle unsized array CXXConstructExprs in constant evaluation

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-29T19:33:56-07:00
New Revision: 9a7eda1bece887ca9af085d79fe6e4fb8826dcda

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

LOG: PR45350: Handle unsized array CXXConstructExprs in constant evaluation
of array new expressions with runtime bound.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 06f4885e47d6..53e168a81bc3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8677,6 +8677,10 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
 static bool EvaluateArrayNewInitList(EvalInfo &Info, LValue &This,
  APValue &Result, const InitListExpr *ILE,
  QualType AllocType);
+static bool EvaluateArrayNewConstructExpr(EvalInfo &Info, LValue &This,
+  APValue &Result,
+  const CXXConstructExpr *CCE,
+  QualType AllocType);
 
 bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
   if (!Info.getLangOpts().CPlusPlus2a)
@@ -8726,6 +8730,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
 
   const Expr *Init = E->getInitializer();
   const InitListExpr *ResizedArrayILE = nullptr;
+  const CXXConstructExpr *ResizedArrayCCE = nullptr;
 
   QualType AllocType = E->getAllocatedType();
   if (Optional ArraySize = E->getArraySize()) {
@@ -8769,7 +8774,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
 //   -- the new-initializer is a braced-init-list and the number of
 //  array elements for which initializers are provided [...]
 //  exceeds the number of elements to initialize
-if (Init) {
+if (Init && !isa(Init)) {
   auto *CAT = Info.Ctx.getAsConstantArrayType(Init->getType());
   assert(CAT && "unexpected type for array initializer");
 
@@ -8792,6 +8797,8 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
   // special handling for this case when we initialize.
   if (InitBound != AllocBound)
 ResizedArrayILE = cast(Init);
+} else if (Init) {
+  ResizedArrayCCE = cast(Init);
 }
 
 AllocType = Info.Ctx.getConstantArrayType(AllocType, ArrayBound, nullptr,
@@ -8856,6 +8863,10 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
 if (!EvaluateArrayNewInitList(Info, Result, *Val, ResizedArrayILE,
   AllocType))
   return false;
+  } else if (ResizedArrayCCE) {
+if (!EvaluateArrayNewConstructExpr(Info, Result, *Val, ResizedArrayCCE,
+   AllocType))
+  return false;
   } else if (Init) {
 if (!EvaluateInPlace(*Val, Info, Result, Init))
   return false;
@@ -9683,6 +9694,16 @@ static bool EvaluateArrayNewInitList(EvalInfo &Info, 
LValue &This,
   .VisitInitListExpr(ILE, AllocType);
 }
 
+static bool EvaluateArrayNewConstructExpr(EvalInfo &Info, LValue &This,
+  APValue &Result,
+  const CXXConstructExpr *CCE,
+  QualType AllocType) {
+  assert(CCE->isRValue() && CCE->getType()->isArrayType() &&
+ "not an array rvalue");
+  return ArrayExprEvaluator(Info, This, Result)
+  .VisitCXXConstructExpr(CCE, This, &Result, AllocType);
+}
+
 // Return true iff the given array filler may depend on the element index.
 static bool MaybeElementDependentArrayFiller(const Expr *FillerExpr) {
   // For now, just whitelist non-class value-initialization and initialization

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 42d7300c5fde..f66f380b635f 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1381,3 +1381,23 @@ namespace PR45133 {
   constinit V v2 = X<2>();
   constinit V v3 = X<3>(); // expected-error {{constant init}} expected-note 
{{constinit}} expected-note {{in call}}
 }
+
+namespace PR45350 {
+  int q;
+  struct V { int n; int *p = &n; constexpr ~V() { *p = *p * 10 + n; }};
+  constexpr int f(int n) {
+int k = 0;
+V *p = new V[n];
+for (int i = 0; i != n; ++i) {
+  if (p[i].p != &p[i].n) return -1;
+  p[i].n = i;
+  p[i].p = &k;
+}
+delete[] p;
+return k;
+  }
+  // [expr.delete]p6:
+  //   In the case of an array, the elements will be destroyed in order of
+  //   decreasing address
+  static_assert(f(6) == 543210);
+}


 

[PATCH] D77028: [NFC] Refactor DeferredDiagsEmitter and skip redundant visit

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, ABataev.

Move function emitDeferredDiags from Sema to DeferredDiagsEmitter since it
is only used by DeferredDiagsEmitter.

Also record number of diagnostics triggered by each function and skip a function
if it is known not to emit any diagnostics.


https://reviews.llvm.org/D77028

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp

Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1436,48 +1436,24 @@
 }
 
 // Print notes showing how we can reach FD starting from an a priori
-// known-callable function.
-static void emitCallStackNotes(Sema &S, FunctionDecl *FD) {
+// known-callable function. If \DiagsCount is not null pointer, the number
+// of diagnostics emitted for a function is accumulated.
+static void emitCallStackNotes(Sema &S, FunctionDecl *FD,
+   llvm::DenseMap,
+  unsigned> *DiagsCount = nullptr) {
   auto FnIt = S.DeviceKnownEmittedFns.find(FD);
   while (FnIt != S.DeviceKnownEmittedFns.end()) {
 DiagnosticBuilder Builder(
 S.Diags.Report(FnIt->second.Loc, diag::note_called_by));
 Builder << FnIt->second.FD;
 Builder.setForceEmit();
+if (DiagsCount)
+  (*DiagsCount)[FnIt->second.FD]++;
 
 FnIt = S.DeviceKnownEmittedFns.find(FnIt->second.FD);
   }
 }
 
-// Emit any deferred diagnostics for FD and erase them from the map in which
-// they're stored.
-void Sema::emitDeferredDiags(FunctionDecl *FD, bool ShowCallStack) {
-  auto It = DeviceDeferredDiags.find(FD);
-  if (It == DeviceDeferredDiags.end())
-return;
-  bool HasWarningOrError = false;
-  bool FirstDiag = true;
-  for (PartialDiagnosticAt &PDAt : It->second) {
-const SourceLocation &Loc = PDAt.first;
-const PartialDiagnostic &PD = PDAt.second;
-HasWarningOrError |= getDiagnostics().getDiagnosticLevel(
- PD.getDiagID(), Loc) >= DiagnosticsEngine::Warning;
-{
-  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
-  Builder.setForceEmit();
-  PD.Emit(Builder);
-}
-
-// Emit the note on the first diagnostic in case too many diagnostics cause
-// the note not emitted.
-if (FirstDiag && HasWarningOrError && ShowCallStack) {
-  emitCallStackNotes(*this, FD);
-  FirstDiag = false;
-}
-  }
-
-}
-
 namespace {
 /// Helper class that emits deferred diagnostic messages if an entity directly
 /// or indirectly using the function that causes the deferred diagnostic
@@ -1488,6 +1464,10 @@
   typedef UsedDeclVisitor Inherited;
   llvm::SmallSet, 4> Visited;
   llvm::SmallVector, 4> UseStack;
+
+  // Deferred diagnostics triggered by a declaration.
+  llvm::DenseMap, unsigned> DiagsCount;
+
   bool ShouldEmit;
   unsigned InOMPDeviceContext;
 
@@ -1509,6 +1489,8 @@
 
   void visitUsedDecl(SourceLocation Loc, Decl *D) {
 if (auto *FD = dyn_cast(D)) {
+
+  auto DiagsCountIt = DiagsCount.find(FD);
   FunctionDecl *Caller = UseStack.empty() ? nullptr : UseStack.back();
   auto IsKnownEmitted = S.getEmissionStatus(FD, /*Final=*/true) ==
 Sema::FunctionEmissionStatus::Emitted;
@@ -1520,10 +1502,15 @@
   // Finalize analysis of OpenMP-specific constructs.
   if (Caller && S.LangOpts.OpenMP && UseStack.size() == 1)
 S.finalizeOpenMPDelayedAnalysis(Caller, FD, Loc);
+  else if (DiagsCountIt != DiagsCount.end() && DiagsCountIt->second == 0 &&
+   !InOMPDeviceContext)
+return;
+  if (DiagsCountIt == DiagsCount.end())
+DiagsCount[FD] = 0;
   if (Caller)
 S.DeviceKnownEmittedFns[FD] = {Caller, Loc};
   if (ShouldEmit || InOMPDeviceContext)
-S.emitDeferredDiags(FD, Caller);
+emitDeferredDiags(FD, Caller);
   Visited.insert(D);
   UseStack.push_back(FD);
   if (auto *S = FD->getBody()) {
@@ -1551,6 +1538,35 @@
 } else
   Inherited::visitUsedDecl(Loc, D);
   }
+
+  // Emit any deferred diagnostics for FD
+  void emitDeferredDiags(FunctionDecl *FD, bool ShowCallStack) {
+auto It = S.DeviceDeferredDiags.find(FD);
+if (It == S.DeviceDeferredDiags.end())
+  return;
+bool HasWarningOrError = false;
+bool FirstDiag = true;
+for (PartialDiagnosticAt &PDAt : It->second) {
+  const SourceLocation &Loc = PDAt.first;
+  const PartialDiagnostic &PD = PDAt.second;
+  HasWarningOrError |=
+  S.getDiagnostics().getDiagnosticLevel(PD.getDiagID(), Loc) >=
+  DiagnosticsEngine::Warning;
+  {
+DiagnosticBuilder Builder(S.Diags.Report(Loc, PD.getDiagID()));
+Builder.setForceEmit();
+PD.Emit(Builder);
+  }
+
+  DiagsCount[FD]++;
+  // Emit the note on the first diagnostic in case too many diagnostics
+  // cause the note not emitted.
+  if (FirstDiag && HasWarning

[PATCH] D76211: OpenMP Metadirective with user defined condition

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Apologies for my delay.

In D76211#1940445 , @alokmishra.besu 
wrote:

> Updated the test files


It seems there is only an AST dump test case. Where is the codegen tests (and 
others)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76211



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

[PATCH] D70555: [coroutines] Don't build promise init with no args

[JunMa via Phabricator via cfe-commits]

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

I'm not sure whether I can approve this.  If not, @modocache  please ignore 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70555



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

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 253498.
atmnpatel added a comment.

Second try at generating a patch file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591

Files:
  clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst
  clang-tools-extra/test/clang-tidy/checkers/openmp-use-default-none.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/driver.c
  clang/test/OpenMP/parallel_default_messages.cpp
  clang/test/OpenMP/parallel_for_default_messages.cpp
  clang/test/OpenMP/parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/parallel_master_default_messages.cpp
  clang/test/OpenMP/parallel_sections_default_messages.cpp
  clang/test/OpenMP/target_parallel_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/target_teams_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_default_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/task_default_messages.cpp
  clang/test/OpenMP/task_messages.cpp
  clang/test/OpenMP/teams_default_messages.cpp
  clang/test/OpenMP/teams_distribute_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_default_messages.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -430,6 +430,7 @@
 
 __OMP_DEFAULT_KIND(none)
 __OMP_DEFAULT_KIND(shared)
+__OMP_DEFAULT_KIND(firstprivate)
 __OMP_DEFAULT_KIND(unknown)
 
 #undef __OMP_DEFAULT_KIND
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1843,11 +1843,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(MatchFinderAPI, matchesDynamic) {
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2805,11 +2805,18 @@
   EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
 
   const std::string Source4 = R"(
+void x() {
+#pragma omp parallel default(firstprivate)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+  EXPECT_TRUE(matchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isNoneKind) {
@@ -2845,10 +2852,17 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
 }
 
 TEST(OMPDefaultClause, isSharedKind) {
@@ -2884,10 +2898,63 @@
 
   const std::string Source4 = R"(
 void x(int x) {
-#pragma omp parallel num_threads(x)
+#pragma omp parallel default(firstprivate)
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(
+void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+}
+
+TEST(OMPDefaultClause, isFirstPrivateKind) {
+  auto Matcher = ompExecutableDirective(
+  hasAnyClause(ompDefaultClause(isFirstPrivateKind(;
+
+  con