How do you access the body of a template function in the AST?

Clang Frontend
1

Given this source file:

template <int N>
bool fn()
{
    return N > 1;
}

clang -Xclang -ast-dump -fsyntax-only tmp.cpp reports:

TranslationUnitDecl 0x2a354087370 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x2a3540878b0 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x2a3540875c0 '__int128'
|-TypedefDecl 0x2a354087918 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x2a3540875e0 'unsigned __int128'
|-CXXRecordDecl 0x2a354087968 <<invalid sloc>> <invalid sloc> implicit class type_info
| `-TypeVisibilityAttr 0x2a354087a30 <<invalid sloc>> Implicit Default
|-TypedefDecl 0x2a354087a88 <<invalid sloc>> <invalid sloc> implicit size_t 'unsigned long long'
| `-BuiltinType 0x2a354087540 'unsigned long long'
|-TypedefDecl 0x2a354087b18 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x2a354087ae0 'char *'
|   `-BuiltinType 0x2a354087400 'char'
|-TypedefDecl 0x2a354087b78 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'char *'
| `-PointerType 0x2a354087ae0 'char *'
|   `-BuiltinType 0x2a354087400 'char'
`-FunctionTemplateDecl 0x2a354087d48 <c:\tmp\tmp.cpp:1:1, line:2:9> col:6 fn
  |-NonTypeTemplateParmDecl 0x2a354087be0 <line:1:11, col:15> col:15 'int' N
  `-FunctionDecl 0x2a354087ca0 <line:2:1, col:9> col:6 fn '_Bool (void)'
    `-<<<NULL>>>

Why is the body NULL? Where is it represented in the AST?

2

That looks like delayed template instantiation behavior (the default you get on Windows, because thatā€™s how MSVC behaves). You have to instantiate the template in order to generate its body in that case (or disable delayed template parsing with -fno-delayed-template-parsing).

3

OK, yes, that does it. Thanks

4

Doesnā€™t this imply that clang-tidy checks are always missing bits when run on Windows?

5

Not usually. A lot of the tests specify -fno-delayed-template-parsing or instantiate the templates somehow. However, we do run into the issue where someone forgets to do that and build bots break with some degree of regularity. (The same happens in Clang when writing SemaCXX template tests.)

6

I think Iā€™ll put a mention of this in the clang-tidy contributing docs.

1 Like
7

IMO itā€™s high time to reconsider the default setting for this flag. This MSVC compatibility is regularly causing more issues than it solves. It most recently came up in this code review in 2021, which as I understand it did not land:
https://reviews.llvm.org/D103772

Previously it was mentioned in an issue in 2019:

https://github.com/llvm/llvm-project/issues/42377

As I understand it, flipping the default here would be inconsistent with MSVC. Some people look at this issue and say, ā€œMSVC uses /Zc:twoPhase- by default, so clang-cl should follow MSVCā€. However, Clangā€™s -fdelayed-template-parsing setting isnā€™t really the same thing as /Zc:twoPhase-, and I think if we can address the build failure from last time, users will get more value from a more C+Ā±conforming default compiler behavior. So, I think we should go ahead and make the change anyway.

8

I think an RFC to reconsider that behavior would be interesting, because I agree, itā€™s a disruptive behavior to have on by default. However, I think we need be very cautious about breaking system headers. Iā€™m not super worried about most Win32 headers given that theyā€™re generally C headers. But Iā€™m definitely worried about the massive amount of e.g., MFC (and other more C++ heavy) code that still exists.

(A secondary concern would be silent ABI breakage, but Iā€™m not certain changing this default should impact ABI at all ā€“ if the template was being instantiated, ABI wonā€™t be changed, and if the template was never instantiated, it was never ODR-used to begin with and so there should be no ABI impact either.)

9

Is this a Windows-only problem though? We use Linux and experience many situations where clang-tidy will not catch issues simply due to templates not being instantiated. From this thread my understanding is that ā€œdelayed template instantiationā€ is a feature enabled by default only on Windows; why does it happen on Linux too? For example:

template <typename T>
T foo(int x)
{
    return T(x);  // Expect: google-readability-casting
}

int main()
{
    float x = foo<float>(123);  // Only when the function is instantiated we get the warning
}
10

However, I think we need be very cautious about breaking system headers. ā€¦ But Iā€™m definitely worried about the massive amount of e.g., MFC (and other more C++ heavy) code that still exists.

Iā€™m optimistic that MFC, ATL, and other C++ template libraries already work without delayed template parsing because Microsoft added the /permissive- flag to new VS projects by default a long time ago. That means that most templates written by Microsoft should parse correctly where they are written, and not later at the point of instantiation.

Thereā€™s always some user somewhere that relies on every compiler implementation detail, but I feel pretty confident that we can find a way to make this flag flip. Itā€™s certainly release noteworthy.

Re: ABI, yep, this should be pretty safe.

Is this a Windows-only problem though? We use Linux and experience many situations where clang-tidy will not catch issues simply due to templates not being instantiated.

I think what you describe is the expected behavior.

1 Like
11

There are also a significant amount of warnings that are only possible once expression is instantiated. I donā€™t know much about Tidy, but in the CFE itself we have a bunch that are like that.

For example:

return T(x);

might just be a constructor call, at which point it makes little sense to diagnose it as a ā€˜readability castā€™, IMO.

12

I share that optimism, but Iā€™d want to know how we fare against the oldest headers we expect to support rather than the newest. But if the newest work well but previous versions are a problem, this at least sets a timer for when we can enable the functionality by default (or it may inform us that we can never switch the defaults, but that would be a surprise to me).

+1; if the breakage doesnā€™t happen in a system or ā€œcommonly used 3rd party headerā€ (whatever that means to folks), I think itā€™s worth trying. And I agree, warning folks weā€™re thinking about doing this switch is something we definitely should do. Perhaps we could run an RFC by community and at least get the release note into Clang 15 so that we can flip defaults (if possible) in Clang 16?

Some checks elect to not diagnose at all unless the template is instantiated; you found one of them: https://github.com/llvm/llvm-project/blob/5bbe50148f3b515c170be22209395b72890f5b8c/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp#L31

1 Like