[PATCH] D71963: clang-tidy doc: Add the severities description

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal added a comment.

It's very hard to write these kinds of definitions without ambiguity and plenty 
of subjective interpretation creeping in. I'll try my best to provide 
constructive feedback but I'm admittedly struggling a bit with providing 
helpful counter proposals.
Ideally these levels would be based on a data driven approach, like when we say 
that something is error prone we should be able to support that with data.

I think the high severity is well defined it's distinctly different from the 
other levels from a qualitative perspective.
I would also like to suggest the addition that any check reporting security 
vulnerabilities should be classified as high severity.
One way to make all levels qualitatively different would be to use something 
like the following definitions:

- Medium:  things that aren't direct a error but are error prone somehow 
(ideally there would be data to support the claim that this issue often cause 
errors)
- Low: things that are not direct errors neither, prone to indirectly cause 
errors, but which still cause quality issues like unnecessarily low performance.
- Style: things that are handled by clang-format rather than clang-tidy.

Given how hard it is to write these kinds of definitions clearly I'm not sure 
it is a good idea to introduce severity for clang tidy checks.
Unless we can find a very strong definitions for distinctly different levels, 
supported with actual data, it might be better to just not do it.
Also I think there is already a difference in severity level indicated by 
whether the check is part of clang-format, clang-tidy or clang-diagnostics
and by defining these severity levels we need to make sure they don't conflict 
in any way with these already implied severity levels.

In short: My first suggestion is to just remove severity from the table instead 
of trying to improve the definitions.
If there is a strong preference towards keeping them I suggest making them more 
qualitatively different as described above.




Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:414
+
+- **STYLE**: A true positive indicates that the source code is against a 
specific coding guideline or could improve readability.
+  Example: LLVM Coding Guideline: Do not use else or else if after something 
that interrupts control flow (break, return, throw, continue).

It seems like there is a bit of overlap between STYLE and LOW. They both 
mention readability.
Might I suggest that style could be only issues related to naming convention 
and text formatting, like indentation, line breaks -like things that could be 
clang format rules.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:418
+- **LOW**: A true positive indicates that the source code is hard to 
read/understand or could be easily optimized.
+  Example: Unused variables, Dead code.
+

To me it's not obvious why these examples aren't medium severity. They seem to 
fit the description of medium severity, they are also very similar to the 
example in medium severity.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:423
+
+- **HIGH**: A true positive indicates that the source code will cause a 
run-time error.
+  Example of this category: out of bounds array access, division by zero, 
memory leak.

Does something need to always result in division by zero to be of high severity 
or is it enough that it introduces the possibility for division by zero to 
occur?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963



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

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

[Kim Viggedal via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:29
+  unless(anyOf(
+  isConstexpr(), hasDeclContext(namespaceDecl(isAnonymous())),
+  hasType(isConstQualified()),

aaron.ballman wrote:
> vingeldal wrote:
> > aaron.ballman wrote:
> > > vingeldal wrote:
> > > > aaron.ballman wrote:
> > > > > Why are you filtering out anonymous namespaces?
> > > > If it's in an anonymous namespace it's no longer globally accessible, 
> > > > it's only accessible within the file it is declared.
> > > It is, however, at namespace scope which is what the C++ Core Guideline 
> > > suggests to diagnose. Do you have evidence that this is the 
> > > interpretation the guideline authors were looking for (perhaps they would 
> > > like to update their suggested guidance for diagnosing)?
> > > 
> > > There are two dependency issues to global variables -- one is surprising 
> > > linkage interactions (where the extern nature of the symbol is an issue 
> > > across module boundaries) and the other is surprising name lookup 
> > > behavior within the TU (where the global nature of the symbol is an 
> > > issue). e.g.,
> > > ```
> > > namespace {
> > > int ik;
> > > }
> > > 
> > > void f() {
> > >   for (int ij = 0; ik < 10; ij++) { // Oops, typo, but still compiles
> > >   }
> > > }
> > > ```
> > > That's why I think the guideline purposefully does not exclude things 
> > > like anonymous namespaces.
> > I don't have any evidence. To me the guideline still looks a bit 
> > draft-like, so I just tried my best guess as to the intent of the guideline.
> > In reading the guideline I get the impression that the intent is to avoid 
> > globally accessible data which is mutable,
> > mainly for two reason:
> >  * It's hard to reason about code where you are dependent upon state which 
> > can be changed from anywhere in the code.
> >  * There is a risk of race conditions with this kind of data.
> > 
> > Keeping the variable in an anonymous namespace seems to deals with the 
> > issues which I interpret to be the focus of this guideline.
> > Consider that the guideline is part of the interface section. If the 
> > variable is declared in an anonymous namespace there is no risk that a user 
> > of some service interface adds a dependency to that variable, since the 
> > variable will be a hidden part of the provider implementation.
> > 
> > Admittedly the wording doesn't mention an exception for anonymous 
> > namespaces, and the sentence you have referenced seems to suggest that any 
> > non-const variable in namespace scope should be matched.
> > I'm guessing though, that was intended to clarify that a variable is still 
> > global even if in a (named) namespace, because that would not have been an 
> > obvious interpretation otherwise.
> > Without the referenced sentence one might interpret only variables outside 
> > of namespace scope as global.
> > Arguably a variable in namespace scope isn't globally declared, though it 
> > is globally accessible, via the namespace name. I think the global 
> > accessibility is the reason for dragging in variables from namespace scope 
> > and if that is the case then we shouldn't also need to worry about 
> > anonymous namespaces.
> > This should probably be clarified in the actual guideline.
> > This should probably be clarified in the actual guideline.
> 
> This sort of thing comes up with coding guidelines from time to time and the 
> way we usually handle it is to ask the guideline authors for guidance. If 
> they come back with an answer, then we go that route (while the authors fix 
> the text) and if they don't come back with an answer, we muddle our way 
> through. Would you mind filing an issue with the C++ Core Guideline folks to 
> see if they can weigh in on the topic? If they don't respond in a timely 
> fashion, I think it would make more sense to err on the side of caution and 
> diagnose declarations within anonymous namespaces. This matches the current 
> text from the core guideline, and we can always relax the rule if we find it 
> causes a lot of heartache in the wild. (If you have data about how often this 
> particular aspect of the check triggers on large real-world code bases, that 
> could help us make a decision too.)
I sent a pull request to the guide lines, suggesting a clarification. If that 
is addressed in the near future I guess we can go on what they say about the 
pull request. If it takes to long I'll just modify the code to warn in 
anonymous namespace as well.
https://github.com/isocpp/CppCoreGuidelines/pull/1553


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70265



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Michael via Phabricator via cfe-commits]

Alundra added a comment.

Please review and consider a merge of this change which looks to not have news 
since long time.
It's useful for a lot of people and it's awesome that there is a ready or 
almost ready patchset about it.
Thanks a lot!


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Taw via Phabricator via cfe-commits]

tawmoto added a comment.

In D44609#1798209 , @Alundra wrote:

> Please review and consider a merge of this change which looks to not have 
> news since long time.
>  It's useful for a lot of people and it's awesome that there is a ready or 
> almost ready patchset about it.
>  Thanks a lot!


+1, I need this functionality also


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

https://reviews.llvm.org/D44609



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

[PATCH] D71963: clang-tidy doc: Add the severities description

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

I do agree that they are subjective and not perfect.

However, I found the classification extremely useful when you look at the 
results on big projects.
I have been using codechecker (where the severities are coming from) for 
Firefox and its has been extremely useful to evaluate the importance of the 
checkers.

> For instance, the CERT rules all come with a severity specified by the rule 
> itself

Did you see some difference?

> it if each coding standard has drastically different ideas about severity

Do you have some examples of this occurrence?

thanks for the feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963



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

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

In D57662#1797607 , @derek wrote:

> @zinovy.nis @alexfh @JonasToth @MyDeveloperDay
>
> I'd be happy to provide a patch.


It would be nice, thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57662



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

[clang-tools-extra] 544f200 - Fix newline handling in clang-query parser

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-29T14:58:56Z
New Revision: 544f200c785f0314949ba3b8d1c51f65bf8d7761

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

LOG: Fix newline handling in clang-query parser

Don't prematurely remove characters from the end of the string

Added: 


Modified: 
clang-tools-extra/clang-query/QueryParser.cpp
clang-tools-extra/unittests/clang-query/QueryParserTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/QueryParser.cpp 
b/clang-tools-extra/clang-query/QueryParser.cpp
index a980722de9e6..896145bf961a 100644
--- a/clang-tools-extra/clang-query/QueryParser.cpp
+++ b/clang-tools-extra/clang-query/QueryParser.cpp
@@ -250,7 +250,7 @@ QueryRef QueryParser::doParse() {
   return completeMatcherExpression();
 
 Diagnostics Diag;
-auto MatcherSource = Line.trim();
+auto MatcherSource = Line.ltrim();
 auto OrigMatcherSource = MatcherSource;
 Optional Matcher = Parser::parseMatcherExpression(
 MatcherSource, nullptr, &QS.NamedValues, &Diag);

diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp 
b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
index 79fcfcae6e0d..fa16c7172bac 100644
--- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
+++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
@@ -348,4 +348,12 @@ match callExpr
 
   ASSERT_TRUE(isa(Q));
   EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+
+  Q = parse("\nm parmVarDecl()\nlet someMatcher\n");
+
+  ASSERT_TRUE(isa(Q));
+  Q = parse(Q->RemainingContent);
+
+  ASSERT_TRUE(isa(Q));
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
 }



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

[clang-tools-extra] dc93540 - Fix handling of newlines in clang-query

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-29T14:58:56Z
New Revision: dc93540acbf047cf54052568d2826d1a06df025e

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

LOG: Fix handling of newlines in clang-query

Replace assert with diagnostic for missing newline.

Added: 


Modified: 
clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
clang/lib/ASTMatchers/Dynamic/Parser.cpp

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp 
b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
index 4725789f29f2..79fcfcae6e0d 100644
--- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
+++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
@@ -330,4 +330,22 @@ match callExpr
   EXPECT_EQ("1:9: Error parsing matcher. Found token  "
 "while looking for '('.",
 cast(Q)->ErrStr);
+
+  Q = parse("let someMatcher\nm parmVarDecl()");
+
+  ASSERT_TRUE(isa(Q));
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+
+  Q = parse("\nm parmVarDecl()\nlet someMatcher\nm parmVarDecl()");
+
+  ASSERT_TRUE(isa(Q));
+  Q = parse(Q->RemainingContent);
+
+  ASSERT_TRUE(isa(Q));
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+
+  Q = parse("\nlet someMatcher\n");
+
+  ASSERT_TRUE(isa(Q));
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
 }

diff  --git a/clang/lib/ASTMatchers/Dynamic/Parser.cpp 
b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
index caa3a3bd0953..ef209d1274af 100644
--- a/clang/lib/ASTMatchers/Dynamic/Parser.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
@@ -607,15 +607,13 @@ bool Parser::parseExpressionImpl(VariantValue *Value) {
 // This error was already reported by the tokenizer.
 return false;
   case TokenInfo::TK_NewLine:
-llvm_unreachable("Newline should never be found here");
-return false;
   case TokenInfo::TK_OpenParen:
   case TokenInfo::TK_CloseParen:
   case TokenInfo::TK_Comma:
   case TokenInfo::TK_Period:
   case TokenInfo::TK_InvalidChar:
 const TokenInfo Token = Tokenizer->consumeNextToken();
-Error->addError(Token.Range, Error->ET_ParserInvalidToken) << Token.Text;
+Error->addError(Token.Range, Error->ET_ParserInvalidToken) << (Token.Kind 
== TokenInfo::TK_NewLine ? "NewLine" : Token.Text);
 return false;
   }
 



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

Re: [clang-tools-extra] dc93540 - Fix handling of newlines in clang-query

[Nico Weber via cfe-commits]

On Sun, Dec 29, 2019 at 6:59 AM Stephen Kelly via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Stephen Kelly
> Date: 2019-12-29T14:58:56Z
> New Revision: dc93540acbf047cf54052568d2826d1a06df025e
>
> URL:
> https://github.com/llvm/llvm-project/commit/dc93540acbf047cf54052568d2826d1a06df025e
> DIFF:
> https://github.com/llvm/llvm-project/commit/dc93540acbf047cf54052568d2826d1a06df025e.diff
>
> LOG: Fix handling of newlines in clang-query
>
> Replace assert with diagnostic for missing newline.
>
> Added:
>
>
> Modified:
> clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> clang/lib/ASTMatchers/Dynamic/Parser.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> index 4725789f29f2..79fcfcae6e0d 100644
> --- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> +++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> @@ -330,4 +330,22 @@ match callExpr
>EXPECT_EQ("1:9: Error parsing matcher. Found token  "
>  "while looking for '('.",
>  cast(Q)->ErrStr);
> +
> +  Q = parse("let someMatcher\nm parmVarDecl()");
> +
> +  ASSERT_TRUE(isa(Q));
> +  EXPECT_EQ("1:1: Invalid token  found when looking for a
> value.", cast(Q)->ErrStr);
> +
> +  Q = parse("\nm parmVarDecl()\nlet someMatcher\nm parmVarDecl()");
> +
> +  ASSERT_TRUE(isa(Q));
> +  Q = parse(Q->RemainingContent);
> +
> +  ASSERT_TRUE(isa(Q));
> +  EXPECT_EQ("1:1: Invalid token  found when looking for a
> value.", cast(Q)->ErrStr);
> +
> +  Q = parse("\nlet someMatcher\n");
> +
> +  ASSERT_TRUE(isa(Q));
> +  EXPECT_EQ("1:1: Invalid token  found when looking for a
> value.", cast(Q)->ErrStr);
>  }
>
> diff  --git a/clang/lib/ASTMatchers/Dynamic/Parser.cpp
> b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
> index caa3a3bd0953..ef209d1274af 100644
> --- a/clang/lib/ASTMatchers/Dynamic/Parser.cpp
> +++ b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
> @@ -607,15 +607,13 @@ bool Parser::parseExpressionImpl(VariantValue
> *Value) {
>  // This error was already reported by the tokenizer.
>  return false;
>case TokenInfo::TK_NewLine:
> -llvm_unreachable("Newline should never be found here");
> -return false;
>case TokenInfo::TK_OpenParen:
>case TokenInfo::TK_CloseParen:
>case TokenInfo::TK_Comma:
>case TokenInfo::TK_Period:
>case TokenInfo::TK_InvalidChar:
>  const TokenInfo Token = Tokenizer->consumeNextToken();
> -Error->addError(Token.Range, Error->ET_ParserInvalidToken) <<
> Token.Text;
> +Error->addError(Token.Range, Error->ET_ParserInvalidToken) <<
> (Token.Kind == TokenInfo::TK_NewLine ? "NewLine" : Token.Text);
>

Please clang-format your local diff before committing.


>  return false;
>}
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71969: [OpenMP] diagnose zero-length array section in the depend clause

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I would put this check right next to the one that issues 
`err_omp_section_length_negative`. SemaExpr.cpp +4668


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

https://reviews.llvm.org/D71969



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze created this revision.
gbencze added reviewers: aaron.ballman, alexfh, JonasToth, Charusso.
gbencze added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, xazax.hun, whisperity, mgorny.

The check warns on suspicious calls to memcmp.
It currently checks for comparing padding and non-standard-layout types.
Based on
https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data
and part of
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions
Add alias cert-exp42-c.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71973

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,352 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+
+typedef unsigned long long size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+namespace std {
+int memcmp(const void *lhs, const void *rhs, size_t count);
+}
+
+namespace sei_cert_example_exp42_c {
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+
+void noncompliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: comparing padding bytes
+  }
+}
+
+void compliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (left->c == right->c) && (left->i == right->i) &&
+  (0 == memcmp(left->buffer, right->buffer, 13))) {
+  }
+}
+} // namespace sei_cert_example_exp42_c
+
+namespace sei_cert_example_exp42_c_ex1 {
+#pragma pack(push, 1)
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+#pragma pack(pop)
+
+void compare(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// no-warning
+  }
+}
+} // namespace sei_cert_example_exp42_c_ex1
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+  int i;
+
+public:
+  virtual void f();
+};
+
+void f(C &c1, C &c2) {
+  if (!std::memcmp(&c1, &c2, sizeof(C))) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+// of non-standard-layout type 'sei_cert_example_oop57_cpp::C'; consider
+// using a comparison operator instead
+  }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace trailing_padding {
+struct S {
+  int i;
+  char c;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(int));
+  memcmp(&a, &b, sizeof(int) + sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace trailing_padding
+
+namespace inner_padding {
+struct S {
+  char c;
+  int i;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(char) + sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace inner_padding
+
+namespace bitfield {
+struct S {
+  int x : 10;
+  int y : 6;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, 2); // no-warning: no padding in first 2 bytes
+}
+} // namespace bitfield
+
+namespace bitfield_2 {
+struct S {
+  int x : 10;
+  int y : 7;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, 2);
+  memcmp(&a, &b, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace bitfield_2
+
+namespace bitfield_3 {
+struct S {
+  int x : 2;
+  int : 0;
+  int y : 6;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace bitfield_3
+
+namespace array_no_padding {
+struct S {
+

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 235509.
tejohnson added a comment.

Implement suggested name change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71911

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Assembler/thinlto-vtable-summary.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll

Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -48,8 +48,11 @@
   ret i32 %call1
 }
 
+!llvm.module.flags = !{!4}
+
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFivE.virtual"}
 !2 = !{i64 24, !"_ZTSM1AFivE.virtual"}
 !3 = !{i64 2}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
 !9 = !{}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
@@ -1,3 +1,5 @@
+; Tests that VFE is not performed when the Virtual Function Elim metadata set
+; to 0. This is the same as virtual-functions.ll otherwise.
 ; RUN: opt < %s -globaldce -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -11,14 +13,12 @@
 ; intrinsic. Function test_A makes a call to A::foo, but there is no call to
 ; A::bar anywhere, so A::bar can be deleted, and its vtable slot replaced with
 ; null.
+; However, with the metadata set to 0 we should not perform this VFE.
 
 %struct.A = type { i32 (...)** }
 
-; The pointer to A::bar in the vtable can be removed, because it will never be
-; loaded. We replace it with null to keep the layout the same. Because it is at
-; the 

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 235510.
tejohnson added a comment.

Attempt to fix patch to not include parent revision changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71911

Files:
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Assembler/thinlto-vtable-summary.ll

Index: llvm/test/Assembler/thinlto-vtable-summary.ll
===
--- llvm/test/Assembler/thinlto-vtable-summary.ll
+++ llvm/test/Assembler/thinlto-vtable-summary.ll
@@ -29,9 +29,9 @@
 
 ^0 = module: (path: "", hash: (0, 0, 0, 0, 0))
 ^1 = gv: (name: "_ZN1A1nEi") ; guid = 1621563287929432257
-^2 = gv: (name: "_ZTV1B", summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0), vTableFuncs: ((virtFunc: ^3, offset: 16), (virtFunc: ^1, offset: 24)), refs: (^3, ^1 ; guid = 5283576821522790367
+^2 = gv: (name: "_ZTV1B", summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, vcall_visibility: 0), vTableFuncs: ((virtFunc: ^3, offset: 16), (virtFunc: ^1, offset: 24)), refs: (^3, ^1 ; guid = 5283576821522790367
 ^3 = gv: (name: "_ZN1B1fEi") ; guid = 7162046368816414394
-^4 = gv: (name: "_ZTV1C", summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0), vTableFuncs: ((virtFunc: ^5, offset: 16), (virtFunc: ^1, offset: 24)), refs: (^1, ^5 ; guid = 1362402378846296
+^4 = gv: (name: "_ZTV1C", summaries: (variable: (module: ^0, flags: (linkage: external, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, vcall_visibility: 0), vTableFuncs: ((virtFunc: ^5, offset: 16), (virtFunc: ^1, offset: 24)), refs: (^1, ^5 ; guid = 1362402378846296
 ^5 = gv: (name: "_ZN1C1fEi") ; guid = 14876272565662207556
 ^6 = typeidCompatibleVTable: (name: "_ZTS1A", summary: ((offset: 16, ^2), (offset: 16, ^4))) ; guid = 7004155349499253778
 ^7 = typeidCompatibleVTable: (name: "_ZTS1B", summary: ((offset: 16, ^2))) ; guid = 6203814149063363976
Index: llvm/lib/IR/AsmWriter.cpp
===
--- llvm/lib/IR/AsmWriter.cpp
+++ llvm/lib/IR/AsmWriter.cpp
@@ -2896,10 +2896,14 @@
 }
 
 void AssemblyWriter::printGlobalVarSummary(const GlobalVarSummary *GS) {
+  auto VTableFuncs = GS->vTableFuncs();
   Out << ", varFlags: (readonly: " << GS->VarFlags.MaybeReadOnly << ", "
-  << "writeonly: " << GS->VarFlags.MaybeWriteOnly << ")";
+  << "writeonly: " << GS->VarFlags.MaybeWriteOnly;
+  if (!VTableFuncs.empty())
+Out << ", "
+<< "vcall_visibility: " << GS->VarFlags.VCallVisibility;
+  Out << ")";
 
-  auto VTableFuncs = GS->vTableFuncs();
   if (!VTableFuncs.empty()) {
 Out << ", vTableFuncs: (";
 FieldSeparator FS;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1028,7 +1028,8 @@
 }
 
 static uint64_t getEncodedGVarFlags(GlobalVarSummary::GVarFlags Flags) {
-  uint64_t RawFlags = Flags.MaybeReadOnly | (Flags.MaybeWriteOnly << 1);
+  uint64_t RawFlags = Flags.MaybeReadOnly | (Flags.MaybeWriteOnly << 1) |
+  Flags.VCallVisibility << 2;
   return RawFlags;
 }
 
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -985,8 +985,9 @@
 
 // Decode the flags for GlobalVariable in the summary
 static GlobalVarSummary::GVarFlags getDecodedGVarFlags(uint64_t RawFlags) {
-  return GlobalVarSummary::GVarFlags((RawFlags & 0x1) ? true : false,
- (RawFlags & 0x2) ? true : false);
+  return GlobalVarSummary::GVarFlags(
+  (RawFlags & 0x1) ? true : false, (RawFlags & 0x2) ? true : false,
+  (GlobalObject::VCallVisibility)(RawFlags >> 2));
 }
 
 static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {
Index: llvm/lib/AsmParser/LLToken.h
===
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -421,6 +421,7 @@
   kw_sizeM1,
   kw_bitMask,
   kw_inlineBits,
+  kw_vcall_visibility,
   kw_wpdResolutions,
   kw_wp

[clang] f7d9584 - Fix formatting in previous commits

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2019-12-29T19:41:30Z
New Revision: f7d9584c56d9d8e6092cc37d5e859db47b1a40d7

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

LOG: Fix formatting in previous commits

Added: 


Modified: 
clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
clang/lib/ASTMatchers/Dynamic/Parser.cpp

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp 
b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
index 1f0bbfa52693..4ebe1237b21c 100644
--- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
+++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
@@ -334,7 +334,8 @@ match callExpr
   Q = parse("let someMatcher\nm parmVarDecl()");
 
   ASSERT_TRUE(isa(Q));
-  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.",
+cast(Q)->ErrStr);
 
   Q = parse("\nm parmVarDecl()\nlet someMatcher\nm parmVarDecl()");
 
@@ -342,12 +343,14 @@ match callExpr
   Q = parse(Q->RemainingContent);
 
   ASSERT_TRUE(isa(Q));
-  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.",
+cast(Q)->ErrStr);
 
   Q = parse("\nlet someMatcher\n");
 
   ASSERT_TRUE(isa(Q));
-  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.",
+cast(Q)->ErrStr);
 
   Q = parse("\nm parmVarDecl()\nlet someMatcher\n");
 
@@ -355,7 +358,8 @@ match callExpr
   Q = parse(Q->RemainingContent);
 
   ASSERT_TRUE(isa(Q));
-  EXPECT_EQ("1:1: Invalid token  found when looking for a value.", 
cast(Q)->ErrStr);
+  EXPECT_EQ("1:1: Invalid token  found when looking for a value.",
+cast(Q)->ErrStr);
 
   Q = parse(R"matcher(
 
@@ -368,8 +372,8 @@ m parmVarDecl(
 
   ASSERT_TRUE(isa(Q));
   {
-  llvm::raw_null_ostream NullOutStream;
-  dyn_cast(Q)->run(NullOutStream, QS);
+llvm::raw_null_ostream NullOutStream;
+dyn_cast(Q)->run(NullOutStream, QS);
   }
 
   Q = parse(Q->RemainingContent);

diff  --git a/clang/lib/ASTMatchers/Dynamic/Parser.cpp 
b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
index 061da8b8600f..a0037549ca61 100644
--- a/clang/lib/ASTMatchers/Dynamic/Parser.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
@@ -614,7 +614,8 @@ bool Parser::parseExpressionImpl(VariantValue *Value) {
   case TokenInfo::TK_Period:
   case TokenInfo::TK_InvalidChar:
 const TokenInfo Token = Tokenizer->consumeNextToken();
-Error->addError(Token.Range, Error->ET_ParserInvalidToken) << (Token.Kind 
== TokenInfo::TK_NewLine ? "NewLine" : Token.Text);
+Error->addError(Token.Range, Error->ET_ParserInvalidToken)
+<< (Token.Kind == TokenInfo::TK_NewLine ? "NewLine" : Token.Text);
 return false;
   }
 



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

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:29
+  unless(anyOf(
+  isConstexpr(), hasDeclContext(namespaceDecl(isAnonymous())),
+  hasType(isConstQualified()),

vingeldal wrote:
> aaron.ballman wrote:
> > vingeldal wrote:
> > > aaron.ballman wrote:
> > > > vingeldal wrote:
> > > > > aaron.ballman wrote:
> > > > > > Why are you filtering out anonymous namespaces?
> > > > > If it's in an anonymous namespace it's no longer globally accessible, 
> > > > > it's only accessible within the file it is declared.
> > > > It is, however, at namespace scope which is what the C++ Core Guideline 
> > > > suggests to diagnose. Do you have evidence that this is the 
> > > > interpretation the guideline authors were looking for (perhaps they 
> > > > would like to update their suggested guidance for diagnosing)?
> > > > 
> > > > There are two dependency issues to global variables -- one is 
> > > > surprising linkage interactions (where the extern nature of the symbol 
> > > > is an issue across module boundaries) and the other is surprising name 
> > > > lookup behavior within the TU (where the global nature of the symbol is 
> > > > an issue). e.g.,
> > > > ```
> > > > namespace {
> > > > int ik;
> > > > }
> > > > 
> > > > void f() {
> > > >   for (int ij = 0; ik < 10; ij++) { // Oops, typo, but still compiles
> > > >   }
> > > > }
> > > > ```
> > > > That's why I think the guideline purposefully does not exclude things 
> > > > like anonymous namespaces.
> > > I don't have any evidence. To me the guideline still looks a bit 
> > > draft-like, so I just tried my best guess as to the intent of the 
> > > guideline.
> > > In reading the guideline I get the impression that the intent is to avoid 
> > > globally accessible data which is mutable,
> > > mainly for two reason:
> > >  * It's hard to reason about code where you are dependent upon state 
> > > which can be changed from anywhere in the code.
> > >  * There is a risk of race conditions with this kind of data.
> > > 
> > > Keeping the variable in an anonymous namespace seems to deals with the 
> > > issues which I interpret to be the focus of this guideline.
> > > Consider that the guideline is part of the interface section. If the 
> > > variable is declared in an anonymous namespace there is no risk that a 
> > > user of some service interface adds a dependency to that variable, since 
> > > the variable will be a hidden part of the provider implementation.
> > > 
> > > Admittedly the wording doesn't mention an exception for anonymous 
> > > namespaces, and the sentence you have referenced seems to suggest that 
> > > any non-const variable in namespace scope should be matched.
> > > I'm guessing though, that was intended to clarify that a variable is 
> > > still global even if in a (named) namespace, because that would not have 
> > > been an obvious interpretation otherwise.
> > > Without the referenced sentence one might interpret only variables 
> > > outside of namespace scope as global.
> > > Arguably a variable in namespace scope isn't globally declared, though it 
> > > is globally accessible, via the namespace name. I think the global 
> > > accessibility is the reason for dragging in variables from namespace 
> > > scope and if that is the case then we shouldn't also need to worry about 
> > > anonymous namespaces.
> > > This should probably be clarified in the actual guideline.
> > > This should probably be clarified in the actual guideline.
> > 
> > This sort of thing comes up with coding guidelines from time to time and 
> > the way we usually handle it is to ask the guideline authors for guidance. 
> > If they come back with an answer, then we go that route (while the authors 
> > fix the text) and if they don't come back with an answer, we muddle our way 
> > through. Would you mind filing an issue with the C++ Core Guideline folks 
> > to see if they can weigh in on the topic? If they don't respond in a timely 
> > fashion, I think it would make more sense to err on the side of caution and 
> > diagnose declarations within anonymous namespaces. This matches the current 
> > text from the core guideline, and we can always relax the rule if we find 
> > it causes a lot of heartache in the wild. (If you have data about how often 
> > this particular aspect of the check triggers on large real-world code 
> > bases, that could help us make a decision too.)
> I sent a pull request to the guide lines, suggesting a clarification. If that 
> is addressed in the near future I guess we can go on what they say about the 
> pull request. If it takes to long I'll just modify the code to warn in 
> anonymous namespace as well.
> https://github.com/isocpp/CppCoreGuidelines/pull/1553
Thank you, I think that approach makes sense.


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D71963: clang-tidy doc: Add the severities description

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D71963#1798212 , @sylvestre.ledru 
wrote:

> I do agree that they are subjective and not perfect.
>
> However, I found the classification extremely useful when you look at the 
> results on big projects.
>  I have been using codechecker (where the severities are coming from) for 
> Firefox and its has been extremely useful to evaluate the importance of the 
> checkers.


IMO, that usefulness comes from consistency when picking a severity. I share 
the concern that these are pretty subjective descriptions currently. For 
instance, the guidance you give in this patch is somewhat different than the 
guidance picked by CERT 
(https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RiskAssessment)
 and this will lead to discrepancies if it hasn't already.

>> For instance, the CERT rules all come with a severity specified by the rule 
>> itself
> 
> Did you see some difference?

I've not looked for them specifically yet (tbh, this severity thing caught me 
off guard, I didn't see the reviews for adding it), but my concern comes from 
the fact that the process of picking severity already differs between what's 
written and one of the coding standards we have checks for.

>> it if each coding standard has drastically different ideas about severity
> 
> Do you have some examples of this occurrence?

Not off the top of my head. I think it would be useful for someone to look at 
the coding standards we currently have clang-tidy checks for to see if those 
standards specify a severity for their rules. From there, we can see what 
commonalities there are between the coding standards and see if we can come up 
with a heuristic for picking a severity that roughly matches. Or maybe we 
should only specify a severity when one is picked by a coding standard and not 
attempt to determine our own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963



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

[PATCH] D71976: Match code following lambdas when ignoring invisible nodes

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71976

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1760,6 +1760,7 @@
 
 void func14() {
   []  (TemplateType t, TemplateType u) { int e = t + u; 
};
+  float i = 42.0;
 }
 
 )cpp";
@@ -1849,6 +1850,11 @@
  lambdaExpr(
  forFunction(functionDecl(hasName("func14"))),
  
has(templateTypeParmDecl(hasName("TemplateType")));
+
+  EXPECT_TRUE(
+  matches(Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ functionDecl(hasName("func14"),
+  hasDescendant(floatLiteral());
 }
 
 TEST(IgnoringImpCasts, MatchesImpCasts) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -248,7 +248,7 @@
 if (!match(*Node->getBody()))
   return false;
 
-return false;
+return true;
   }
 
   bool shouldVisitTemplateInstantiations() const { return true; }


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1760,6 +1760,7 @@
 
 void func14() {
   []  (TemplateType t, TemplateType u) { int e = t + u; };
+  float i = 42.0;
 }
 
 )cpp";
@@ -1849,6 +1850,11 @@
  lambdaExpr(
  forFunction(functionDecl(hasName("func14"))),
  has(templateTypeParmDecl(hasName("TemplateType")));
+
+  EXPECT_TRUE(
+  matches(Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ functionDecl(hasName("func14"),
+  hasDescendant(floatLiteral());
 }
 
 TEST(IgnoringImpCasts, MatchesImpCasts) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -248,7 +248,7 @@
 if (!match(*Node->getBody()))
   return false;
 
-return false;
+return true;
   }
 
   bool shouldVisitTemplateInstantiations() const { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71977: Implement additional traverse() overloads

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These overloads make it possible to wrap unless(), anyOf(), has() etc
with the traverse matcher.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71977

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1612,6 +1612,87 @@
   matches(VarDeclCode,
   traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
Matcher)));
+
+  EXPECT_TRUE(
+  matches(VarDeclCode, decl(traverse(ast_type_traits::TK_AsIs,
+ anyOf(cxxRecordDecl(), varDecl());
+
+  EXPECT_TRUE(matches(VarDeclCode,
+  floatLiteral(traverse(ast_type_traits::TK_AsIs,
+hasParent(implicitCastExpr());
+
+  EXPECT_TRUE(matches(
+  VarDeclCode,
+  floatLiteral(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+hasParent(varDecl());
+
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  varDecl(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+   unless(parmVarDecl());
+
+  EXPECT_TRUE(notMatches(
+  VarDeclCode,
+  varDecl(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+   has(implicitCastExpr());
+
+  EXPECT_TRUE(matches(VarDeclCode, varDecl(traverse(ast_type_traits::TK_AsIs,
+has(implicitCastExpr());
+
+  EXPECT_TRUE(notMatches(
+  VarDeclCode,
+  varDecl(has(traverse(ast_type_traits::TK_AsIs, floatLiteral());
+
+  EXPECT_TRUE(matches(
+  VarDeclCode,
+  functionDecl(traverse(ast_type_traits::TK_AsIs, hasName("foo");
+
+  EXPECT_TRUE(matches(
+  VarDeclCode,
+  functionDecl(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+hasName("foo");
+
+  EXPECT_TRUE(
+  matches(VarDeclCode, functionDecl(traverse(ast_type_traits::TK_AsIs,
+ hasAnyName("foo", "bar");
+
+  EXPECT_TRUE(matches(
+  VarDeclCode,
+  functionDecl(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+hasAnyName("foo", "bar");
+
+  EXPECT_TRUE(
+  matches(R"cpp(
+void foo(int a)
+{
+  int i = 3.0 + a;
+}
+void bar()
+{
+  foo(7.0);
+}
+)cpp",
+  callExpr(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+hasArgument(0, floatLiteral());
+
+  EXPECT_TRUE(
+  matches(R"cpp(
+void foo(int a)
+{
+  int i = 3.0 + a;
+}
+void bar()
+{
+  foo(7.0);
+}
+)cpp",
+  callExpr(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+hasAnyArgument(floatLiteral());
+
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  varDecl(traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+   hasType(builtinType());
 }
 
 TEST(Traversal, traverseMatcherNesting) {
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1129,6 +1129,23 @@
  TemplateSpecializationType, TemplateTypeParmType, TypedefType,
  UnresolvedUsingType, ObjCIvarRefExpr>;
 
+template  class ArgumentAdapterT,
+  typename T, typename ToTypes>
+class ArgumentAdaptingMatcherFuncAdaptor {
+public:
+  explicit ArgumentAdaptingMatcherFuncAdaptor(const Matcher &InnerMatcher)
+  : InnerMatcher(InnerMatcher) {}
+
+  using ReturnTypes = ToTypes;
+
+  template  operator Matcher() const {
+return Matcher(new ArgumentAdapterT(InnerMatcher));
+  }
+
+private:
+  const Matcher InnerMatcher;
+};
+
 /// Converts a \c Matcher to a matcher of desired type \c To by
 /// "adapting" a \c To into a \c T.
 ///
@@ -1146,28 +1163,16 @@
   typename FromTypes = AdaptativeDefaultFromTypes,
   typename ToTypes = AdaptativeDefaultToTypes>
 struct ArgumentAdaptingMatcherFunc {
-  template  class Adaptor {
-  public:
-explicit Adaptor(const Matcher &InnerMatcher)
-: InnerMatcher(InnerMatcher) {}
-
-using ReturnTypes = ToTypes;
-
-template  operator Matcher() const {
-  return Matcher(new ArgumentAdapterT(InnerMatcher));
-}
-
-  private:
-const Matcher InnerMatcher;
-  };
-
   template 
-  static Adap

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D57662#1797607 , @derek wrote:

> @zinovy.nis @alexfh @JonasToth @MyDeveloperDay
>
> Hi folks, please correct me if I'm wrong but it appears that an effect of 
> this change is that this script will no longer exit non-zero if `clang-tidy` 
> discovers any errors, which was the previous functionality with 
> `sys.exit(subprocess.call(' '.join(command), shell=True))`. As a result, when 
> we upgraded a project to LLVM 9 our CI began having false-positives, as we 
> relied on the exit code of this script to indicate success/failure. Would it 
> be possible to restore that functionality? I'd be happy to provide a patch.


Yeah, that would be good to restore. `run-clang-tidy.py` is parallelized and 
does fail properly (95% sure about that xD), so in doubt you can check that 
script as well.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57662



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:38
+
+  if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
+const auto NonEmptyBaseIt = llvm::find_if(CXXRD->bases(), IsNotEmptyBase);

You could use const auto * because type is specified in same statement.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:44
+
+  const auto *BaseRD = NonEmptyBaseIt->getType()->getAsCXXRecordDecl();
+  const uint64_t SizeOfBase = Ctx.getTypeSize(BaseRD->getTypeForDecl());

Please don't use auto when type is not specified in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:128
+  assert(SizeExpr != nullptr);
+  const auto ComparedBits = tryEvaluateSizeExpr(SizeExpr, Ctx);
+

Please don't use auto when type is not specified in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:139
+  if (RD != nullptr) {
+if (const CXXRecordDecl *CXXDecl = dyn_cast(RD)) {
+  if (!CXXDecl->isStandardLayout()) {

You could use const auto * because type is specified in same statement.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:103
+
+- New alias :doc:`cert-exp42-c
+  ` to

Please move into aliases section (in alphabetical order)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71973



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

[PATCH] D71980: Fix ClangTidy Bug - 44229, 33203 and 32204

[Nathan James via Phabricator via cfe-commits]

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

Forgot to clang-format...


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

https://reviews.llvm.org/D71980

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp


Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -106,7 +106,11 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+ hasElse(stmt()))
+  .bind("if"),
+  this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,10 @@
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation(
+  .bind("if"),
+  this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,8 @@
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  ifStmt(stmt().bind("if"),
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+ stmt().bind("if"),
  hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")),
  hasElse(stmt().bind("else"))),
   this);


Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -106,7 +106,11 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+ hasElse(stmt()))
+  .bind("if"),
+  this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,10 @@
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation(
+  .bind("if"),
+  this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,8 @@
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  ifStmt(stmt().bind("if"),
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+ stmt().bind("if"),
  hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")),
  hasElse(stmt().bind("else"))),
   this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71980: Fix ClangTidy Bug - 44229, 33203 and 32204

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: clang-tools-extra, cfe-commits.

fixes readability-misleading-indentation broken for if constexpr 
, 
readability-braces-around-statements broken for if constexpr 
 and bugprone-branch-clone false 
positive with template functions and constexpr 
 by disabling the relevant checks 
on if constexpr statements while inside an instantiated template. This is due 
to how the else branch of an if constexpr statement is folded away to a null 
statement if the condition evaluates to false


https://reviews.llvm.org/D71980

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp


Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -106,7 +106,7 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(ifStmt(unless(allOf(isConstexpr(), 
isInTemplateInstantiation())),hasElse(stmt())).bind("if"), this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,7 @@
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(ifStmt(unless(allOf(isConstexpr(), 
isInTemplateInstantiation(.bind("if"), this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,7 @@
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  ifStmt(stmt().bind("if"),
+  ifStmt(unless(allOf(isConstexpr(), 
isInTemplateInstantiation())),stmt().bind("if"),
  hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")),
  hasElse(stmt().bind("else"))),
   this);


Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -106,7 +106,7 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),hasElse(stmt())).bind("if"), this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,7 @@
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation(.bind("if"), this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,7 @@
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  ifStmt(stmt().bind("if"),
+  ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),stmt().bind("if"),
  hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")),

[PATCH] D71953: [Tooling] Infer driver mode and target for FixedCompilationDatabase

[Hanjiang Yu via Phabricator via cfe-commits]

de1acr0ix abandoned this revision.
de1acr0ix added a comment.

It is impossible to handle the case that positional argument starts with a 
source file with a driver mode suffix in file name stem, such as "somecpp.cpp".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71953



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze updated this revision to Diff 235526.
gbencze added a comment.

Update style based on comments.


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

https://reviews.llvm.org/D71973

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,352 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+
+typedef unsigned long long size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+namespace std {
+int memcmp(const void *lhs, const void *rhs, size_t count);
+}
+
+namespace sei_cert_example_exp42_c {
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+
+void noncompliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: comparing padding bytes
+  }
+}
+
+void compliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (left->c == right->c) && (left->i == right->i) &&
+  (0 == memcmp(left->buffer, right->buffer, 13))) {
+  }
+}
+} // namespace sei_cert_example_exp42_c
+
+namespace sei_cert_example_exp42_c_ex1 {
+#pragma pack(push, 1)
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+#pragma pack(pop)
+
+void compare(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// no-warning
+  }
+}
+} // namespace sei_cert_example_exp42_c_ex1
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+  int i;
+
+public:
+  virtual void f();
+};
+
+void f(C &c1, C &c2) {
+  if (!std::memcmp(&c1, &c2, sizeof(C))) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+// of non-standard-layout type 'sei_cert_example_oop57_cpp::C'; consider
+// using a comparison operator instead
+  }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace trailing_padding {
+struct S {
+  int i;
+  char c;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(int));
+  memcmp(&a, &b, sizeof(int) + sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace trailing_padding
+
+namespace inner_padding {
+struct S {
+  char c;
+  int i;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(char) + sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace inner_padding
+
+namespace bitfield {
+struct S {
+  int x : 10;
+  int y : 6;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, 2); // no-warning: no padding in first 2 bytes
+}
+} // namespace bitfield
+
+namespace bitfield_2 {
+struct S {
+  int x : 10;
+  int y : 7;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+  memcmp(&a, &b, 2);
+  memcmp(&a, &b, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace bitfield_2
+
+namespace bitfield_3 {
+struct S {
+  int x : 2;
+  int : 0;
+  int y : 6;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace bitfield_3
+
+namespace array_no_padding {
+struct S {
+  int x;
+  int y;
+};
+
+void test() {
+  S a[3];
+  S b[3];
+  memcmp(a, b, 3 * sizeof(S));
+}
+} // namespace array_no_padding
+
+namespace array_with_padding {
+struct S {
+  int x;
+  char y;
+};
+
+void test() {
+  S a[3];
+  S b[3];
+  memcmp(a, b, 3 * sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding bytes
+}
+} // namespace array_with_padding
+
+namespace unknown_count {
+struct S {
+  char c;
+  i

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze marked 5 inline comments as done.
gbencze added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:103
+
+- New alias :doc:`cert-exp42-c
+  ` to

Eugene.Zelenko wrote:
> Please move into aliases section (in alphabetical order)
I'm not quite sure where these are supposed to be. I can only find one other 
alias (cert-pos44-c) in the release notes, should I put it immediately before 
that? 


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

https://reviews.llvm.org/D71973



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:103
+
+- New alias :doc:`cert-exp42-c
+  ` to

gbencze wrote:
> Eugene.Zelenko wrote:
> > Please move into aliases section (in alphabetical order)
> I'm not quite sure where these are supposed to be. I can only find one other 
> alias (cert-pos44-c) in the release notes, should I put it immediately before 
> that? 
In 9.0 Release Notes aliases were placed after new checks.


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

https://reviews.llvm.org/D71973



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin created this revision.
AlexanderLanin added projects: clang-tools-extra, clang.
Herald added a subscriber: cfe-commits.

> tools/clang/tools/extra

has become

> clang-tools-extra

which was not updated in all docs.

Also hacking page has an outdated anchor for git.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71982

Files:
  clang-tools-extra/docs/clang-include-fixer.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/pp-trace.rst
  clang/www/hacking.html

Index: clang/www/hacking.html
===
--- clang/www/hacking.html
+++ clang/www/hacking.html
@@ -298,7 +298,7 @@
   Note that the paths embedded in the patch depend on where you run it,
   so changing directory to the llvm/tools/clang directory is recommended.
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git to contribute to Clang.
 
   
   LLVM IR Generation
Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -104,16 +104,16 @@
 
   ---
   - Callback: FileChanged
-Loc: "c:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "c:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
 (etc.)
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:5:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:5:1"
 Reason: ExitFile
 FileType: C_User
-PrevFID: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
+PrevFID: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/Input/Level1B.h"
   - Callback: EndOfMainFile
   ...
 
@@ -172,7 +172,7 @@
 Example:::
 
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
@@ -248,8 +248,8 @@
 FileName: "Input/Level1B.h"
 IsAngled: false
 FilenameRange: "Input/Level1B.h"
-File: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
-SearchPath: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace"
+File: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/Input/Level1B.h"
+SearchPath: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace"
 RelativePath: "Input/Level1B.h"
 Imported: (null)
 
@@ -271,8 +271,8 @@
 Example:::
 
   - Callback: moduleImport
-ImportLoc: "d:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:2"
-Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
+ImportLoc: "d:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:2"
+Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
 Imported: Level2B
 
 `EndOfMainFile `_ Callback
@@ -309,7 +309,7 @@
 Example:::
 
   - Callback: Ident
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-ident.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-ident.cpp:3:1"
 str: "$Id$"
 
 `PragmaDirective `_ Callback
@@ -329,7 +329,7 @@
 Example:::
 
   - Callback: PragmaDirective
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Introducer: PIK_HashPragma
 
 `PragmaComment `_ Callback
@@ -350,7 +350,7 @@
 Example:::
 
   - Callback: PragmaComment
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Kind: library
 Str: kernel32.lib
 
@@ -372,7 +372,7 @@
 Example:::
 
   - Callback: PragmaDetectMismatch
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Name:

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

[JunMa via Phabricator via cfe-commits]

junparser added a comment.

There is another issue we should consider:  clang is crashed when compile 
coroutine with -disable-llvm-passes and output an object file. 
Is it reasonable to run coroutine passes even  -disable-llvm-passes is enabled?




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1227
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroElidePass()));
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));

modocache wrote:
> junparser wrote:
> > Since coro elision depends on other optimization pass(inline and so on)  
> > implicitly,  how can we adjust the pipeline to achieve this.
> One option would be to use the new pass manager's registration callbacks, 
> like `PassBuilder::registerPipelineStartEPCallback` or 
> `PassBuilder::registerOptimizerLastEPCallback`. These work similarly to the 
> old pass manager's `PassManagerBuilder::addExtension`. That's something that 
> I think would be good to improve in a follow-up patch, but let me know if 
> you'd rather see it in this one.
yes,  please. It should be done in this patch sets. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71903



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

[clang] eadc97b - [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

[Vladimir Vereschaka via cfe-commits]

Author: Vladimir Vereschaka
Date: 2019-12-29T20:36:19-08:00
New Revision: eadc97b0ec87801ddcf35f03d1d005f9929a5254

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

LOG: [CMake] Added remote test execution support into CrossWinToARMLinux CMake 
cache file.

Added two confguration argument to provide a host name and SSH user name
to run the tests on the remote target host.

* REMOTE_TEST_HOST  - remote host name or address.
* REMOTE_TEST_USER  - passwordless SSH account name.

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

Added: 


Modified: 
clang/cmake/caches/CrossWinToARMLinux.cmake

Removed: 




diff  --git a/clang/cmake/caches/CrossWinToARMLinux.cmake 
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index 944465411363..9fbf090767e9 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_HOST="" ^
+#   -DREMOTE_TEST_USER="" ^
 #   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,33 @@ set(LIBCXX_SYSROOT  
"${DEFAULT_SYSROOT}" CACHE STRING ""
 set(BUILTINS_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLYON CACHE BOOL "")
+# Remote test configuration.
+if(DEFINED REMOTE_TEST_HOST)
+  set(DEFAULT_TEST_EXECUTOR 
"SSHExecutor('${REMOTE_TEST_HOST}', '${REMOTE_TEST_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  
"libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov



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

[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

[Vlad Vereschaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeadc97b0ec87: [CMake] Added remote test execution support 
into CrossWinToARMLinux CMake cache… (authored by vvereschaka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71625

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_HOST="" ^
+#   -DREMOTE_TEST_USER="" ^
 #   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,33 @@
 set(BUILTINS_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLYON CACHE BOOL "")
+# Remote test configuration.
+if(DEFINED REMOTE_TEST_HOST)
+  set(DEFAULT_TEST_EXECUTOR 
"SSHExecutor('${REMOTE_TEST_HOST}', '${REMOTE_TEST_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  
"libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_HOST="" ^
+#   -DREMOTE_TEST_USER="" ^
 #   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,33 @@
 set(BUILTINS_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLY 			ON CACHE BOOL "")
+# Remote test configuration.
+if(DEFINED REMOTE_TEST_HOST)
+  set(DEFAULT_TEST_EXECUTOR "SSHExecutor('${REMOTE_TEST_HOST}', '${REMOTE_TEST_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  "libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] ce1f95a - Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"

[Djordje Todorovic via cfe-commits]

Thanks!

Best regards,
Djordje

On 27.12.19. 20:30, David Blaikie wrote:
> 
> 
> On Thu, Dec 26, 2019 at 11:58 PM Djordje Todorovic 
> mailto:djordje.todoro...@rt-rk.com>> wrote:
> 
> Hi David,
> 
> It's a good question.
> 
> Current approach of the debug entry values will consider an entry value 
> as a valid value until the variable gets modified.
> 
> Please consider this.
> 
> void fn(int a) {
>   ...
>   a++;
> }
> 
> If there is an instruction that does not affect the code generated, e.g. 
> an ADD instruction that gets optimized out from the case above, it won't 
> force us to invalidate all the entry values before, since the instruction is 
> not there in the final code generated. The GCC does the same thing in that 
> situation. But if the instruction were at the beginning of the function (or 
> somewhere else), we believe that there is an DBG_VALUE representing that 
> variable's change (e.g. generated from the Salvage Debug Info), so the entry 
> value would not be used any more.
> 
> If we come up with a case where a dead store causing an invalid use of 
> the entry values, that will be good point for improvements.
> 
> 
> Ah, OK, so you actually want to know whether the entry value gets really 
> modified, makes sense to do that in the backend then - thanks for explaining!
>  
> 
> 
> Best regards,
> Djordje
> 
> On 26.12.19. 22:33, David Blaikie wrote:
> >
> >
> > On Wed, Nov 20, 2019 at 1:08 AM Djordje Todorovic via cfe-commits 
> mailto:cfe-commits@lists.llvm.org> 
> >> 
> wrote:
> >
> >
> >     Author: Djordje Todorovic
> >     Date: 2019-11-20T10:08:07+01:00
> >     New Revision: ce1f95a6e077693f93d8869245f911aff3eb7e4c
> >
> >     URL: 
> https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c
> >     DIFF: 
> https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c.diff
> >
> >     LOG: Reland "[clang] Remove the DIFlagArgumentNotModified debug 
> info flag"
> >
> >     It turns out that the ExprMutationAnalyzer can be very slow when AST
> >     gets huge in some cases. The idea is to move this analysis to the 
> LLVM
> >     back-end level (more precisely, in the LiveDebugValues pass). The 
> new
> >     approach will remove the performance regression, simplify the
> >     implementation and give us front-end independent implementation.
> >
> >
> > What if the LLVM backend optimized out a dead store? (then we might 
> concnlude that the argument is not modified, when it actually is modified?)
> >  
> >
> >
> >     Differential Revision: https://reviews.llvm.org/D68206
> >
> >     Added:
> >
> >
> >     Modified:
> >         clang/lib/CodeGen/CGDebugInfo.cpp
> >         clang/lib/CodeGen/CGDebugInfo.h
> >         
> lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
> >
> >     Removed:
> >         clang/test/CodeGen/debug-info-param-modification.c
> >
> >
> >     
> 
> >     diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> >     index 116517a9cb99..a9b3831aa0b5 100644
> >     --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> >     +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> >     @@ -18,7 +18,6 @@
> >      #include "CodeGenFunction.h"
> >      #include "CodeGenModule.h"
> >      #include "ConstantEmitter.h"
> >     -#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
> >      #include "clang/AST/ASTContext.h"
> >      #include "clang/AST/DeclFriend.h"
> >      #include "clang/AST/DeclObjC.h"
> >     @@ -3686,15 +3685,6 @@ void 
> CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
> >        if (HasDecl && isa(D))
> >          DeclCache[D->getCanonicalDecl()].reset(SP);
> >
> >     -  // We use the SPDefCache only in the case when the debug entry 
> values option
> >     -  // is set, in order to speed up parameters modification analysis.
> >     -  //
> >     -  // FIXME: Use AbstractCallee here to support ObjCMethodDecl.
> >     -  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)
> >     -    if (auto *FD = dyn_cast(D))
> >     -      if (FD->hasBody() && !FD->param_empty())
> >     -        SPDefCache[FD].reset(SP);
> >     -
> >        // Push the function onto the lexical block stack.
> >        LexicalBlockStack.emplace_back(SP);
> >
> >     @@ -4097,11 +4087,6 @@ llvm::DILocalVariable 
> *CGDebugInfo::EmitDeclare(const VarDecl *VD,
> >                               llvm::DebugLoc::get(Line, Column, Scope, 
> CurInline

[PATCH] D71980: [clang-tidy] Fix bug - 44229, 33203 and 32204

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Missing tests; please upload patches with full context (`-U9`); just 
referencing bug# is non-informative - best to say `"[clang-tidy] check-name: 
fix 'constexpr if' handling (PR#)"`; possibly you want to split this up into 
per-check patches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71980



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

There shows "Context not available".
Could you update for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71982



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