Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointless -Wundefined-internal warning from concept checks #61566

Open
timsong-cpp opened this issue Mar 20, 2023 · 6 comments
Open

Pointless -Wundefined-internal warning from concept checks #61566

timsong-cpp opened this issue Mar 20, 2023 · 6 comments
Labels
c++20 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer concepts C++20 concepts

Comments

@timsong-cpp
Copy link

timsong-cpp commented Mar 20, 2023

auto begin(auto&& r) {
    return r.begin();
}

template<class T>
concept Range = requires (T t) { ::begin(t); };

namespace {
    struct R {
        int* begin();
    };
}

static_assert(Range<R>);

Here, I'm just trying to check the concept; nothing is actually calling ::begin or R::begin. Yet clang warns:

<source>:10:14: warning: function '(anonymous namespace)::R::begin' has internal linkage but is not defined [-Wundefined-internal]
        int* begin();
             ^
<source>:2:14: note: used here
    return r.begin();
             ^
@EugeneZelenko EugeneZelenko added c++20 concepts C++20 concepts clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Mar 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Mar 20, 2023

@llvm/issue-subscribers-c-20

@shafik
Copy link
Collaborator

shafik commented Mar 21, 2023

Note, gcc also produces a diagnostic for this: https://godbolt.org/z/E7Yfv9css

I am not sure if this is expected though CC @erichkeane @royjacobson

@erichkeane
Copy link
Collaborator

This seems to just be a problem with our diagnostic, not concept specific. I'm guessing it isn't considering the type of 'used' in some way.

I wouldn't expect the 'internal linkage without definition' to be a problem if only used in a concept though, so I agree with Tim that this should likely not be sufficient to trigger it. However, I DO note that if you call t.begin inside the concept, it doesn't trigger the warning, so it is something to do with how we're evaluating the ::begin.

@cor3ntin
Copy link
Contributor

cor3ntin commented Jun 4, 2024

@erichkeane poking around, we are considering an ODR-used, either because requires constraints use the wrong sort of context somewhere, or because we do not consider the case of a potentially evaluated context nested in an unevaluated context

@erichkeane
Copy link
Collaborator

Ah, interesting! I wouldn't be surprised, we've noticed a few times already we had the context incorrect. I THINK we just fixed one recently.

@cor3ntin
Copy link
Contributor

cor3ntin commented Jun 7, 2024

This is going to be super tricky to "fix", if at all possible.

1 - check whether ::begin(t) is a valid expression. This happens in an unevaluated context
2 - ::begin has a deduced return type, we need to deduce it
3 - The function body of ::begin is instantiated - that body is not an unevaluated context
4 - This causes r.begin() to be odr used (it is a potentially evaluated expression)
5 - This triggers the warning to be emitted at the end of the TU

To silence the warning, we would need to track that the point of instantiation was in an unevaluated context.
But then, if the same instantiated function is used in a potentially evaluated context, we do want a warning emitted, so we also need a way to track that calling a function cause all the function in its body to require a definition...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer concepts C++20 concepts
Projects
Status: No status
Development

No branches or pull requests

6 participants