Skip to content

[Clang] avoid adding consteval condition as the last statement to preserve valid CFG #116513

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 6 commits into from
Nov 20, 2024

Conversation

a-tarasyuk
Copy link
Member

Fixes #116485

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:analysis labels Nov 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 16, 2024

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #116485


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Analysis/CFG.cpp (+4-1)
  • (modified) clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp (+6)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a8830a5658c7da..d81085d011b866 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -535,6 +535,8 @@ Improvements to Clang's diagnostics
 
 - Improved diagnostic message for ``__builtin_bit_cast`` size mismatch (#GH115870).
 
+- Clang now diagnoses missing return value in functions containing ``if consteval`` (#GH116485).
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index f678ac6f2ff36a..7a6bd8b6f8d070 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -3177,11 +3177,14 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt *I) {
     if (!I->isConsteval())
       KnownVal = tryEvaluateBool(I->getCond());
 
-    // Add the successors.  If we know that specific branches are
+    // Add the successors. If we know that specific branches are
     // unreachable, inform addSuccessor() of that knowledge.
     addSuccessor(Block, ThenBlock, /* IsReachable = */ !KnownVal.isFalse());
     addSuccessor(Block, ElseBlock, /* IsReachable = */ !KnownVal.isTrue());
 
+    if (I->isConsteval())
+      return Block;
+
     // Add the condition as the last statement in the new block.  This may
     // create new blocks as the condition may contain control-flow.  Any newly
     // created blocks will be pointed to be "Block".
diff --git a/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp b/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp
index 25d1f8df7f7166..3d993f000e8dda 100644
--- a/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp
+++ b/clang/test/SemaCXX/constexpr-return-non-void-cxx2b.cpp
@@ -5,3 +5,9 @@ static_assert(__is_same(decltype([] constexpr -> int { }( )), int)); // expected
 
 consteval int g() { } // expected-warning {{non-void function does not return a value}}
 static_assert(__is_same(decltype([] consteval -> int { }( )), int)); // expected-warning {{non-void lambda does not return a value}}
+
+namespace GH116485 {
+int h() {
+    if consteval { }
+} // expected-warning {{non-void function does not return a value}}
+}

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

The changes generally LGTM, but I did have additional tests to try.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@AaronBallman AaronBallman merged commit fe697ef into llvm:main Nov 20, 2024
9 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 20, 2024

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building clang at step 5 "ninja check 1".

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

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/129/174' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests-LLVM-Unit-846629-129-174.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=174 GTEST_SHARD_INDEX=129 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests
--

Script:
--
/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests --gtest_filter=ProgramEnvTest.TestLockFile
--
../llvm/llvm/unittests/Support/ProgramTest.cpp:558: Failure
Value of: Error.empty()
  Actual: false
Expected: true


../llvm/llvm/unittests/Support/ProgramTest.cpp:558
Value of: Error.empty()
  Actual: false
Expected: true



********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 20, 2024

LLVM Buildbot has detected a new failure on builder clang-armv8-quick running on linaro-clang-armv8-quick while building clang at step 5 "ninja check 1".

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

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'lit :: googletest-timeout.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 9
not env -u FILECHECK_OPTS "/usr/bin/python3.10" /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout    --param gtest_filter=InfiniteLoopSubTest --timeout=1 > /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out
# executed command: not env -u FILECHECK_OPTS /usr/bin/python3.10 /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --timeout=1
# .---command stderr------------
# | lit.py: /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# | Traceback (most recent call last):
# |   File "/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit/formats/googletest.py", line 304, in post_process_shard_results
# |     testsuites = json.load(f)["testsuites"]
# |   File "/usr/lib/python3.10/json/__init__.py", line 293, in load
# |     return loads(fp.read(),
# |   File "/usr/lib/python3.10/json/__init__.py", line 346, in loads
# |     return _default_decoder.decode(s)
# |   File "/usr/lib/python3.10/json/decoder.py", line 337, in decode
# |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
# |   File "/usr/lib/python3.10/json/decoder.py", line 355, in raw_decode
# |     raise JSONDecodeError("Expecting value", s, err.value) from None
# | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
# | 
# | During handling of the above exception, another exception occurred:
# | 
# | Traceback (most recent call last):
# |   File "/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit.py", line 6, in <module>
# |     main()
# |   File "/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit/main.py", line 130, in main
# |     selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
# |   File "/home/tcwg-buildbot/worker/clang-armv8-quick/llvm/llvm/utils/lit/lit/formats/googletest.py", line 306, in post_process_shard_results
# |     raise RuntimeError(
# | RuntimeError: Failed to parse json file: /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py-googletest-timeout-623663-1-2.json
# | 
# `-----------------------------
# RUN: at line 11
FileCheck --check-prefix=CHECK-INF < /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/googletest-timeout.py
# .---command stderr------------
# | /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/utils/lit/tests/googletest-timeout.py:34:14: error: CHECK-INF: expected string not found in input
# | # CHECK-INF: Timed Out: 1
# |              ^
# | <stdin>:13:29: note: scanning from here
# | Reached timeout of 1 seconds
# |                             ^
# | <stdin>:15:21: note: possible intended match here
# | TIMEOUT: googletest-timeout :: DummySubDir/OneTest.py/1/2 (2 of 2)
# |                     ^
# | 
# | Input file: <stdin>
...

a-tarasyuk added a commit that referenced this pull request Nov 27, 2024
…ow terminators (#117403)

Fixes #117385

---

These changes extend the work done in #116513. The changes add
additional handling to ensure correct behavior by skipping further
checks when a **CFG** contains a `consteval` condition, where no
_explicit expression_ is present, which is required to proceed with
consumed analyses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang doesn't warn for missing return value if function contains "if consteval"
4 participants