Bug 238455 - Wrong variable scope in subdirs
Summary: Wrong variable scope in subdirs
Alias: None
Product: kdevelop
Classification: Unclassified
Component: Build tools: CMake (show other bugs)
Version: unspecified
Platform: unspecified Linux
: LO normal (vote)
Target Milestone: ---
Assignee: kdevelop-bugs-null
Depends on:
Reported: 2010-05-22 04:12 UTC by Nicolás Alvarez
Modified: 2021-03-09 22:48 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Note You need to log in before you can comment on or make changes to this bug.
Description Nicolás Alvarez 2010-05-22 04:12:59 UTC
Version:           4.0.60 (using KDevPlatform 1.0.60) (using 4.4.3 (KDE 4.4.3), Debian packages)
OS:                Linux (x86_64) release 2.6.32-3-amd64

I spent a long time reading the CMake parser code trying to figure out where the per-directory variable map was stored, only to find (by testing) that it isn't.

Here is a testcase that shows the wrong behavior:

# CMakeLists.txt:
set(foo "asd")
message(STATUS "result: ${foo}")

# sub/CMakeLists.txt:
message(STATUS "this is sub1: ${foo}")
set(foo "one")
message(STATUS "changed to: ${foo}")

# sub2/CMakeLists.txt:
message(STATUS "this is sub2: ${foo}")
set(foo "two")
message(STATUS "changed to: ${foo}")

CMake 2.8.1 output:
-- this is sub1: asd
-- changed to: one
-- this is sub2: asd
-- changed to: two
-- result: asd

KDevelop 4.0 output:
message: ("result: asd")
message: ("this is sub1: asd")
message: ("changed to: one")
message: ("this is sub2: one")
message: ("changed to: two")

It prints "result: asd" first instead of last due to bug REPLACEHERE; that's a different issue. The problem here is that sub2 sees foo set to "one" instead of "asd".

Currently the variable map is per project, and it's modified directly when processing each directory. To fix this, we would need a *stack* of variable maps.
Comment 1 Nicolás Alvarez 2010-05-22 04:14:16 UTC
And I'm an idiot and forgot to replace REPLACEME with "bug 238456" before submitting -.-
Comment 2 Nicolás Alvarez 2010-05-22 04:17:03 UTC
CMake functions (not macros) have their own variable scope too:

 message(STATUS "this is sub: ${foo}")
 set(foo "one")
 message(STATUS "changed to: ${foo}")

set(foo "asd")
message(STATUS "result: ${foo}")

CMake 2.8.1:
-- this is sub1: asd
-- changed to: one
-- result: asd

KDevelop 4.0:
message: ("this is sub1: asd")
message: ("changed to: one")
message: ("result: one")
Comment 3 Andreas Pakulat 2010-06-10 08:52:36 UTC
Changing severity and importance as this can cause KDevelop to eat all memory because variable contents grow beyond all limits.
Comment 4 Aleix Pol 2010-12-15 02:50:52 UTC
Can't have it as VHI
Comment 5 Aleix Pol 2010-12-21 01:04:28 UTC
It's not a crash either
Comment 6 Milian Wolff 2010-12-21 10:33:44 UTC
it can crash, see Andreas' comment.
Comment 7 Aleix Pol 2011-12-19 02:27:43 UTC
Git commit a06e144523f56358f1921e06e175228f01c5d5ad by Aleix Pol.
Committed on 19/12/2011 at 03:25.
Pushed by apol into branch 'master'.

Support scopes in CMake support
Only implemented for functions for the moment.

CCBUG: 238455

M  +27   -21   projectmanagers/cmake/parser/cmakeprojectvisitor.cpp
M  +3    -1    projectmanagers/cmake/parser/cmakeprojectvisitor.h
M  +52   -2    projectmanagers/cmake/parser/variablemap.cpp
M  +15   -2    projectmanagers/cmake/parser/variablemap.h
M  +37   -5    projectmanagers/cmake/tests/cmake_cmakeprojectvisitor_test.cpp

Comment 8 Aleix Pol 2011-12-19 03:02:08 UTC
Since last commit, this can't lead to recursively eating all memory in function, so it's not a crash anymore.
Comment 9 Aleix Pol 2012-11-15 02:07:56 UTC
*** Bug 309830 has been marked as a duplicate of this bug. ***
Comment 10 Aleix Pol 2012-11-15 02:51:06 UTC
Git commit df00d058fcce0d4026704aeffbb6802dd9582433 by Aleix Pol.
Committed on 15/11/2012 at 03:48.
Pushed by apol into branch 'master'.

Fix subdirectory scoping in cmake projects

Don't pop the scope until all the subdirectories have been processed.

Thanks to Daniel Calviño Sánchez for such a helpful bug report :)
Related: bug 309687

M  +6    -3    projectmanagers/cmake/cmakemanager.cpp
M  +27   -0    projectmanagers/cmake/tests/cmakemanagertest.cpp
M  +1    -0    projectmanagers/cmake/tests/cmakemanagertest.h

Comment 11 Justin Zobel 2021-03-09 22:48:24 UTC
Thank you for the bug report.

As this report hasn't seen any changes in 5 years or more, we ask if you can please confirm that the issue still persists.

If this bug is no longer persisting or relevant please change the status to resolved.