[clang] SemaFunctionEffects: When verifying a function, ignore any trailing 'requires' clause.

[dougsonos]

Clearly there's an omission here, that a trailing requires clause in a called function is being subject to effect analysis.

But despite many hours of effort I haven't been able to create a self-contained reproducer. My best effort:

#include <valarray>
#include <type_traits>
#include <expected>

template <class _Tp, _Tp __v>
struct integral_constant {
  static constexpr _Tp value = __v;
  typedef _Tp value_type;
  typedef integral_constant type;
  constexpr operator value_type() const noexcept { return value; }
  constexpr value_type operator()() const noexcept { return value; }
};

template <typename T>
struct IsInt : public integral_constant<bool, false> {};

template <>
struct IsInt<int> : public integral_constant<bool, true> {};

template <typename T>
inline constexpr bool IsInt_V = IsInt<T>::value;

template <typename T, typename E>
struct ExpectedLike {
	ExpectedLike() = default;
	
	constexpr ExpectedLike(const ExpectedLike&)
// 	requires(IsInt_V<T> && IsInt_V<E> && IsInt_V<T>)
    requires(std::is_copy_constructible_v<T> && std::is_copy_constructible_v<E> && std::is_trivially_copy_constructible_v<T> &&
             std::is_trivially_copy_constructible_v<E>)

	= default;
	
};

void nb_xx() [[clang::nonblocking]]
{
	ExpectedLike<int, int> a;
	auto b = a;
// No warning. I don't know why.

	std::expected<int, int> c;
	std::expected<int, int> d = c;
// ^^ warning: function with 'nonblocking' attribute must not call non-'nonblocking' constructor 'std::expected<int, int>::expected' [-Wfunction-effects]
}

The two elements of the mystery are:

• why doesn't my ExpectedLike<T, E> copy constructor reproduce the behavior of std::expected<T, E>?
• why does reproduction with std::expected depend on the global operator && in <valarray>?

I've verified that the change to SemaFunctionEffects fixes the issue, but I'd sure like to be able to construct a self-contained test.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

Clearly there's an omission here, that a trailing requires clause in a called function is being subject to effect analysis.

But despite many hours of effort I haven't been able to create a self-contained reproducer. My best effort:

#include <valarray>
#include <type_traits>
#include <expected>

template <class _Tp, _Tp __v>
struct integral_constant {
  static constexpr _Tp value = __v;
  typedef _Tp value_type;
  typedef integral_constant type;
  constexpr operator value_type() const noexcept { return value; }
  constexpr value_type operator()() const noexcept { return value; }
};

template <typename T>
struct IsInt : public integral_constant<bool, false> {};

template <>
struct IsInt<int> : public integral_constant<bool, true> {};

template <typename T>
inline constexpr bool IsInt_V = IsInt<T>::value;

template <typename T, typename E>
struct ExpectedLike {
	ExpectedLike() = default;
	
	constexpr ExpectedLike(const ExpectedLike&)
// 	requires(IsInt_V<T> && IsInt_V<E> && IsInt_V<T>)
    requires(std::is_copy_constructible_v<T> && std::is_copy_constructible_v<E> && std::is_trivially_copy_constructible_v<T> &&
             std::is_trivially_copy_constructible_v<E>)

	= default;
	
};

void nb_xx() [[clang::nonblocking]]
{
	ExpectedLike<int, int> a;
	auto b = a;
// No warning. I don't know why.

	std::expected<int, int> c;
	std::expected<int, int> d = c;
// ^^ warning: function with 'nonblocking' attribute must not call non-'nonblocking' constructor 'std::expected<int, int>::expected' [-Wfunction-effects]
}

The two elements of the mystery are:

• why doesn't my ExpectedLike<T, E> copy constructor reproduce the behavior of std::expected<T, E>?
• why does reproduction with std::expected depend on the global operator && in <valarray>?

I've verified that the change to SemaFunctionEffects fixes the issue, but I'd sure like to be able to construct a self-contained test.

---

Full diff: https://github.com/llvm/llvm-project/pull/114266.diff

1 Files Affected:

• (modified) clang/lib/Sema/SemaFunctionEffects.cpp (+10)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 3fa326db06ee41..f7ff8b92d8a929 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -971,6 +971,7 @@ class Analyzer {
     PendingFunctionAnalysis &CurrentFunction;
     CallableInfo &CurrentCaller;
     ViolationSite VSite;
+    const Expr *TrailingRequiresClause = nullptr;
 
     FunctionBodyASTVisitor(Analyzer &Outer,
                            PendingFunctionAnalysis &CurrentFunction,
@@ -985,6 +986,9 @@ class Analyzer {
       if (auto *Dtor = dyn_cast<CXXDestructorDecl>(CurrentCaller.CDecl))
         followDestructor(dyn_cast<CXXRecordDecl>(Dtor->getParent()), Dtor);
 
+      if (auto *FD = dyn_cast<FunctionDecl>(CurrentCaller.CDecl))
+        TrailingRequiresClause = FD->getTrailingRequiresClause();
+
       // Do an AST traversal of the function/block body
       TraverseDecl(const_cast<Decl *>(CurrentCaller.CDecl));
     }
@@ -1259,6 +1263,12 @@ class Analyzer {
       return true;
     }
 
+    bool TraverseStmt(Stmt *Statement) {
+      if (Statement != TrailingRequiresClause)
+        return Base::TraverseStmt(Statement);
+      return true;
+    }
+
     bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
       ViolationSite PrevVS = VSite;
       if (Init->isAnyMemberInitializer())

[cjappl]

@dougsonos - I was recently made aware of C-reduce, which takes some code, how to compile it, and an "interestingness test" and eventually spits out the smallest subset of things that are interesting.

https://www.mikeash.com/pyblog/friday-qa-2018-06-29-debugging-with-c-reduce.html

Could be useful in your hunt for an example/test.

This is something the clang frontend folks recommend using when they get compiler bugs (https://shafik.github.io/c++/llvm/2024/10/17/triaging-clang-fronend-bugs.html, cmd+f "reduce")

[dougsonos]

@dougsonos - I was recently made aware of C-reduce, which takes some code, how to compile it, and an "interestingness test" and eventually spits out the smallest subset of things that are interesting.

https://www.mikeash.com/pyblog/friday-qa-2018-06-29-debugging-with-c-reduce.html

Could be useful in your hunt for an example/test.

This is something the clang frontend folks recommend using when they get compiler bugs (https://shafik.github.io/c++/llvm/2024/10/17/triaging-clang-fronend-bugs.html, cmd+f "reduce")

Yeah, I tried creduce earlier this morning when the reproducer was 120,000 lines :-P It ran for maybe half an hour and gobbled up 20 GB of disk space so I cancelled and worked out the above by hand. I'm pretty sure the first missing ingredient has to do with the space-saving optimizations in __expected_base, and that might also be what's triggering valarray's operator&&...

[dougsonos]

@dougsonos - I was recently made aware of C-reduce, which takes some code, how to compile it, and an "interestingness test" and eventually spits out the smallest subset of things that are interesting.
https://www.mikeash.com/pyblog/friday-qa-2018-06-29-debugging-with-c-reduce.html
Could be useful in your hunt for an example/test.
This is something the clang frontend folks recommend using when they get compiler bugs (https://shafik.github.io/c++/llvm/2024/10/17/triaging-clang-fronend-bugs.html, cmd+f "reduce")

Yeah, I tried creduce earlier this morning when the reproducer was 120,000 lines :-P It ran for maybe half an hour and gobbled up 20 GB of disk space so I cancelled and worked out the above by hand. I'm pretty sure the first missing ingredient has to do with the space-saving optimizations in __expected_base, and that might also be what's triggering valarray's operator&&...

but thanks for the nudge -- creduce is making progress with this more minimal starting point!

[Sirraide]

The two elements of the mystery are:

• why doesn't my ExpectedLike<T, E> copy constructor reproduce the behavior of std::expected<T, E>?
• why does reproduction with std::expected depend on the global operator && in <valarray>?

Yeah, that sounds pretty horrible to reduce. Dumping the AST to figure out what’s going on might be worth a try, but I’m not sure this would actually help too much...

[Sirraide]

Also, @dougsonos, can you make a godbolt link for this? I’m trying to reproduce it, but compiling the code you provided doesn’t result in any diagnostics for me, but I might be missing a flag.

[dougsonos]

Also, @dougsonos, can you make a godbolt link for this? I’m trying to reproduce it, but compiling the code you provided doesn’t result in any diagnostics for me, but I might be missing a flag.

Yeah, I can't repro in godbolt either. This shows that all the effect analysis is in the clang-20 trunk on godbolt though:
https://godbolt.org/z/rP1E5ebhG

That makes me think that the issue may be specific to the um, older, version of libc++ that I've been testing against.

I'm pretty sure it has something to do with type traits in the requires() clause of the std::expected copy constructor, interacting badly with a global operator&& in <valarray>.

What I found impossible about creduce is that at some point it started ignoring requirements like the requires clause and collapsing all the templates.

Thanks for taking a look.

[dougsonos]

I spent a bit more time trying to extract a reduction from libc++ today but failed. Also I noted that my "older" libc++ has no suspicious diffs with the one on main. Haven't quite figured out how to build it locally.

Here are the diagnostics that (fortunately) were enough to devise the fix in this PR:

...../nonblocking-wip.cpp:54:30: warning: function with 'nonblocking' attribute must not call non-'nonblocking' constructor 'std::expected<int, int>::expected' [-Wfunction-effects]
   54 |  std::expected<int, int> d = c;
      |                              ^
...../usr/include/c++/v1/__expected/expected.h:483:14: note: function pointer cannot be inferred 'nonblocking'
  483 |     requires(is_copy_constructible_v<_Tp> && is_copy_constructible_v<_Err> && is_trivially_copy_constructible_v<_Tp> &&
      |              ^
.....//nonblocking-wip.cpp:54:30: note: in template expansion here
   54 |  std::expected<int, int> d = c;
      |                              ^

That showed me that the requires clause is being traversed (inappropriately) and doing something that looks like an indirect call (i.e. through a function pointer). Underneath is_copy_constructible_v is a builtin function __is_constructible(), so the indirect call isn't surprising (though I can't say I 100% understand yet).

[dougsonos]

I was able to (manually) extract a reduction from libc++ and write a test that exposed the issue / verified that it's fixed. It's also here: https://godbolt.org/z/9absooo6G

[cjappl]

for whatever it's worth - LGTM. Leaving to more experienced heads in this area to actually approve.

Good sleuthing!

[Sirraide]

is a builtin function __is_constructible()

Technically a type trait not a function—those are different because they take types as arguments, and there are separate expressions to represent them (UnaryExprOrTypeTraitExpr, TypeTraitExpr, and a few more); maybe we’re somehow not accounting for those?

Do we have tests for effect analysis that use builtin type traits?

[Sirraide]

Now that we have a test, this definitely lgtm. We should still look into the type traits thing, but that can be a separate pr if it turns out that any changes are necessary to account for them.

[Sirraide]

I was able to (manually) extract a reduction from libc++

Also, thanks for reducing this!

[dougsonos]

is a builtin function __is_constructible()

Technically a type trait not a function—those are different because they take types as arguments, and there are separate expressions to represent them (UnaryExprOrTypeTraitExpr, TypeTraitExpr, and a few more); maybe we’re somehow not accounting for those?

Do we have tests for effect analysis that use builtin type traits?

Thanks, I didn't know that these were type traits, they looked like special builtin functions to me, and that ignorance may be reflected in some of the diagnostics.

Generally expressions are ignored except CallExpr IIRC.

I'll look into this further. Thanks for the review here.