[SR-13237] Remove ModuleDecl::isClangModule()

[typesanitizer]

Previous ID	SR-13237
Radar	rdar://problem/65695036
Original Reporter	@typesanitizer
Type	Task
Status	Closed
Resolution	Done

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Task, StarterBug
Assignee	mateioprea (JIRA)
Priority	Medium

md5: 391d6faee40b854813aa4ea3984c1ae5

Issue Description:

The method's name is awfully misleading. A function prefixed with is should not have side-effects. But this method does module loading!!! 🙁

We should get rid of this method and replace call-sites with isNonSwiftModule(). I looked at the two uses and I think both of them are relying on it only for printing and not for module loading, so I'm somewhat confident the behavior is not going to change.

[typesanitizer]

Feel free to ask for help if you'd like to tackle this and are having trouble building the compiler or need someone to review your PR something else. 🙂

[typesanitizer]

@swift-ci create

[swift-ci]

Comment by Matei Oprea (JIRA)

Hi theindigamer (JIRA User) if this is a good starter bug, I can try to fix it. I've tried to find where isClangModule is being used by running

grep -rnw '.' -e 'isClangModule' and I can see that it's being used in multiple components. Do you have any advice of how to tackle it?

Thanks!

[swift-ci]

Comment by Nikhil (JIRA)

theindigamer (JIRA User)..I will work on SR-13245 and will un assign myself here.

[swift-ci]

Comment by Matei Oprea (JIRA)

Oh, sorry srinikhil07 (JIRA User). Didn't see that this was assigned to you.

[swift-ci]

Comment by Nikhil (JIRA)

mateioprea (JIRA User),

No problem, my hands are full right now anyway. Please assign this to yourself and start working.

[typesanitizer]

Hi Matei! Are you using Xcode or are you using some other editor? Xcode does have functionality of finding all call-sites (IIRC it is called "Find Call Hierarchy"), so you don't need to use a regex search (also ripgrep is much faster than grep -rnw in case you want to try it 🙂). Another good way to do it would be:

1. First makes sure the baseline compiler (without any changes) compiles ok. You can run tests too (there shouldn't be any baseline failures, but if there are, just note them down).

2. Comment out the ModuleDecl::isClangModule method and recompile.

3. You will see a bunch of compilation errors. Go to each call-site and fix the errors (replacing each call to isClangModule with isNonSwiftModule) until things compile ok.

4. Make sure the tests run fine.

5. Remove the commented method.

6. Commit with a useful commit message. Say something like

   [NFC] Remove ModuleDecl::isClangModule in favor of isNonSwiftModule.
   
   Fixes SR-13237.

7. Open a new PR and tag me as reviewer (varungandhi-apple on GitHub).

(The Jira bullet numbering seems to be goofing up for some reason.)

Let me know if you're having trouble building the compiler or following any of the steps. 🙂

[swift-ci]

Comment by Matei Oprea (JIRA)

Hi Varun,

I'm using Xcode, but I don't seem to find a xcode project in swift-source/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64.

I've followed the instructions in the readme and compiled using utils/build-script --release-debuginfo. Should I use something else for compilation?

Also, I was just playing around and I had to stash the changes. Just to be sure, I need to be up to date with "master" branch, right?

[typesanitizer]

If you want to build for Xcode (you will need a recent beta) you can use the --xcode flag:

utils/build-script --release-debuginfo --xcode

This will create an Xcode project in build/Xcode-RelWithDebInfoAssert/swift-macosx-x86_64/.

Yes, you need to be up-to-date with the master branch.

[swift-ci]

Comment by Matei Oprea (JIRA)

theindigamer (JIRA User) sorry it's taking too long. I got this when running with --xcode. But apparently, without using this the --xcode arguments everything works.

1. mateioprea in ~/www/swift-source (⎈ |N/A:default) [21:41:11]
   → ./swift/utils/build-script --release-debuginfo --xcode
   [./swift/utils/build-script] NOTE: Using toolchain default
   • /usr/libexec/PlistBuddy -c 'Print :SupportedTargets:macosx:Archs' /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk/SDKSettings.plist
   • mkdir -p /Users/mateioprea/www/swift-source/build/Xcode-RelWithDebInfoAssert
     Building the standard library for: swift-test-stdlib-macosx-x86_64
   • /usr/local/bin/cmake --build /Users/mateioprea/www/swift-source/build/Xcode-RelWithDebInfoAssert/cmark-macosx-x86_64 --target ZERO_CHECK --config RelWithDebInfo
     Command line invocation:
     /Applications/Xcode-beta.app/Contents/Developer/usr/bin/xcodebuild -project cmark.xcodeproj build -target ZERO_CHECK -configuration RelWithDebInfo -hideShellScriptEnvironment

User defaults from command line:
HideShellScriptEnvironment = YES

Build settings from command line:
TOOLCHAINS = default

xcodebuild: error: 'cmark.xcodeproj' does not exist.
ERROR: command terminated with a non-zero exit status 66, aborting

I've already ran: sudo xcode-select -switch /Applications/Xcode-beta.app and Xcode-beta.app is Version 12.0 beta 3 (12A8169g).

My setup is: Catalina 10.15.6 on MacBook Pro (15-inch, 2018).

Anything that I can do? Thanks!

[typesanitizer]

Did you run utils/update-checkout --clone-with-ssh when checking out the repository?

Also, please don't apologize: we're not in a rush to fix this issue and I'm guessing you're contributing on your own time as a volunteer.

[swift-ci]

Comment by Matei Oprea (JIRA)

Yes, i did. I am able to build without passing --xcode as an option. I can also run the swift executable that's generated. The only problem seems to be when running with --xcode.

I have also deleted the old clone, cloned the repo again and try to run: utils/build-script --release-debuginfo --xcode and still doesnt work.
When I remove the --xcode option everything runs smooth. Quick sample below:

[108/3824][ 2%][16.787s] Building CXX object lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/Native/NativeTypeUDT.cpp.o

Xcode beta has been downloaded today if it matters in any way.

[typesanitizer]

I'm afraid I can't tell what the issue is from the limited information. There was a similar issue reported here: https://forums.swift.org/t/compiling-swift-with-xcode-option-on-xcode-12-beta-3/38768

If you are facing the same issue (Swift builds for arm64), then you can change the SUPPORTED_OSX_ARCHS setting in cmake/modules/DarwinSDKs.cmake to have only x86_64 and then rebuild with an extra --reconfigure flag (or do a clean build).

[swift-ci]

Comment by Matei Oprea (JIRA)

Varun,

It worked with the following options:

1. upgrade to CMake 3.16.5 or later

2. run utils/build-script --reconfigure --release-debuginfo --xcode -~~clean ~~-extra-cmake-options="-DCMAKE_OSX_ARCHITECTURES=x86_64"

Apparently it started building everything. Thank you.

[swift-ci]

Comment by Matei Oprea (JIRA)

Hi Varun,

Rolling back to Xcode 12 beta 1 fixed the issue. Opened up a PR here: #33202

Also, one question: is there a way for compiling the code for swift again without compiling the entire modules base?

I had to run swift/utils/build-script --release-debuginfo --xcode --extra-cmake-options="-DCMAKE_OSX_ARCHITECTURES=x86_64" every time. I tried with ninja swift-frontend, but I guess that doesn't work when first compiling with --xcode option.