[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

I think interacting with git or even blocking for input doesn’t feel right

I have an issue out on llvm preserve checks that would run this tool and that 
couldn’t block like this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

LLVM lab network outage

[Galina Kistanova via cfe-commits]

Hello,

There is a network outage in the llvm lab.

We are working with the technical support, but there is no ETA yet.
I'll keep everyone updated.

Sorry for the inconvenience.

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

[PATCH] D105014: added some example code for llvm::Expected

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

I missed this thread earlier -- thanks to Dave for pointing me to it.

@kuhnel -- Thanks very much for working on this.

Out of interest, did you see 
https://llvm.org/docs/ProgrammersManual.html#error-handling ? If not (and if 
you find it helpful) then maybe we need to make that document more 
discoverable. If that document is not helpful then we should improve it.

> I guess the major difference in perspective may be due to different valuation 
> of the cost/"disease" it's intended to prevent - for myself I think the cost 
> is worthwhile but I understand that's not the case for you.

I've certainly found Error / Expected to be valuable for my work, but I 
designed them with my use-cases in mind so take that with a grain of salt. ;)

As a general observation: critiques of Error/Expected include the avoidable 
ergonomic issues (potentially addressable by API improvements), unavoidable 
ergonomic issues (we really want language support for this, but can't have it), 
and questions around correct application of Error/Expected -- sometimes error 
handling is just hard and Error/Expected feel awkward because they force you to 
confront it, and other times they're just not a good fit for whatever you're 
trying to do and you have to recognize when they should be dropped in favor of 
a more appropriate tool.

Especially on that first point (the avoidable ergonomic issues) I think that 
Error/Expected could benefit from review now that we've gained more practical 
experience with them. I'm not in a position to drive any such effort, but would 
be happy to help out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105014

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

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

[lin.sun via Phabricator via cfe-commits]

sunlin abandoned this revision.
sunlin added a comment.

Close it for ignore these headers from command line.

It is included from chain: "include/clang/Basic/TargetBuiltins.h" --> 
"clang/Basic/BuiltinsARM.def" --> "include/clang/Basic/arm_cde_builtins.inc"...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103080

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:21-25
+  # To allow testing with an untracked PLURAL_FILE
+  open(PLURAL_FILE, 'w').close() # TODO: remove this line when review is 
accepted
+  # TODO: use check_call when review is accepted
+  subprocess.call(['git', 'checkout', '--', PLURAL_FILE])
+

Just to let you know, I've made modifications to be able to test the code with 
an untracked plurals file. We might want stricter checks by calling check_call 
instead, also we won't need line 22 above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

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

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

From my point we can try that one, if there are problems we have plenty of time 
to revert it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370751.
FederAndInk added a comment.

use correct python assignment from tuple, ask the user if they want to invoke 
git, use call instead of check_call to allow testing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,76 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
 import collections
+import inspect
 import os
 import re
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+
+clean_plurals = input('Reset plurals file (to reemit warnings)? [y/N] ')
+if clean_plurals.lower() == 'y':
+  # To allow testing with an untracked PLURAL_FILE
+  open(PLURAL_FILE, 'w').close() # TODO: remove this line when review is accepted
+  # TODO: use check_call when review is accepted
+  subprocess.call(['git', 'checkout', '--', PLURAL_FILE])
+
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +93,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +138,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += i

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[ThePhD via Phabricator via cfe-commits]

ThePhD added a comment.

Hi, my name is JeanHeyd Meneide. I'm the Project Editor for C, but more 
importantly I'm the author of this paper: 
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2728.htm

This paper was accepted yesterday (September 3rd, 2021) into the C Standard, 
and (after I merge it and the like ~25 other papers + Annex X I need to merge), 
will appear in the next Draft of the C Standard.

As the paper's introduction and movtiation notes, the interpretation above that 
the locale-dependent encoding of `wchar_t` strings and `char` (MBS) strings for 
runtime functions like `mbstowcs` and `wcstombs` was not only a little bit 
silly, but also impossible to enforce properly on most systems without severe 
undue burden.

The wording of the paper explicitly removes the tie-in of the encoding of 
string literals and wide string literals to the library functions and instead 
makes them implementation-defined. This has no behavior change on any platform 
(it is, in a very strict sense, an expansion of the current definition and a 
standardization of existing practice amongst all implementations). What it does 
mean is that, however, Clang and every other compiler - so long as they pick a 
ISO10646-code point capable encoding for their `wchar_t` literals - can define 
this preprocessor macro unconditionally. My understanding is that on most 
systems where things have not been patched / tweaked, this applies since Clang 
vastly prefers UTF-32 in most of its setups.

It is my strong recommendation this patch be accepted and made unconditional, 
both in anticipation of the upcoming standard and the widespread existing 
practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[clang] 15cd16a - [Driver] Drop unnecessary const from return types (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-09-04T08:05:27-07:00
New Revision: 15cd16aaf0b7a01e9f4f9f7ddfcf866c4db47199

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

LOG: [Driver] Drop unnecessary const from return types (NFC)

Identified with readability-const-return-type.

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/lib/Driver/ToolChains/Hexagon.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.h 
b/clang/lib/Driver/ToolChains/AMDGPU.h
index b93d554683ff..63364d19a521 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -51,7 +51,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
   const std::map OptionsDefault;
 
   Tool *buildLinker() const override;
-  const StringRef getOptionDefault(options::ID OptID) const {
+  StringRef getOptionDefault(options::ID OptID) const {
 auto opt = OptionsDefault.find(OptID);
 assert(opt != OptionsDefault.end() && "No Default for Option");
 return opt->second;

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index a6cf378f81f3..2f184731d829 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -849,7 +849,7 @@ void arm::getARMTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
 
 }
 
-const std::string arm::getARMArch(StringRef Arch, const llvm::Triple &Triple) {
+std::string arm::getARMArch(StringRef Arch, const llvm::Triple &Triple) {
   std::string MArch;
   if (!Arch.empty())
 MArch = std::string(Arch);

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.h 
b/clang/lib/Driver/ToolChains/Arch/ARM.h
index 8e7c10ecd5d6..0ab0d6c281f8 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -24,7 +24,7 @@ namespace arm {
 
 std::string getARMTargetCPU(StringRef CPU, llvm::StringRef Arch,
 const llvm::Triple &Triple);
-const std::string getARMArch(llvm::StringRef Arch, const llvm::Triple &Triple);
+std::string getARMArch(llvm::StringRef Arch, const llvm::Triple &Triple);
 StringRef getARMCPUForMArch(llvm::StringRef Arch, const llvm::Triple &Triple);
 llvm::ARM::ArchKind getLLVMArchKindForARM(StringRef CPU, StringRef Arch,
   const llvm::Triple &Triple);

diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index f610bc2a781a..15c885fa6507 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -34,7 +34,7 @@ using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
-static const VersionTuple minimumMacCatalystDeploymentTarget() {
+static VersionTuple minimumMacCatalystDeploymentTarget() {
   return VersionTuple(13, 1);
 }
 

diff  --git a/clang/lib/Driver/ToolChains/Hexagon.cpp 
b/clang/lib/Driver/ToolChains/Hexagon.cpp
index 5f5964ec982b..01568e0b2edd 100644
--- a/clang/lib/Driver/ToolChains/Hexagon.cpp
+++ b/clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -704,11 +704,11 @@ bool HexagonToolChain::isAutoHVXEnabled(const 
llvm::opt::ArgList &Args) {
 // Returns the default CPU for Hexagon. This is the default compilation target
 // if no Hexagon processor is selected at the command-line.
 //
-const StringRef HexagonToolChain::GetDefaultCPU() {
+StringRef HexagonToolChain::GetDefaultCPU() {
   return "hexagonv60";
 }
 
-const StringRef HexagonToolChain::GetTargetCPUVersion(const ArgList &Args) {
+StringRef HexagonToolChain::GetTargetCPUVersion(const ArgList &Args) {
   Arg *CpuArg = nullptr;
   if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
 CpuArg = A;

diff  --git a/clang/lib/Driver/ToolChains/Hexagon.h 
b/clang/lib/Driver/ToolChains/Hexagon.h
index 9dc9b3ceddde..899630555352 100644
--- a/clang/lib/Driver/ToolChains/Hexagon.h
+++ b/clang/lib/Driver/ToolChains/Hexagon.h
@@ -107,8 +107,8 @@ class LLVM_LIBRARY_VISIBILITY HexagonToolChain : public 
Linux {
   std::string getCompilerRTPath() const override;
 
   static bool isAutoHVXEnabled(const llvm::opt::ArgList &Args);
-  static const StringRef GetDefaultCPU();
-  static const StringRef GetTargetCPUVersion(const llvm::opt::ArgList &Args);
+  static StringRef GetDefaultCPU();
+  static StringRef GetTargetCPUVersion(const llvm::opt::ArgList &Args);
 
   static Optional getSmallDataThreshold(
   const llvm::opt::ArgList &Args);



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In D108003#2983609 , @xbolva00 wrote:

> I think I will start with AND only as this is more error prone pattern.

FWIW, I still see no reason //not// to warn on `|`-for-`||`. Again per 
https://quuxplusone.github.io/blog/2020/09/02/wparentheses/ : if we see the 
programmer writing `somebool | foo`, we //know// they've messed up — they might 
have meant either `int(somebool) | foo` or `somebool || foo`, we're not sure 
//which//, but we know they've messed up //somehow//. So it makes sense to tell 
them about it. The codepaths for `&&/&` and `||/|` are going to be shared, 
right? So I see no reason to special-case-//not//-warn on `|`.
I do agree that it seems reasonable for people to screw up `&` more frequently 
than `|`. It's very common in C and C++ to type just a single `&` (e.g. for 
address-of); it's very uncommon to ever type a single `|`, so the muscle memory 
won't be there to typo it. Also, `|` is way off on the side of the keyboard 
where the typist is probably paying a little more attention; `&` is right in 
the hot path where our fingers are likely moving quickly. But from the 
compiler's POV, we might be //surprised// to see the rare `|`-for-`||` typo 
("whoa! I hardly ever see one of those!") but that's no reason to keep it a 
secret from the user.




Comment at: clang/test/Sema/warn-bitwise-or-bool.c:38
+#ifdef __cplusplus
+  b = foo() bitor bar(); // expected-warning {{use of bitwise '|' with boolean 
operands}}
+#endif

Arguably, `foo() bitor bar()` is a sufficiently high "degree of ornamentation" 
to unambiguously signal the programmer's intent here. What are the chances that 
the programmer wrote `bitor` instead of `or` by accident?  But it's not worth 
adding any code just to special-case-//not//-warn here.
(The reverse — writing `or` when you meant `bitor` — strikes me as a more 
likely error.)
D107294 is related.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens in declarations

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1553
+  "%select{references|destructors|block pointers|ref-qualified member 
functions}2">,
+  InGroup>;
+

@cjdb: I suggest splitting this diagnostic up, mainly for sanity's sake (it 
currently has 4x2x4 = 32 possible outputs, which is Too Much) but also to 
improve greppability (it took me five tries to find the source of the message 
"when declaring destructors" in this PR). I would do one diagnostic `"use 
'%{&|&&}0' when declaring %{lvalue|rvalue}1 %{references|ref-qualified member 
functions}"`; another diagnostic `"use '~' when declaring destructors"`; and a 
third `"use '^' when declaring block pointers"`.

@aaron.ballman wrote:
> One question I have about both declarations and expressions are whether we 
> have an appetite to diagnose overloaded operators or not. Personally, I think 
> it'd be reasonable to diagnose something like `foo->operator bitand();` or 
> `operator not_eq(A, B);` as expressions, but not reasonable to diagnose the 
> declaration of the overloaded operators using alternative tokens.

AIUI, @cjdb is deferring all diagnostics related to //expressions// into a 
later PR, and I think that's reasonable. There's a subtlety to your example I 
can't tell if you intended: `foo->operator bitand()` is clearly wrong because 
the unary operator `&` being invoked there is not `bitand`; it's address-of! 
`foo->operator bitand()` will presumably fall into the same category as 
`foo->compl Foo()`. Whereas there's nothing so wrong with `foo->operator 
not_eq(bar)` or `operator not_eq(foo, bar)`. 

However, all that has at least two adjacent issues that I know are on @cjdb's 
radar: (1) `auto transformed = foo bitor std::views::transform(bar)` is 
"wrong", and in fact (2) //any// use of alternative tokens other than `and`, 
`or`, `not` is unstylish. If you diagnose //all// usage of `bitand`, `bitor`, 
`compl`, etc., then you don't get any of these subtle problems, because 
`&&/||/!` only have one meaning each (except for rvalue references, which is 
handled in this PR; and GCC-extension `&&label`, which should be easy to catch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

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

xbolva00 added a comment.

Ping

I think I will start with AND only as this is more error prone pattern.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

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

steakhal added a comment.

Looks great.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1636-1640
+  // Technically, only i == length is guaranteed to be null.
+  // However, such overflows should be caught before reaching this point;
+  // the only time such an access would be made is if a string literal was
+  // used to initialize a larger array.
+  // FIXME: Take array dimension into account to prevent exceeding its size.

This seems like a huge hack. Do we really need this? I think we should account 
for this case at the initialization of the mentioned array...
Leave it as-is right now, but eh, ugly.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641
+  // FIXME: Take array dimension into account to prevent exceeding its size.
+  const int64_t I = Idx.getExtValue();
+  uint32_t Code =

You could use the `uint64_t` type here, and spare the subsequent explicit cast. 
This operation would be safe since `Idx` must be non-negative here.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1677-1682
+if (const auto *SL = dyn_cast(Init)) {
+  if (auto CI = R->getIndex().getAs()) {
+const llvm::APSInt &Idx = CI->getValue();
+QualType ElemT = R->getElementType();
+return getSValFromStringLiteralByIndex(SL, Idx, ElemT);
+  }

So, your patch is NFC except for this part, which applies the very same logic 
for global initializers.
Am'I right?



Comment at: clang/test/Analysis/initialization.cpp:132
+
+char const glob_arr6[5] = "123";
+void glob_array_index4() {

Ah, it's somewhat confusing.
At first, when I looked at it, I assumed that this array has `6` elements as 
its name suggests.
But it has actually 5 elements.



Comment at: clang/test/Analysis/initialization.cpp:156-157
+void glob_invalid_index7() {
+  int idx = -42;
+  auto x = glob_arr6[idx]; // expected-warning{{garbage or undefined}}
+}

You could inline the `-42` without changing any expected behavior.
It would make the test terser IMO. The same applies to the other case.



Comment at: clang/test/Analysis/initialization.cpp:160-166
+// TODO: Support multidimensional array.
+void glob_invalid_index8() {
+  const char *ptr = glob_arr6;
+  int idx = 42;
+  // FIXME: Should warn {{garbage or undefined}}.
+  auto x = ptr[idx]; // no-warning
+}

I'm not sure if I follow. The `TODO` implies to me that this case is about 
//multidimensional  array//s, but it's actually not.
`glob_arr6` is of type `const char[5]`
Could you clarify this?
BTW, at first glance, the gist of this case is the same as the 
`glob_invalid_index7`.
Why does this behave differently? I'm puzzled.


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

https://reviews.llvm.org/D107339

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

[PATCH] D109237: [clang][AST] Add support for SubstTemplateTypeParmPackType to ASTImporter

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

steakhal added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:1493
+const SubstTemplateTypeParmPackType *T) {
+  ExpectedType ReplacedOrErr = import(QualType(T->getReplacedParameter(), 0));
+  if (!ReplacedOrErr)

martong wrote:
> steakhal wrote:
> > I know it's somewhat ugly, but it gets the job done.
> > There are other instances like this already.
> It is unfortunate that we can't we import the `Type` ptr directly and we must 
> create a qualified type with meaningless qualifiers. However, we use the very 
> same methods for `VisitSubstTemplateTypeParmType` so, this is okay for now. 
> Perhaps, in a later patch it would make sense to create an `import` overload 
> that returns and `Exptected` and takes `Type*` as a param. That could 
> simplify this line and the `cast` below.
D109269 aims to address this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109237

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

[PATCH] D109237: [clang][AST] Add support for SubstTemplateTypeParmPackType to ASTImporter

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

steakhal added a comment.

In D109237#2982674 , @shafik wrote:

> LGTM, please run the `check-lldb` before landing this since lldb can be 
> sensitive to `ASTImporter` changes and it is nice to catch regressions there 
> before landing.

The patch did not introduce new failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109237

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

[PATCH] D109237: [clang][AST] Add support for SubstTemplateTypeParmPackType to ASTImporter

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6ca91ea4245: [clang][AST] Add support for 
SubstTemplateTypeParmPackType to ASTImporter (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109237

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4693,6 +4693,57 @@
   ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
 }
 
+const AstTypeMatcher
+substTemplateTypeParmPackType;
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportSubstTemplateTypeParmPackType) 
{
+  constexpr auto Code = R"(
+template struct D {
+  template using B = int(int (*...p)(T, U));
+  template D(B*);
+};
+int f(int(int, int), int(int, int));
+
+using asd = D::B;
+)";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input.cpp");
+  auto *FromClass = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl());
+
+  {
+ASTContext &FromCtx = FromTU->getASTContext();
+const auto *FromSubstPack = selectFirst(
+"pack", match(substTemplateTypeParmPackType().bind("pack"), FromCtx));
+
+ASSERT_TRUE(FromSubstPack);
+ASSERT_EQ(FromSubstPack->getIdentifier()->getName(), "T");
+ArrayRef FromArgPack =
+FromSubstPack->getArgumentPack().pack_elements();
+ASSERT_EQ(FromArgPack.size(), 3u);
+ASSERT_EQ(FromArgPack[0].getAsType(), FromCtx.FloatTy);
+ASSERT_EQ(FromArgPack[1].getAsType(), FromCtx.DoubleTy);
+ASSERT_EQ(FromArgPack[2].getAsType(), FromCtx.FloatTy);
+  }
+  {
+// Let's do the import.
+ClassTemplateSpecializationDecl *ToClass = Import(FromClass, Lang_CXX11);
+ASTContext &ToCtx = ToClass->getASTContext();
+
+const auto *ToSubstPack = selectFirst(
+"pack", match(substTemplateTypeParmPackType().bind("pack"), ToCtx));
+
+// Check if it meets the requirements.
+ASSERT_TRUE(ToSubstPack);
+ASSERT_EQ(ToSubstPack->getIdentifier()->getName(), "T");
+ArrayRef ToArgPack =
+ToSubstPack->getArgumentPack().pack_elements();
+ASSERT_EQ(ToArgPack.size(), 3u);
+ASSERT_EQ(ToArgPack[0].getAsType(), ToCtx.FloatTy);
+ASSERT_EQ(ToArgPack[1].getAsType(), ToCtx.DoubleTy);
+ASSERT_EQ(ToArgPack[2].getAsType(), ToCtx.FloatTy);
+  }
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -383,6 +383,8 @@
 ExpectedType VisitTemplateTypeParmType(const TemplateTypeParmType *T);
 ExpectedType VisitSubstTemplateTypeParmType(
 const SubstTemplateTypeParmType *T);
+ExpectedType
+VisitSubstTemplateTypeParmPackType(const SubstTemplateTypeParmPackType *T);
 ExpectedType VisitTemplateSpecializationType(
 const TemplateSpecializationType *T);
 ExpectedType VisitElaboratedType(const ElaboratedType *T);
@@ -1486,6 +1488,22 @@
 Replaced, (*ToReplacementTypeOrErr).getCanonicalType());
 }
 
+ExpectedType ASTNodeImporter::VisitSubstTemplateTypeParmPackType(
+const SubstTemplateTypeParmPackType *T) {
+  ExpectedType ReplacedOrErr = import(QualType(T->getReplacedParameter(), 0));
+  if (!ReplacedOrErr)
+return ReplacedOrErr.takeError();
+  const TemplateTypeParmType *Replaced =
+  cast(ReplacedOrErr->getTypePtr());
+
+  Expected ToArgumentPack = import(T->getArgumentPack());
+  if (!ToArgumentPack)
+return ToArgumentPack.takeError();
+
+  return Importer.getToContext().getSubstTemplateTypeParmPackType(
+  Replaced, *ToArgumentPack);
+}
+
 ExpectedType ASTNodeImporter::VisitTemplateSpecializationType(
const TemplateSpecializationType *T) {
   auto ToTemplateOrErr = import(T->getTemplateName());


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4693,6 +4693,57 @@
   ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
 }
 
+const AstTypeMatcher
+substTemplateTypeParmPackType;
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportSubstTemplateTypeParmPackType) {
+  constexpr auto Code = R"(
+template struct D {
+  template using B = int(int (*...p)(T, U));
+  template D(B*);
+};
+int f(int(int, int), int(int, int));
+
+using asd = D::B;
+)";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input.cpp");
+  auto

[PATCH] D108824: [analyzer] SValBuilder should have an easy access to AnalyzerOptions

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb97a96400a3f: [analyzer] SValBuilder should have an easy 
access to AnalyzerOptions (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108824

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -11,7 +11,6 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -25,7 +24,7 @@
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
 ProgramStateManager &stateMgr)
-: SValBuilder(alloc, context, stateMgr) {}
+  : SValBuilder(alloc, context, stateMgr) {}
   ~SimpleSValBuilder() override {}
 
   SVal evalMinus(NonLoc val) override;
@@ -320,13 +319,10 @@
   // We expect everything to be of the same type - this type.
   QualType SingleTy;
 
-  auto &Opts =
-StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
-
   // FIXME: After putting complexity threshold to the symbols we can always
   //rearrange additive operations but rearrange comparisons only if
   //option is set.
-  if(!Opts.ShouldAggressivelySimplifyBinaryOperation)
+  if (!SVB.getAnalyzerOptions().ShouldAggressivelySimplifyBinaryOperation)
 return None;
 
   SymbolRef LSym = Lhs.getAsSymbol();
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -49,6 +49,16 @@
 
 void SValBuilder::anchor() {}
 
+SValBuilder::SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
+ ProgramStateManager &stateMgr)
+: Context(context), BasicVals(context, alloc),
+  SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
+  StateMgr(stateMgr),
+  AnOpts(
+  stateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions()),
+  ArrayIndexTy(context.LongLongTy),
+  ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
+
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
 return makeNull();
@@ -405,9 +415,7 @@
 
   // TODO: When the Max Complexity is reached, we should conjure a symbol
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = StateMgr.getOwningEngine()
-   .getAnalysisManager()
-   .options.MaxSymbolComplexity;
+  const unsigned MaxComp = AnOpts.MaxSymbolComplexity;
 
   if (symLHS && symRHS &&
   (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -30,7 +30,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -793,11 +792,7 @@
 if (Size.isNullValue())
   return true;
 
-// FIXME: Acquire the AnalyzerOptions in a simpler way.
-const AnalyzerOptions &Opts = SVB.getStateManager()
-  .getOwningEngine()
-  .getAnalysisManager()
-  .getAnalyzerOptions();
+const AnalyzerOptions &Opts = SVB.getAnalyzerOptions();
 if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers &&
 Size.isOneValue())
   return true;
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.

[PATCH] D108230: [analyzer] Ignore single element arrays in getStaticSize() conditionally

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG91c07eb8ee6e: [analyzer] Ignore single element arrays in 
getStaticSize() conditionally (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108230

Files:
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/flexible-array-members.c

Index: clang/test/Analysis/flexible-array-members.c
===
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -9,6 +9,11 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++
 
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
+// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \
+// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS
+
 typedef __typeof(sizeof(int)) size_t;
 size_t clang_analyzer_getExtent(void *);
 void clang_analyzer_dump(size_t);
@@ -75,6 +80,26 @@
 int data[1];
   } FAM;
 
+#ifdef SINGLE_ELEMENT_FAMS
+  FAM likely_fam;
+  clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
+  clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
+  // expected-warning@-2 {{8 S64b}}
+  // expected-warning@-2 {{Unknown}}
+
+  FAM *p = (FAM *)alloca(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(p));
+  clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+  // expected-warning@-2 {{8 U64b}}
+  // expected-warning@-2 {{Unknown}}
+
+  FAM *q = (FAM *)malloc(sizeof(FAM));
+  clang_analyzer_dump(clang_analyzer_getExtent(q));
+  clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+  // expected-warning@-2 {{8 U64b}}
+  // expected-warning@-2 {{Unknown}}
+  free(q);
+#else
   FAM likely_fam;
   clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
   clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
@@ -93,4 +118,5 @@
   // expected-warning@-2 {{8 U64b}}
   // expected-warning@-2 {{4 S64b}}
   free(q);
+#endif
 }
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -31,6 +31,7 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: consider-single-element-arrays-as-flexible-array-members = false
 // CHECK-NEXT: core.CallAndMessage:ArgInitializedness = true
 // CHECK-NEXT: core.CallAndMessage:ArgPointeeInitializedness = false
 // CHECK-NEXT: core.CallAndMessage:CXXDeallocationArg = true
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -28,7 +28,9 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -770,9 +772,16 @@
 QualType Ty = cast(SR)->getDesugaredValueType(Ctx);
 const DefinedOrUnknownSVal Size = getElementExtent(Ty, SVB);
 
-// A zero-length array at the end of a struct often stands for dynamically
-// allocated extra memory.
-const auto isFlexibleArrayMemberCandidate = [this](QualType Ty) -> bool {
+// We currently don't model flexible array members (FAMs), which are:
+//  - int array[]; of IncompleteArrayType
+//  - int array[0]; of ConstantArrayType with size 0
+//  - int array[1]; of ConstantArrayType with size 1 (*)
+// (*): Consider single element array object members as FAM candidates only
+//  if the consider-single-element-arrays-as-flexible-array-members
+//  analyzer option is true.
+// https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
+const auto isFlexibleArrayMemberCandidate = [this,
+  

[clang] d6ca91e - [clang][AST] Add support for SubstTemplateTypeParmPackType to ASTImporter

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-09-04T10:19:57+02:00
New Revision: d6ca91ea42455453d08a7922e96fa6685827028d

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

LOG: [clang][AST] Add support for SubstTemplateTypeParmPackType to ASTImporter

Thank you @martong for acquiring a suitable test case!

Reviewed By: shafik, martong

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index e7d02b69c9d71..c30d35ae884b4 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -383,6 +383,8 @@ namespace clang {
 ExpectedType VisitTemplateTypeParmType(const TemplateTypeParmType *T);
 ExpectedType VisitSubstTemplateTypeParmType(
 const SubstTemplateTypeParmType *T);
+ExpectedType
+VisitSubstTemplateTypeParmPackType(const SubstTemplateTypeParmPackType *T);
 ExpectedType VisitTemplateSpecializationType(
 const TemplateSpecializationType *T);
 ExpectedType VisitElaboratedType(const ElaboratedType *T);
@@ -1486,6 +1488,22 @@ ExpectedType 
ASTNodeImporter::VisitSubstTemplateTypeParmType(
 Replaced, (*ToReplacementTypeOrErr).getCanonicalType());
 }
 
+ExpectedType ASTNodeImporter::VisitSubstTemplateTypeParmPackType(
+const SubstTemplateTypeParmPackType *T) {
+  ExpectedType ReplacedOrErr = import(QualType(T->getReplacedParameter(), 0));
+  if (!ReplacedOrErr)
+return ReplacedOrErr.takeError();
+  const TemplateTypeParmType *Replaced =
+  cast(ReplacedOrErr->getTypePtr());
+
+  Expected ToArgumentPack = import(T->getArgumentPack());
+  if (!ToArgumentPack)
+return ToArgumentPack.takeError();
+
+  return Importer.getToContext().getSubstTemplateTypeParmPackType(
+  Replaced, *ToArgumentPack);
+}
+
 ExpectedType ASTNodeImporter::VisitTemplateSpecializationType(
const TemplateSpecializationType *T) {
   auto ToTemplateOrErr = import(T->getTemplateName());

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index d49e7c0cd1fef..556f206f2dc41 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4693,6 +4693,57 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
 }
 
+const AstTypeMatcher
+substTemplateTypeParmPackType;
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportSubstTemplateTypeParmPackType) 
{
+  constexpr auto Code = R"(
+template struct D {
+  template using B = int(int (*...p)(T, U));
+  template D(B*);
+};
+int f(int(int, int), int(int, int));
+
+using asd = D::B;
+)";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input.cpp");
+  auto *FromClass = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl());
+
+  {
+ASTContext &FromCtx = FromTU->getASTContext();
+const auto *FromSubstPack = selectFirst(
+"pack", match(substTemplateTypeParmPackType().bind("pack"), FromCtx));
+
+ASSERT_TRUE(FromSubstPack);
+ASSERT_EQ(FromSubstPack->getIdentifier()->getName(), "T");
+ArrayRef FromArgPack =
+FromSubstPack->getArgumentPack().pack_elements();
+ASSERT_EQ(FromArgPack.size(), 3u);
+ASSERT_EQ(FromArgPack[0].getAsType(), FromCtx.FloatTy);
+ASSERT_EQ(FromArgPack[1].getAsType(), FromCtx.DoubleTy);
+ASSERT_EQ(FromArgPack[2].getAsType(), FromCtx.FloatTy);
+  }
+  {
+// Let's do the import.
+ClassTemplateSpecializationDecl *ToClass = Import(FromClass, Lang_CXX11);
+ASTContext &ToCtx = ToClass->getASTContext();
+
+const auto *ToSubstPack = selectFirst(
+"pack", match(substTemplateTypeParmPackType().bind("pack"), ToCtx));
+
+// Check if it meets the requirements.
+ASSERT_TRUE(ToSubstPack);
+ASSERT_EQ(ToSubstPack->getIdentifier()->getName(), "T");
+ArrayRef ToArgPack =
+ToSubstPack->getArgumentPack().pack_elements();
+ASSERT_EQ(ToArgPack.size(), 3u);
+ASSERT_EQ(ToArgPack[0].getAsType(), ToCtx.FloatTy);
+ASSERT_EQ(ToArgPack[1].getAsType(), ToCtx.DoubleTy);
+ASSERT_EQ(ToArgPack[2].getAsType(), ToCtx.FloatTy);
+  }
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {



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

[clang] b97a964 - [analyzer] SValBuilder should have an easy access to AnalyzerOptions

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-09-04T10:19:57+02:00
New Revision: b97a96400a3f9ec33b80d0726111aae1c7b24513

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

LOG: [analyzer] SValBuilder should have an easy access to AnalyzerOptions

`SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()`
is quite a mouthful and might involve a few pointer indirections to get
such a simple thing like an analyzer option.

This patch introduces an `AnalyzerOptions` reference to the `SValBuilder`
abstract class, while refactors a few cases to use this /simpler/ accessor.

Reviewed By: martong, Szelethus

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 87a49cf4ffe9a..61dfdbb0688bc 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -33,6 +33,7 @@
 
 namespace clang {
 
+class AnalyzerOptions;
 class BlockDecl;
 class CXXBoolLiteralExpr;
 class CXXMethodDecl;
@@ -66,6 +67,8 @@ class SValBuilder {
 
   ProgramStateManager &StateMgr;
 
+  const AnalyzerOptions &AnOpts;
+
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
@@ -96,11 +99,7 @@ class SValBuilder {
 
 public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
-  ProgramStateManager &stateMgr)
-  : Context(context), BasicVals(context, alloc),
-SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
-ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
+  ProgramStateManager &stateMgr);
 
   virtual ~SValBuilder() = default;
 
@@ -188,6 +187,8 @@ class SValBuilder {
   MemRegionManager &getRegionManager() { return MemMgr; }
   const MemRegionManager &getRegionManager() const { return MemMgr; }
 
+  const AnalyzerOptions &getAnalyzerOptions() const { return AnOpts; }
+
   // Forwarding methods to SymbolManager.
 
   const SymbolConjured* conjureSymbol(const Stmt *stmt,

diff  --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp 
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 0358430f519e2..261bec6fabfe8 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -30,7 +30,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -793,11 +792,7 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const 
MemRegion *MR,
 if (Size.isNullValue())
   return true;
 
-// FIXME: Acquire the AnalyzerOptions in a simpler way.
-const AnalyzerOptions &Opts = SVB.getStateManager()
-  .getOwningEngine()
-  .getAnalysisManager()
-  .getAnalyzerOptions();
+const AnalyzerOptions &Opts = SVB.getAnalyzerOptions();
 if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers &&
 Size.isOneValue())
   return true;

diff  --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 8d27ba3e65c46..34eeb6cee9012 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -49,6 +49,16 @@ using namespace ento;
 
 void SValBuilder::anchor() {}
 
+SValBuilder::SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
+ ProgramStateManager &stateMgr)
+: Context(context), BasicVals(context, alloc),
+  SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
+  StateMgr(stateMgr),
+  AnOpts(
+  
stateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions()),
+  ArrayIndexTy(context.LongLongTy),
+  ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
+
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
 return makeNull();
@@ -405,9 +415,7 @@ S

[clang] 91c07eb - [analyzer] Ignore single element arrays in getStaticSize() conditionally

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-09-04T10:19:57+02:00
New Revision: 91c07eb8ee6ea2d48158dce123bac7b7c30eb294

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

LOG: [analyzer] Ignore single element arrays in getStaticSize() conditionally

Quoting https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html:
> In the absence of the zero-length array extension, in ISO C90 the contents
> array in the example above would typically be declared to have a single
> element.

We should not assume that the size of the //flexible array member// field has
a single element, because in some cases they use it as a fallback for not
having the //zero-length array// language extension.
In this case, the analyzer should return `Unknown` as the extent of the field
instead.

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/flexible-array-members.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index e97e0a6892a93..16ca7a8456e39 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -320,6 +320,14 @@ ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, 
"display-checker-name",
 "Display the checker name for textual outputs",
 true)
 
+ANALYZER_OPTION(
+bool, ShouldConsiderSingleElementArraysAsFlexibleArrayMembers,
+"consider-single-element-arrays-as-flexible-array-members",
+"Consider single element arrays as flexible array member candidates. "
+"This will prevent the analyzer from assuming that a single element array "
+"holds a single element.",
+false)
+
 
//===--===//
 // Unsigned analyzer options.
 
//===--===//

diff  --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp 
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 1a614d4d2bcdb..0358430f519e2 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -28,7 +28,9 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -770,9 +772,16 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const 
MemRegion *MR,
 QualType Ty = cast(SR)->getDesugaredValueType(Ctx);
 const DefinedOrUnknownSVal Size = getElementExtent(Ty, SVB);
 
-// A zero-length array at the end of a struct often stands for dynamically
-// allocated extra memory.
-const auto isFlexibleArrayMemberCandidate = [this](QualType Ty) -> bool {
+// We currently don't model flexible array members (FAMs), which are:
+//  - int array[]; of IncompleteArrayType
+//  - int array[0]; of ConstantArrayType with size 0
+//  - int array[1]; of ConstantArrayType with size 1 (*)
+// (*): Consider single element array object members as FAM candidates only
+//  if the consider-single-element-arrays-as-flexible-array-members
+//  analyzer option is true.
+// https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
+const auto isFlexibleArrayMemberCandidate = [this,
+ &SVB](QualType Ty) -> bool {
   const ArrayType *AT = Ctx.getAsArrayType(Ty);
   if (!AT)
 return false;
@@ -783,6 +792,15 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const 
MemRegion *MR,
 const llvm::APInt &Size = CAT->getSize();
 if (Size.isNullValue())
   return true;
+
+// FIXME: Acquire the AnalyzerOptions in a simpler way.
+const AnalyzerOptions &Opts = SVB.getStateManager()
+  .getOwningEngine()
+  .getAnalysisManager()
+  .getAnalyzerOptions();
+if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers &&
+Size.isOneValue())
+  return true;
   }
   return false;
 };

diff  --git a/clang/test/Analysis/analyzer-confi

[PATCH] D77470: [clang] NFC: Fix trivial typo in comments and document

[Kazuaki Ishizaki via Phabricator via cfe-commits]

kiszk added a comment.

Thank you very much at 
https://github.com/llvm/llvm-project/commit/8f77dc459e31aad6daab89a124fa92067916274c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77470

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

[PATCH] D77470: [clang] NFC: Fix trivial typo in comments and document

[Shivam Gupta via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f77dc459e31: [clang] NFC: Fix trivial typo in comments and 
document (authored by kiszk, committed by xgupta).

Changed prior to commit:
  https://reviews.llvm.org/D77470?vs=255065&id=370704#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77470

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTStructuralEquivalence.h
  clang/include/clang/AST/ComparisonCategories.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Sema/SemaAvailability.cpp

Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -57,7 +57,7 @@
 /// \param D The declaration to check.
 /// \param Message If non-null, this will be populated with the message from
 /// the availability attribute that is selected.
-/// \param ClassReceiver If we're checking the the method of a class message
+/// \param ClassReceiver If we're checking the method of a class message
 /// send, the class. Otherwise nullptr.
 static std::pair
 ShouldDiagnoseAvailabilityOfDecl(Sema &S, const NamedDecl *D,
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -112,7 +112,7 @@
 // the specified module, meaning clang won't build the specified module. This is
 // useful in a number of situations, for instance, when building a library that
 // vends a module map, one might want to avoid hitting intermediate build
-// products containing the the module map or avoid finding the system installed
+// products containimg the module map or avoid finding the system installed
 // modulemap for that library.
 static bool isForModuleBuilding(Module *M, StringRef CurrentModule,
 StringRef ModuleName) {
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -4961,7 +4961,7 @@
 
   auto *GEP = cast(GEPVal);
   assert(GEP->getPointerOperand() == BasePtr &&
- "BasePtr must be the the base of the GEP.");
+ "BasePtr must be the base of the GEP.");
   assert(GEP->isInBounds() && "Expected inbounds GEP");
 
   auto *IntPtrTy = DL.getIntPtrType(GEP->getPointerOperandType());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -50,7 +50,7 @@
 /// Selects the (empty) range [B,B) when \p Selector selects the range [B,E).
 RangeSelector before(RangeSelector Selector);
 
-/// Selects the the point immediately following \p Selector. That is, the
+/// Selects the point immediately following \p Selector. That is, the
 /// (empty) range [E,E), when \p Selector selects either
 /// * the CharRange [B,E) or
 /// * the TokenRange [B,E'] where the token at E' spans the range [E',E).
Index: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
===
--- clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
+++ clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
@@ -20,7 +20,7 @@
 /// Provides notifications for file changes in a directory.
 ///
 /// Invokes client-provided function on every filesystem event in the watched
-/// directory. Initially the the watched directory is scanned and for every file
+/// directory. Initially the watched directory is scanned and for every file
 /// found, an event is synthesized as if the file was added.
 ///
 /// This is not a general purpose directory monitoring tool - list of
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -308,7 +308,7 @@
 
 /// Matches statements that are (transitively) expanded from the named macro.
 /// Does not match if only part of the statement is expanded from that macro or
-/// if different parts of the the statement are expanded from different
+/// if different parts of the statement are expanded from different
 /// appearances of the macro.
 AST_POLYMORPHIC_MATCHER_P(isExpandedFromMacro,
   AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
Ind

[clang] 8f77dc4 - [clang] NFC: Fix trivial typo in comments and document

[Shivam Gupta via cfe-commits]

Author: Kazuaki Ishizaki
Date: 2021-09-04T12:59:42+05:30
New Revision: 8f77dc459e31aad6daab89a124fa92067916274c

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

LOG: [clang] NFC: Fix trivial typo in comments and document

`the the` -> `the`

Reviewed By: xgupta

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

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/AST/ASTStructuralEquivalence.h
clang/include/clang/AST/ComparisonCategories.h
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
clang/include/clang/Tooling/Transformer/RangeSelector.h
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Sema/SemaAvailability.cpp

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index 73b8a4fcafe51..016531043a831 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -5038,7 +5038,7 @@ Narrowing Matchers
 MatcherStmt>isExpandedFromMacrostd::string 
MacroName
 Matches 
statements that are (transitively) expanded from the named macro.
 Does not match if only part of the statement is expanded from that macro or
-if 
diff erent parts of the the statement are expanded from 
diff erent
+if 
diff erent parts of the statement are expanded from 
diff erent
 appearances of the macro.
 
 

diff  --git a/clang/include/clang/AST/ASTStructuralEquivalence.h 
b/clang/include/clang/AST/ASTStructuralEquivalence.h
index c958a16aba213..0b9e9e4e605fd 100644
--- a/clang/include/clang/AST/ASTStructuralEquivalence.h
+++ b/clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -117,7 +117,7 @@ struct StructuralEquivalenceContext {
   static llvm::Optional
   findUntaggedStructOrUnionIndex(RecordDecl *Anon);
 
-  // If ErrorOnTagTypeMismatch is set, return the the error, otherwise get the
+  // If ErrorOnTagTypeMismatch is set, return the error, otherwise get the
   // relevant warning for the input error diagnostic.
   unsigned getApplicableDiagnostic(unsigned ErrorDiagnostic);
 

diff  --git a/clang/include/clang/AST/ComparisonCategories.h 
b/clang/include/clang/AST/ComparisonCategories.h
index fb648b322b61c..7b73b582fe2a6 100644
--- a/clang/include/clang/AST/ComparisonCategories.h
+++ b/clang/include/clang/AST/ComparisonCategories.h
@@ -145,7 +145,7 @@ class ComparisonCategoryInfo {
 return Kind == CCK::PartialOrdering;
   }
 
-  /// Converts the specified result kind into the the correct result kind
+  /// Converts the specified result kind into the correct result kind
   /// for this category. Specifically it lowers strong equality results to
   /// weak equivalence if needed.
   ComparisonCategoryResult makeWeakResult(ComparisonCategoryResult Res) const {

diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index fdf032ce63923..3b78dc87684f1 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -2005,13 +2005,13 @@ class OMPUpdateClause final
 return IsExtended ? 2 : 0;
   }
 
-  /// Sets the the location of '(' in clause for 'depobj' directive.
+  /// Sets the location of '(' in clause for 'depobj' directive.
   void setLParenLoc(SourceLocation Loc) {
 assert(IsExtended && "Expected extended clause.");
 *getTrailingObjects() = Loc;
   }
 
-  /// Sets the the location of '(' in clause for 'depobj' directive.
+  /// Sets the location of '(' in clause for 'depobj' directive.
   void setArgumentLoc(SourceLocation Loc) {
 assert(IsExtended && "Expected extended clause.");
 *std::next(getTrailingObjects(), 1) = Loc;
@@ -2085,13 +2085,13 @@ class OMPUpdateClause final
 return const_child_range(const_child_iterator(), const_child_iterator());
   }
 
-  /// Gets the the location of '(' in clause for 'depobj' directive.
+  /// Gets the location of '(' in clause for 'depobj' directive.
   SourceLocation getLParenLoc() const {
 assert(IsExtended && "Expected extended clause.");
 return *getTrailingObjects();
   }
 
-  /// Gets the the location of argument in clause for 'depobj' directive.
+  /// Gets the location of argument in clause for 'depobj' directive.
   SourceLocation getArgumentLoc() const {
 assert(IsExtended && "Expected extended clause.");
 return *std::next(getTrailingObjects(), 1);

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 25b4499146e90..2baa17b59da83 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatche