Skip to content

[Clang] Remove the special-casing for RequiresExprBodyDecl in BuildResolvedCallExpr() after fd87d765c0 #111277

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

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Oct 6, 2024

The special-casing for RequiresExprBodyDecl caused a regression, as reported in #110785.

The original fix for #84020 has been superseded by fd87d76, which establishes a DependentScopeDeclRefExpr instead of a CXXDependentScopeMemberExpr for the case in issue. So the spurious diagnostic in #84020 would no longer occur.

This also merges the test for #84020 together with that for #110785 into clang/test/SemaTemplate/instantiate-requires-expr.cpp.

No release note because I think this merits a backport.

Fixes #110785

@zyn0217 zyn0217 force-pushed the regression-110785 branch from d48512e to 9956038 Compare October 6, 2024 05:05
@zyn0217 zyn0217 marked this pull request as ready for review October 6, 2024 05:33
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

The special-casing for RequiresExprBodyDecl caused a regression, as reported in #110785.

This also merged the test for #84020 together with that of #110785 into clang/test/SemaTemplate/instantiate-requires-expr.cpp.

No release note because I think this merits a backport.

Fixes #110785


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

4 Files Affected:

  • (modified) clang/lib/Sema/SemaExpr.cpp (+1-2)
  • (modified) clang/lib/Sema/TreeTransform.h (+1-1)
  • (removed) clang/test/SemaCXX/PR84020.cpp (-23)
  • (modified) clang/test/SemaTemplate/instantiate-requires-expr.cpp (+31)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ae7bcedfb28c73..959f0739f03fb9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6963,8 +6963,7 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
   }
 
   if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(FDecl))
-    if (!isa<RequiresExprBodyDecl>(CurContext) &&
-        Method->isImplicitObjectMemberFunction())
+    if (Method->isImplicitObjectMemberFunction())
       return ExprError(Diag(LParenLoc, diag::err_member_call_without_object)
                        << Fn->getSourceRange() << 0);
 
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 76efc9fe831854..ed9412c93c62b3 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13907,7 +13907,7 @@ bool TreeTransform<Derived>::TransformOverloadExprDecls(OverloadExpr *Old,
     }
 
     AllEmptyPacks &= Decls.empty();
-  };
+  }
 
   // C++ [temp.res]/8.4.2:
   //   The program is ill-formed, no diagnostic required, if [...] lookup for
diff --git a/clang/test/SemaCXX/PR84020.cpp b/clang/test/SemaCXX/PR84020.cpp
deleted file mode 100644
index 8ea5dcc4527ae7..00000000000000
--- a/clang/test/SemaCXX/PR84020.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 -std=c++20 -verify %s
-// RUN: %clang_cc1 -std=c++23 -verify %s
-// expected-no-diagnostics
-
-struct B {
-    template <typename S>
-    void foo();
-
-    void bar();
-};
-
-template <typename T, typename S>
-struct A : T {
-    auto foo() {
-        static_assert(requires { T::template foo<S>(); });
-        static_assert(requires { T::bar(); });
-    }
-};
-
-int main() {
-    A<B, double> a;
-    a.foo();
-}
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index 20a19d731ae169..a1f5456156a06f 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -237,3 +237,34 @@ constexpr bool e_v = true;
 static_assert(e_v<bool>);
 
 } // namespace GH73885
+
+namespace GH84020 {
+
+struct B {
+  template <typename S> void foo();
+  void bar();
+};
+
+template <typename T, typename S> struct A : T {
+  void foo() {
+    static_assert(requires { T::template foo<S>(); });
+    static_assert(requires { T::bar(); });
+  }
+};
+
+template class A<B, double>;
+
+} // namespace GH84020
+
+namespace GH110785 {
+
+struct Foo {
+  static void f(auto) requires(false) {}
+  void f(int) {}
+
+  static_assert([](auto v) {
+    return requires { f(v); };
+  } (0) == false);
+};
+
+} // namespace GH110785

@cor3ntin
Copy link
Contributor

cor3ntin commented Oct 6, 2024

Do you know why this check was there in the first place?

@zyn0217
Copy link
Contributor Author

zyn0217 commented Oct 6, 2024

That's #85198. Which was merged prior to the generic solution fd87d76 by @sdkrystian

@cor3ntin
Copy link
Contributor

cor3ntin commented Oct 6, 2024

That's #85198. Which was merged prior to the generic solution fd87d76 by @sdkrystian

Thanks!
Can you make the description reflect that more clearly? Thanks!

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zyn0217
Copy link
Contributor Author

zyn0217 commented Oct 7, 2024

Can you make the description reflect that more clearly? Thanks!

Done.

@zyn0217 zyn0217 merged commit 8c15470 into llvm:main Oct 7, 2024
13 checks passed
@zyn0217 zyn0217 deleted the regression-110785 branch October 7, 2024 01:38
@zyn0217 zyn0217 added this to the LLVM 19.X Release milestone Oct 7, 2024
@zyn0217
Copy link
Contributor Author

zyn0217 commented Oct 7, 2024

/cherry-pick 8c15470

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 7, 2024
…solvedCallExpr() after fd87d76 (llvm#111277)

The special-casing for RequiresExprBodyDecl caused a regression, as
reported in llvm#110785.

The original fix for llvm#84020 has been superseded by fd87d76, which
establishes a `DependentScopeDeclRefExpr` instead of a
`CXXDependentScopeMemberExpr` for the case in issue. So the spurious
diagnostic in llvm#84020 would no longer occur.

This also merges the test for llvm#84020 together with that for llvm#110785 into
clang/test/SemaTemplate/instantiate-requires-expr.cpp.

No release note because I think this merits a backport.

Fixes llvm#110785

(cherry picked from commit 8c15470)
@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2024

/pull-request #111324

@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 7, 2024

LLVM Buildbot has detected a new failure on builder lldb-arm-ubuntu running on linaro-lldb-arm-ubuntu while building clang at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/5006

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: lang/cpp/global_variables/TestCPPGlobalVariables.py (788 of 2809)
PASS: lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py (789 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py (790 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/templates/TestGModules.py (791 of 2809)
PASS: lldb-api :: lang/cpp/incompatible-class-templates/TestCppIncompatibleClassTemplates.py (792 of 2809)
PASS: lldb-api :: lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py (793 of 2809)
PASS: lldb-api :: lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py (794 of 2809)
PASS: lldb-api :: lang/cpp/keywords_enabled/TestCppKeywordsEnabled.py (795 of 2809)
PASS: lldb-api :: lang/cpp/incomplete-types/TestCppIncompleteTypes.py (796 of 2809)
PASS: lldb-api :: lang/cpp/inlines/TestInlines.py (797 of 2809)
FAIL: lldb-api :: lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py (798 of 2809)
******************** TEST 'lldb-api :: lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch armv8l --build-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/lang/c/shared_lib_stripped_symbols -p TestSharedLibStrippedSymbols.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 8c1547055eaf65003f3e6fd024195f4926ff2356)
  clang revision 8c1547055eaf65003f3e6fd024195f4926ff2356
  llvm revision 8c1547055eaf65003f3e6fd024195f4926ff2356
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dsym (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase) (test case does not fall in any category of interest for this run) 
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dwo (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dsym (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase) (test case does not fall in any category of interest for this run) 
XFAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
XFAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dwo (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
======================================================================
FAIL: test_expr_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
   Test that types work when defined in a shared library and forwa/d-declared in the main executable
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1769, in test_method
    return attrvalue(self)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py", line 24, in test_expr
    self.expect(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2370, in expect
    self.runCmd(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1000, in runCmd
    self.assertTrue(self.res.Succeeded(), msg + output)
AssertionError: False is not true : Variable(s) displayed correctly
Error output:

Kyvangka1610 added a commit to Kyvangka1610/llvm-project that referenced this pull request Oct 7, 2024
* commit 'FETCH_HEAD':
  [X86] getIntImmCostInst - pull out repeated Imm.getBitWidth() calls. NFC.
  [X86] Add test coverage for llvm#111323
  [Driver] Use empty multilib file in another test (llvm#111352)
  [clang][OpenMP][test] Use x86_64-linux-gnu triple for test referencing avx512f feature (llvm#111337)
  [doc] Fix Kaleidoscope tutorial chapter 3 code snippet and full listing discrepancies (llvm#111289)
  [Flang][OpenMP] Improve entry block argument creation and binding (llvm#110267)
  [x86] combineMul - handle 0/-1 KnownBits cases before MUL_IMM logic (REAPPLIED)
  [llvm-dis] Fix non-deterministic disassembly across multiple inputs (llvm#110988)
  [lldb][test] TestDataFormatterLibcxxOptionalSimulator.py: change order of ifdefs
  [lldb][test] Add libcxx-simulators test for std::optional (llvm#111133)
  [x86] combineMul - use computeKnownBits directly to find MUL_IMM constant splat. (REAPPLIED)
  Reland "[lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout" (llvm#111123)
  Revert "[x86] combineMul - use computeKnownBits directly to find MUL_IMM constant splat."
  update_test_checks: fix a simple regression  (llvm#111347)
  [LegalizeVectorTypes] Always widen fabs (llvm#111298)
  [lsan] Make ReportUnsuspendedThreads return bool also for Fuchsia
  [mlir][vector] Add more tests for ConvertVectorToLLVM (6/n) (llvm#111121)
  [bazel] port 9144fed
  [SystemZ] Remove inlining threshold multiplier. (llvm#106058)
  [LegalizeVectorTypes] When widening don't check for libcalls if promoted (llvm#111297)
  [clang][Driver] Improve multilib custom error reporting (llvm#110804)
  [clang][Driver] Rename "FatalError" key to "Error" in multilib.yaml (llvm#110804)
  [LLVM][Maintainers] Update release managers (llvm#111164)
  [Clang][Driver] Add option to provide path for multilib's YAML config file (llvm#109640)
  [LoopVectorize] Remove redundant code in emitSCEVChecks (llvm#111132)
  [AMDGPU] Only emit SCOPE_SYS global_wb (llvm#110636)
  [ELF] Change Ctx::target to unique_ptr (llvm#111260)
  [ELF] Pass Ctx & to some free functions
  [RISCV] Only disassemble fcvtmod.w.d if the rounding mode is rtz. (llvm#111308)
  [Clang] Remove the special-casing for RequiresExprBodyDecl in BuildResolvedCallExpr() after fd87d76 (llvm#111277)
  [ELF] Pass Ctx & to InputFile
  [clang-format] Add AlignFunctionDeclarations to AlignConsecutiveDeclarations (llvm#108241)
  [AMDGPU] Support preloading hidden kernel arguments (llvm#98861)
  [ELF] Move static nextGroupId isInGroup to LinkerDriver
  [clangd] Add ArgumentLists config option under Completion (llvm#111322)
  [ELF] Pass Ctx & to SyntheticSections
  [ELF] Pass Ctx & to Symbols
  [ELF] Pass Ctx & to Symbols
  [ELF] getRelocTargetVA: pass Ctx and Relocation. NFC
  [clang-tidy] Avoid capturing a local variable in a static lambda in UseRangesCheck (llvm#111282)
  [VPlan] Use pointer to member 0 as VPInterleaveRecipe's pointer arg. (llvm#106431)
  [clangd] Simplify ternary expressions with std::optional::value_or (NFC) (llvm#111309)
  [libc++][format][2/3] Optimizes c-string arguments. (llvm#101805)
  [RISCV] Combine RVBUnary and RVKUnary into classes that are more similar to ALU(W)_r(r/i). NFC (llvm#111279)
  [ELF] Pass Ctx & to InputFiles
  [libc] GPU RPC interface: add return value to `rpc_host_call` (llvm#111288)

Signed-off-by: kyvangka1610 <[email protected]>
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 11, 2024
…solvedCallExpr() after fd87d76 (llvm#111277)

The special-casing for RequiresExprBodyDecl caused a regression, as
reported in llvm#110785.

The original fix for llvm#84020 has been superseded by fd87d76, which
establishes a `DependentScopeDeclRefExpr` instead of a
`CXXDependentScopeMemberExpr` for the case in issue. So the spurious
diagnostic in llvm#84020 would no longer occur.

This also merges the test for llvm#84020 together with that for llvm#110785 into
clang/test/SemaTemplate/instantiate-requires-expr.cpp.

No release note because I think this merits a backport.

Fixes llvm#110785

(cherry picked from commit 8c15470)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

[Clang] Requires expression incorrectly use non-static methods from generic lambda inside static method
5 participants