Revert "[Clang][Comments] Attach comments to decl even if preproc directives are in between (#88367)"

This reverts commit 9f04d75b2bd8ba83863db74ebe1a5c08cfc5815c.

There was post-commit feedback on

Revert "[Clang][Comments] Attach comments to decl even if preproc directives are in between (#88367)"

This reverts commit 9f04d75b2bd8ba83863db74ebe1a5c08cfc5815c.

There was post-commit feedback on the direction this PR took.

show more ...

[Clang][Comments] Attach comments to decl even if preproc directives are in between (#88367)

### Background

It's surprisingly common for C++ code in the wild to conditionally
show/hide declarati

[Clang][Comments] Attach comments to decl even if preproc directives are in between (#88367)

### Background

It's surprisingly common for C++ code in the wild to conditionally
show/hide declarations to Doxygen through the use of preprocessor
directives. One especially common version of this pattern is
demonstrated below:

```cpp
/// @brief Test comment
#ifdef DOXYGEN_BUILD_ENABLED
template<typename T>
#else
template <typename T>
typename std::enable_if<std::is_integral<T>::value>::type
#endif
void f() {}
```

There are more examples I've collected below to demonstrate usage of
this pattern:
- Example 1:
[Magnum](https://github.com/mosra/magnum/blob/8538610fa27e1db37070eaabe34f1e4e41648bab/src/Magnum/Resource.h#L117-L127)
- Example 2:
[libcds](https://github.com/khizmax/libcds/blob/9985d2a87feaa3e92532e28f4ab762a82855a49c/cds/container/michael_list_nogc.h#L36-L54)
- Example 3:
[rocPRIM](https://github.com/ROCm/rocPRIM/blob/609ae19565ff6a3499168b76a0be5652762e24f6/rocprim/include/rocprim/block/detail/block_reduce_raking_reduce.hpp#L60-L65)

From my research, it seems like the most common rationale for this
functionality is hiding difficult-to-parse code from Doxygen, especially
where template metaprogramming is concerned.

Currently, Clang does not support attaching comments to decls if there
are preprocessor comments between the comment and the decl. This is
enforced here:
https://github.com/llvm/llvm-project/blob/b6ebea7972cd05a8e4dcf0d5a54f2c793999995a/clang/lib/AST/ASTContext.cpp#L284-L287

Alongside preprocessor directives, any instance of `;{}#@` between a
comment and decl will cause the comment to not be attached to the decl.

#### Rationale

It would be nice for Clang-based documentation tools, such as
[hdoc](https://hdoc.io), to support code using this pattern. Users
expect to see comments attached to the relevant decl — even if there is
an `#ifdef` in the way — which Clang does not currently do.

#### History

Originally, commas were also in the list of "banned" characters, but
were removed in `b534d3a0ef69`
([link](https://github.com/llvm/llvm-project/commit/b534d3a0ef6970f5e42f10ba5cfcb562d8b184e1))
because availability macros often have commas in them. From my reading
of the code, it appears that the original intent of the code was to
exclude macros and decorators between comments and decls, possibly in an
attempt to properly attribute comments to macros (discussed further in
"Complications", below). There's some more discussion here:
https://reviews.llvm.org/D125061.

### Change

This modifies Clang comment parsing so that comments are attached to
subsequent declarations even if there are preprocessor directives
between the end of the comment and the start of the decl. Furthermore,
this change:

- Adds tests to verify that comments are attached to their associated
decls even if there are preprocessor directives in between
- Adds tests to verify that current behavior has not changed (i.e. use
of the other characters between comment and decl will result in the
comment not being attached to the decl)
- Updates existing `lit` tests which would otherwise break.

#### Complications

Clang [does not yet
support](https://github.com/llvm/llvm-project/issues/38206) attaching
doc comments to macros. Consequently, the change proposed in this RFC
affects cases where a doc comment attached to a macro is followed
immediately by a normal declaration. In these cases, the macro's doc
comments will be attached to the subsequent decl. Previously they would
be ignored because any preprocessor directives between a comment and a
decl would result in the comment not being attached to the decl. An
example of this is shown below.

```cpp
/// Doc comment for a function-like macro
/// @param n
/// A macro argument
#define custom_sqrt(n) __internal_sqrt(n)

int __internal_sqrt(int n) { return __builtin_sqrt(n); }

// NB: the doc comment for the custom_sqrt macro will actually be attached to __internal_sqrt!
```

There is a real instance of this problem in the Clang codebase, namely
here:
https://github.com/llvm/llvm-project/blob/be10070f91b86a6f126d2451852242bfcb2cd366/clang/lib/Headers/amxcomplexintrin.h#L65-L114

As part of this RFC, I've added a semicolon to break up the Clang
comment parsing so that the `-Wdocumentation` errors go away, but this
is a hack. The real solution is to fix Clang comment parsing so that doc
comments are properly attached to macros, however this would be a large
change that is outside of the scope of this RFC.

show more ...

[clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (#92654)

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the
instantiated method but

[clang][AST] Fix end location of DeclarationNameInfo on instantiated methods (#92654)

Fixes #71161

[D64087](https://reviews.llvm.org/D64087) updated some locations of the
instantiated method but forgot `DNLoc`.

`FunctionDecl::getNameInfo()` constructs a `DeclarationNameInfo` using
`Decl::Loc` as the beginning of the declaration name, and
`FunctionDecl::DNLoc` to compute the end of the declaration name. The
former was updated, but the latter was not, so
`DeclarationName::getSourceRange()` would return a range where the end
of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166

Co-authored-by: Alejandro Alvarez Ayllon <alejandro.alvarez@sonarsource.com>

show more ...

[NFC] [Decl] Introduce Decl::isFromExplicitGlobalModule

Introduce `Decl::isFromExplicitGlobalModule` to replace the
`D->getOwningModule() && D->getOwningModule()->isExplicitGlobalModule()`
pattern t

[NFC] [Decl] Introduce Decl::isFromExplicitGlobalModule

Introduce `Decl::isFromExplicitGlobalModule` to replace the
`D->getOwningModule() && D->getOwningModule()->isExplicitGlobalModule()`
pattern to save some typings.

show more ...

[clang][NFC] Refactor `clang::Linkage` (#71049)

This patch introduces a new enumerator `Invalid = 0`, shifting other enumerators by +1. Contrary to how it might sound, this actually affirms status q

[clang][NFC] Refactor `clang::Linkage` (#71049)

This patch introduces a new enumerator `Invalid = 0`, shifting other enumerators by +1. Contrary to how it might sound, this actually affirms status quo of how this enum is stored in `clang::Decl`:
```
/// If 0, we have not computed the linkage of this declaration.
/// Otherwise, it is the linkage + 1.
mutable unsigned CacheValidAndLinkage : 3;
```
This patch makes debuggers to not be mistaken about enumerator stored in this bit-field. It also converts `clang::Linkage` to a scoped enum.

show more ...

-fsanitize=function: fix MSVC hashing to sugared type (#66816)

Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:

```
sta

-fsanitize=function: fix MSVC hashing to sugared type (#66816)

Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:

```
static auto l() {}
int main() {
auto a = l;
a();
}
```
`clang --target=x86_64-pc-windows-msvc -fno-exceptions
-fsanitize=function -g -O0 -fuse-ld=lld -o test.exe test.cc`

produces:
```
test.cc:4:3: runtime error: call to function l through pointer to incorrect function type 'void (*)()'
```

show more ...

[clang] Update NumFunctionDeclBits for FunctionDeclBitfields

NumFunctionDeclBits is not updated when DeductionCandidateKind is
incremented.

Fixes https://github.com/llvm/llvm-project/issues/64171

[clang] Update NumFunctionDeclBits for FunctionDeclBitfields

NumFunctionDeclBits is not updated when DeductionCandidateKind is
incremented.

Fixes https://github.com/llvm/llvm-project/issues/64171

Reviewed By: cor3ntin, balazske, aaron.ballman

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

show more ...

[clang][test] Remove unused variable 'SM' (NFC)

/data/llvm-project/clang/unittests/AST/DeclTest.cpp:153:18: error: unused variable 'SM' [-Werror,-Wunused-variable]
SourceManager &SM = Ctx.getSourc

[clang][test] Remove unused variable 'SM' (NFC)

/data/llvm-project/clang/unittests/AST/DeclTest.cpp:153:18: error: unused variable 'SM' [-Werror,-Wunused-variable]
SourceManager &SM = Ctx.getSourceManager();
^
1 error generated.

show more ...

[clang] adds `conceptDecl` as an ASTMatcher

Closes #63934

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

[NFC] remove duplciated unittests for modules

There was an overlook to duplicate the unittests for modules. This patch
removes one of this duplication.

[clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

The purpose of this patch and follow-on patches is to ensure that
AttributeCommonInfos always have a syntax that is appropriate for

[clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

The purpose of this patch and follow-on patches is to ensure that
AttributeCommonInfos always have a syntax that is appropriate for
their kind (i.e. that it matches one of the entries in Attr.td).

The attribute-specific Create and CreateImplicit methods had four
overloads, based on their tail arguments:

(1) no extra arguments
(2) an AttributeCommonInfo
(3) a SourceRange
(4) a SourceRange, a syntax, and (where necessary) a spelling

When (4) had a spelling argument, it defaulted to
SpellingNotCalculated.

One disadvantage of this was that (1) and (3) zero-initialized
the syntax field of the AttributeCommonInfo, which corresponds
to AS_GNU. But AS_GNU isn't always listed as a possibility
in Attr.td.

This patch therefore removes (1) and (3) and instead provides
the same functionality using default arguments on (4) (a bit
like the existing default argument for the spelling).
The default syntax is taken from the attribute's first valid
spelling.

Doing that raises the question: what should happen for attributes
like AlignNatural and CUDAInvalidTarget that are only ever created
implicitly, and so have no source-code manifestation at all?
The patch adds a new AS_Implicit "syntax" for that case.
The patch also removes the syntax argument for these attributes,
since the syntax must always be AS_Implicit.

For similar reasons, the patch removes the syntax argument if
there is exactly one valid spelling.

Doing this means that AttributeCommonInfo no longer needs the
single-argument constructors. It is always given a syntax instead.

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

show more ...

Fix the buildbots after 636dd1e8a1782e22f9bdee640428ed5c50a4a4f2

This addresses issues found by:
http://45.33.8.238/linux/103908/step_4.txt
https://lab.llvm.org/buildbot/#/builders/216/builds/19607

Fix the buildbots after 636dd1e8a1782e22f9bdee640428ed5c50a4a4f2

This addresses issues found by:
http://45.33.8.238/linux/103908/step_4.txt
https://lab.llvm.org/buildbot/#/builders/216/builds/19607
and others

show more ...

[C++20] [Modules] Treat module linkage as formal linkage only

Close https://github.com/llvm/llvm-project/issues/61321

There are two linkage modes in clang now: one for internal linkage and
one for

[C++20] [Modules] Treat module linkage as formal linkage only

Close https://github.com/llvm/llvm-project/issues/61321

There are two linkage modes in clang now: one for internal linkage and
one for formal linkage. The internal linkage is for the implementation
details and the formal linkage is for the consistency with the C++
standard.

Since we previously implemented the strong ownership for modules, the
module linkage is not meaningful for linkers any more. So the module
linkage should only be used for formal linkage.

show more ...

[Modules] Refactor modules-ts tests to use standard c++ modules

We're going to remove the support for modules-ts. But there are a lot of
tests which uses -fmodules-ts. We shouldn't remove them simpl

[Modules] Refactor modules-ts tests to use standard c++ modules

We're going to remove the support for modules-ts. But there are a lot of
tests which uses -fmodules-ts. We shouldn't remove them simply. This
patch refactor these tests to use standard c++ modules.

show more ...

[clang][TemplateBase] Add IsDefaulted bit to TemplateArgument

**Summary**

This patch adds a `IsDefaulted` field to `clang::TemplateArgument`.

To prevent memory footprint increase we still 1 bit fr

[clang][TemplateBase] Add IsDefaulted bit to TemplateArgument

**Summary**

This patch adds a `IsDefaulted` field to `clang::TemplateArgument`.

To prevent memory footprint increase we still 1 bit from `ArgKind`.

**Changes**

1. `getIsDefaulted`/`setIsDefaulted` to allow clients to communicate
an argument's defaulted-ness to the TypePrinter
2. The `TemplateArgument` properties description had to be changed
to make sure we correctly mark the defaulted-ness of arguments
that came from a deserialized AST (caught by the HLSL test-suite)
3. The `TemplateArgument` constructors now accept a `IsDefaulted`
parameter to simplify construction from the tablegen description.
Though if people don't want to clutter the constructors we can
instead call `setIsDefaulted` from tablegen
4. When `clang::Sema` checks the template arguments against template
parameters we now call `setIsDefaulted`. This makes sure that
whenever a specialization decl gets constructed, the defaulted-ness
of the associated `TemplateArgument`s has already been deduced.
This preserves the immutability of `TemplateArgumentList`s

**Background**

In LLDB we construct ASTs from debug-info and hand it to clang
to perform actions such as printing/formatting a typenames.
Some debug formats, specifically DWARF, may only encode information
about class template instantiations, losing the structure of the generic
class definition. However, the `clang::TypePrinter` needs a properly
constructed `ClassTemplateDecl` with generic default argument decls
to be able to deduce whether a `ClassTemplateSpecializationDecl` was
instantiatiated with `TemplateArgument`s that correspond to the
defaults. LLDB does know whether a particular template argument was
defaulted, but can't currently tell clang about it.

This patch allows LLDB to set the defaulted-ness of a `TemplateArgument`
and thus benefit more from `clang::TypePrinter`.

See discussion in https://reviews.llvm.org/D140423

**Testing**

* Added unit-test
* LLDB/clang/llvm test-suite passes

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

show more ...

[test] Split out Annotations from `TestingSupport`

The Annotations helper class does not have a gtest or gmock dependency, but because it's bundled with the rest of TestingSupport, it gets one. By s

[test] Split out Annotations from `TestingSupport`

The Annotations helper class does not have a gtest or gmock dependency, but because it's bundled with the rest of TestingSupport, it gets one. By splitting it out, a target can use it without being forced to use LLVM's copy of gtest.

Reviewed By: GMNGeoffrey, sammccall, gribozavr2

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

show more ...

[NFC][Clang][ASTTests] Use ASSERT instead of EXPECT for nullptr checks

This avoids basically guaranteed crashes when the check fails.

[C++20] [Modules] Attach implicitly declared allocation funcitons to
global module fragment

[basic.stc.dynamic.general]p2 says:
> The library provides default definitions for the global allocation
>

[C++20] [Modules] Attach implicitly declared allocation funcitons to
global module fragment

[basic.stc.dynamic.general]p2 says:
> The library provides default definitions for the global allocation
> and deallocation functions. Some global allocation and
> deallocation
> functions are replaceable ([new.delete]); these are attached to
> the global module ([module.unit]).

But we didn't take this before and the implicitly generated functions
will live in the module purview if we're compiling a module unit. This
is bad since the owning module will affect the linkage of the
declarations. This patch addresses this.

Closes https://github.com/llvm/llvm-project/issues/58560

show more ...

[clang] initialize type qualifiers for FunctionNoProtoType

When initializing FunctionNoProtoType types, zero out the type
qualifiers. This will ensure the ODR hash remains stable as it
hashes the va

[clang] initialize type qualifiers for FunctionNoProtoType

When initializing FunctionNoProtoType types, zero out the type
qualifiers. This will ensure the ODR hash remains stable as it
hashes the values for these qualifiers for all function types.

Reviewed By: aaron.ballman

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

show more ...

[C++20] [Modules] Make member functions with a in-class definition in HU implicitly inline

According to [dcl.inline]p7/note4,

> In the global module, a function defined within a class definition is

[C++20] [Modules] Make member functions with a in-class definition in HU implicitly inline

According to [dcl.inline]p7/note4,

> In the global module, a function defined within a class definition is
> implicitly inline.

And the declarations in the header unit are attached to the global
module fragment. So the function defined within a class definition in
header units should be implicitly inline too.

This fixes https://github.com/llvm/llvm-project/issues/57571.

show more ...

[NFC] Add unittest for inline functions in modules

[AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

Closing https://github.com/llvm/llvm-project/issues/56826.

The root cause for pr56826 is: when we collect the

[AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

Closing https://github.com/llvm/llvm-project/issues/56826.

The root cause for pr56826 is: when we collect the template args for the
friend, we need to judge if the friend lives in file context. However,
if the friend lives in ExportDecl lexically, the judgement here is
invalid.

The solution is easy. We should judge the non transparent context and
the ExportDecl is transparent context. So the solution should be good.

A main concern may be the patch doesn't handle all the places of the
same defect. I think it might not be bad since the patch itself should
be innocent.

Reviewed By: erichkeane

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

show more ...

[C++20] [Modules] Make the linkage consistent for class template and its
specialization

Previously in D120397, we've handled the linkage for function template
and its specialization. But we forgot t

[C++20] [Modules] Make the linkage consistent for class template and its
specialization

Previously in D120397, we've handled the linkage for function template
and its specialization. But we forgot to handle it for class templates
and their specialization. So we make it in the patch with the similar
approach.

show more ...

[C++20][Modules] Limit ModuleInternalLinkage to modules-ts.

At present, we are generating wrong code for C++20 modules entities which
should have internal linkage. This is because we are assigning

[C++20][Modules] Limit ModuleInternalLinkage to modules-ts.

At present, we are generating wrong code for C++20 modules entities which
should have internal linkage. This is because we are assigning
'ModuleInternalLinkage' unconditionally to such entities. However this mode
is only applicable to the modules-ts.

This change makes the special linkage mode conditional on fmodules-ts and
adds a unit test to verify that we generate the correct linkage.

Currently, static variables and functions in module purview are emitted into
object files as external. On some platforms, lambdas are emitted as global
weak defintions (on Windows this causes a mangler crash).

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

show more ...

[clang][NFC] Fix warning of integer comparison

```
warning: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Wsign-compare]
```

Fix https://lab.llvm.org/buildbot/#

[clang][NFC] Fix warning of integer comparison

```
warning: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Wsign-compare]
```

Fix https://lab.llvm.org/buildbot/#/builders/57/builds/16220.

show more ...