Spurious error with -fincremental-extensions in clang 16

[cov-tprince]

We recently started the process to upgrade our frontend to Clang 16. However, we discovered a fairly serious regression when using -fincremental-extensions:

$ cat bad.cpp
struct S {
  ~S();
};
S::~S() {}
$ /opt/pkg/clang-16/bin/clang++ -c bad.cpp
$ /opt/pkg/clang-16/bin/clang++ -c -Xclang -fincremental-extensions bad.cpp
bad.cpp:4:4: error: call to non-static member function without an object argument
S::~S() {}
~~~^
1 error generated.
$ 

Godbolt link for the above:
https://godbolt.org/z/89jfE3YWP

It's probably worth a little explanation for why we're using this obscure feature--this is an unfortunate wart relating to constraints imposed by our internal AST. When a type contains a member function template that implements a special member function (e.g. operator=) we attempt to instantiate those methods when doing AST translation even if they are not explicitly required by the translation unit. Any errors generated by this instantiation are suppressed and discarded.

However, because we're doing this instantiation after the ASTUnit is fully loaded, clang would normally tear down many of the resources needed to do template instantiations, leading to various memory errors/crashes. We use -fincremental-extensions to suppress this teardown.

In the future, we hope to do these instantiations inline with the initial ASTUnit loading which would allow us to remove our dependency on -fincremental-extensions.

We are currently in the process of trying to identify what commit or set of commits introduced this regression.

[cov-tprince]

It looks like -fincremental-extensions was introduced by clang 16 in commit dc48893. We aren't actually using -fincremental-extensions but rather the (now deprecated) Preprocessor::enableIncrementalProcessing method which sets the same LangOpt field.

[tahonermann]

This looks like intentional behavior.

To support your use case, I think it would make sense to restore the Preprocessor::IncrementalProcessing member so as to separate the ability to enable incremental processing from the incremental extensions behavior of disambiguating declarations and statements at global scope as statements.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[vgvassilev]

Thank you for the explanation. As part of avoiding the tear down process we are doing some extra work to support the major usecase for this flag - c++ repls.

There is a fix for the issue you are seeing which is pending review https://reviews.llvm.org/D148425

FWIW, we are considering consolidating the two flags and dropping the preprocessor one. Do you think we can find another solution for your use case?

[vgvassilev]

This should be resolved by 5a9abe8

Can we close the issue now?

[cov-tprince]

Can we close the issue now?

We ended up removing the need for the call to Preprocessor::enableIncrementalProcessing in our case, but I confirmed the reproducer is fixed in trunk via godbolt.