Skip to content

Commit

Permalink
Choose SDK consistently at least for non-COMPOSITE analysis
Browse files Browse the repository at this point in the history
In COMPOSITE analysis we don't have a constant SDK dependency for whole
ResolutionFacade, so we have to find it manually via calling
'findSdkAcrossDependencies'

In SEPARATE mode we do have such a constant SDK dependency, but there
was an assumption that it doesn't make a difference (i.e.
findSdkAcrossDependencies always return the same SDK as passed constant
one)

Turns out that assumption was wrong, particularly for projects with
duplicate SDK entries in project structure. Then we would choose one SDK
in some cases, and the other one on other cases - even though they a
bit-to-bit equal, we will create separate descriptors for them, which
might lead to some issues, like in KT-34802

^KT-34802 Fixed
  • Loading branch information
dsavvinov committed Nov 22, 2019
1 parent fea32f2 commit ee4a5cc
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.analyzer.common.CommonResolverForModuleFactory
import org.jetbrains.kotlin.builtins.DefaultBuiltIns
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.context.ProjectContext
import org.jetbrains.kotlin.idea.caches.project.SdkInfo
import org.jetbrains.kotlin.idea.caches.resolve.BuiltInsCacheKey
import org.jetbrains.kotlin.idea.framework.CommonLibraryKind
import org.jetbrains.kotlin.platform.impl.CommonIdePlatformKind
Expand All @@ -34,7 +35,7 @@ class CommonPlatformKindResolution : IdePlatformKindResolution {

override fun getKeyForBuiltIns(moduleInfo: ModuleInfo): BuiltInsCacheKey = BuiltInsCacheKey.DefaultBuiltInsKey

override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext): KotlinBuiltIns {
override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext, sdkDependency: SdkInfo?): KotlinBuiltIns {
return DefaultBuiltIns.Instance
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.jetbrains.kotlin.descriptors.ModuleDescriptor
import org.jetbrains.kotlin.descriptors.PackageFragmentProvider
import org.jetbrains.kotlin.extensions.ApplicationExtensionDescriptor
import org.jetbrains.kotlin.idea.caches.project.LibraryInfo
import org.jetbrains.kotlin.idea.caches.project.SdkInfo
import org.jetbrains.kotlin.idea.caches.resolve.BuiltInsCacheKey
import org.jetbrains.kotlin.platform.IdePlatformKind
import org.jetbrains.kotlin.platform.TargetPlatform
Expand All @@ -40,7 +41,7 @@ interface IdePlatformKindResolution {
val kind: IdePlatformKind<*>

fun getKeyForBuiltIns(moduleInfo: ModuleInfo): BuiltInsCacheKey
fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext): KotlinBuiltIns
fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext, sdkDependency: SdkInfo?): KotlinBuiltIns

fun createResolverForModuleFactory(
settings: PlatformAnalysisParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import org.jetbrains.kotlin.analyzer.ResolverForModuleFactory
import org.jetbrains.kotlin.builtins.DefaultBuiltIns
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.context.ProjectContext
import org.jetbrains.kotlin.idea.caches.project.SdkInfo
import org.jetbrains.kotlin.idea.caches.resolve.BuiltInsCacheKey
import org.jetbrains.kotlin.idea.framework.JSLibraryKind
import org.jetbrains.kotlin.js.resolve.JsResolverForModuleFactory
import org.jetbrains.kotlin.js.resolve.JsPlatformAnalyzerServices
import org.jetbrains.kotlin.platform.impl.JsIdePlatformKind
import org.jetbrains.kotlin.resolve.TargetEnvironment
import org.jetbrains.kotlin.platform.TargetPlatform
Expand All @@ -35,7 +35,7 @@ class JsPlatformKindResolution : IdePlatformKindResolution {
return BuiltInsCacheKey.DefaultBuiltInsKey
}

override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext): KotlinBuiltIns {
override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext, sdkDependency: SdkInfo?): KotlinBuiltIns {
return DefaultBuiltIns.Instance
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.builtins.DefaultBuiltIns
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.builtins.jvm.JvmBuiltIns
import org.jetbrains.kotlin.context.ProjectContext
import org.jetbrains.kotlin.idea.caches.project.SdkInfo
import org.jetbrains.kotlin.idea.caches.project.findSdkAcrossDependencies
import org.jetbrains.kotlin.idea.caches.resolve.BuiltInsCacheKey
import org.jetbrains.kotlin.platform.TargetPlatform
Expand Down Expand Up @@ -46,10 +47,8 @@ class JvmPlatformKindResolution : IdePlatformKindResolution {
return if (sdkInfo != null) CacheKeyBySdk(sdkInfo.sdk) else BuiltInsCacheKey.DefaultBuiltInsKey
}

override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext): KotlinBuiltIns {
val sdk = moduleInfo.findSdkAcrossDependencies()

return if (sdk != null)
override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext, sdkDependency: SdkInfo?): KotlinBuiltIns {
return if (sdkDependency != null)
JvmBuiltIns(projectContext.storageManager, JvmBuiltIns.Kind.FROM_CLASS_LOADER)
else
DefaultBuiltIns.Instance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ import org.jetbrains.kotlin.context.ProjectContext
import org.jetbrains.kotlin.context.withModule
import org.jetbrains.kotlin.descriptors.ModuleDescriptor
import org.jetbrains.kotlin.descriptors.impl.ModuleDescriptorImpl
import org.jetbrains.kotlin.idea.caches.project.*
import org.jetbrains.kotlin.idea.caches.project.IdeaModuleInfo
import org.jetbrains.kotlin.idea.caches.project.findSdkAcrossDependencies
import org.jetbrains.kotlin.idea.caches.project.getNullableModuleInfo
import org.jetbrains.kotlin.idea.caches.project.getPlatformModuleInfo
import org.jetbrains.kotlin.idea.compiler.IDELanguageSettingsProvider
import org.jetbrains.kotlin.idea.project.IdeaEnvironment
import org.jetbrains.kotlin.idea.project.findAnalyzerServices
Expand All @@ -45,7 +44,7 @@ class IdeaResolverForProject(
fallbackModificationTracker: ModificationTracker? = null,
private val isReleaseCoroutines: Boolean? = null,
// TODO(dsavvinov): this is needed only for non-composite analysis, extract separate resolver implementation instead
private val constantSdkDependencyIfAny: IdeaModuleInfo? = null
private val constantSdkDependencyIfAny: SdkInfo? = null
) : AbstractResolverForProject<IdeaModuleInfo>(
debugName,
projectContext,
Expand All @@ -54,9 +53,10 @@ class IdeaResolverForProject(
delegateResolver,
ServiceManager.getService(projectContext.project, IdePackageOracleFactory::class.java)
) {
private val builtInsCache: BuiltInsCache = (delegateResolver as? IdeaResolverForProject)?.builtInsCache ?: BuiltInsCache(projectContext, this)
private val builtInsCache: BuiltInsCache =
(delegateResolver as? IdeaResolverForProject)?.builtInsCache ?: BuiltInsCache(projectContext, this)

override fun sdkDependency(module: IdeaModuleInfo): IdeaModuleInfo? {
override fun sdkDependency(module: IdeaModuleInfo): SdkInfo? {
if (projectContext.project.useCompositeAnalysis) {
require(constantSdkDependencyIfAny == null) { "Shouldn't pass SDK dependency manually for composite analysis mode" }
}
Expand Down Expand Up @@ -129,13 +129,13 @@ class IdeaResolverForProject(

// Note #1: we can't use .getOrPut, because we have to put builtIns into map *before* initialization
// Note #2: it's OK to put not-initialized built-ins into public map, because access to [cache] is guarded by storageManager.lock
val newBuiltIns = module.platform.idePlatformKind.resolution.createBuiltIns(module, projectContextFromSdkResolver)
val sdk = resolverForSdk.sdkDependency(module)
val newBuiltIns = module.platform.idePlatformKind.resolution.createBuiltIns(module, projectContextFromSdkResolver, sdk)
cache[key] = newBuiltIns

if (newBuiltIns is JvmBuiltIns) {
// SDK should be present, otherwise we wouldn't have created JvmBuiltIns in createBuiltIns
val sdk = module.findSdkAcrossDependencies()!!
val sdkDescriptor = resolverForSdk.descriptorForModule(sdk)
val sdkDescriptor = resolverForSdk.descriptorForModule(sdk!!)

val isAdditionalBuiltInsFeaturesSupported = module.supportsAdditionalBuiltInsMembers(projectContextFromSdkResolver.project)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import org.jetbrains.kotlin.descriptors.konan.KlibModuleOrigin
import org.jetbrains.kotlin.ide.konan.NativeLibraryInfo.Companion.safeAbiVersion
import org.jetbrains.kotlin.ide.konan.NativeLibraryInfo.Companion.isCompatible
import org.jetbrains.kotlin.ide.konan.analyzer.NativeResolverForModuleFactory
import org.jetbrains.kotlin.idea.caches.project.IdeaModuleInfo
import org.jetbrains.kotlin.idea.caches.project.LibraryInfo
import org.jetbrains.kotlin.idea.caches.project.SdkInfo
import org.jetbrains.kotlin.idea.caches.project.lazyClosure
import org.jetbrains.kotlin.idea.caches.resolve.BuiltInsCacheKey
import org.jetbrains.kotlin.idea.compiler.IDELanguageSettingsProvider
Expand Down Expand Up @@ -130,7 +132,7 @@ class NativePlatformKindResolution : IdePlatformKindResolution {

override fun getKeyForBuiltIns(moduleInfo: ModuleInfo): BuiltInsCacheKey = NativeBuiltInsCacheKey

override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext) =
override fun createBuiltIns(moduleInfo: ModuleInfo, projectContext: ProjectContext, sdkDependency: SdkInfo?) =
createKotlinNativeBuiltIns(moduleInfo, projectContext)

object NativeBuiltInsCacheKey : BuiltInsCacheKey
Expand Down

0 comments on commit ee4a5cc

Please sign in to comment.