Skip to content
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

Add disable-private-check option #8202

Merged
merged 4 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public class RuntimeOptions {
private static final OptionDescriptor DISABLE_INLINE_CACHES_DESCRIPTOR =
OptionDescriptor.newBuilder(DISABLE_INLINE_CACHES_KEY, DISABLE_INLINE_CACHES).build();

public static final String DISABLE_PRIVATE_CHECK = optionName("disablePrivateCheck");
public static final OptionKey<Boolean> DISABLE_PRIVATE_CHECK_KEY = new OptionKey<>(false);
private static final OptionDescriptor DISABLE_PRIVATE_CHECK_DESCRIPTOR =
OptionDescriptor.newBuilder(DISABLE_PRIVATE_CHECK_KEY, DISABLE_PRIVATE_CHECK).build();

public static final String ENABLE_AUTO_PARALLELISM = optionName("withAutoParallelism");
public static final OptionKey<Boolean> ENABLE_AUTO_PARALLELISM_KEY = new OptionKey<>(false);
private static final OptionDescriptor ENABLE_AUTO_PARALLELISM_DESCRIPTOR =
Expand Down Expand Up @@ -125,6 +130,7 @@ public class RuntimeOptions {
STRICT_ERRORS_DESCRIPTOR,
LOG_MASKING_DESCRIPTOR,
DISABLE_INLINE_CACHES_DESCRIPTOR,
DISABLE_PRIVATE_CHECK_DESCRIPTOR,
ENABLE_AUTO_PARALLELISM_DESCRIPTOR,
ENABLE_PROJECT_SUGGESTIONS_DESCRIPTOR,
ENABLE_GLOBAL_SUGGESTIONS_DESCRIPTOR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class ContextFactory {
* @param repl the Repl manager to use for this context
* @param logLevel the log level for this context
* @param enableIrCaches whether or not IR caching should be enabled
* @param disablePrivateCheck If `private` keyword should be disabled.
* @param strictErrors whether or not to use strict errors
* @param useGlobalIrCacheLocation whether or not to use the global IR cache
* location
Expand All @@ -42,6 +43,7 @@ class ContextFactory {
logLevel: Level,
logMasking: Boolean,
enableIrCaches: Boolean,
disablePrivateCheck: Boolean = false,
strictErrors: Boolean = false,
useGlobalIrCacheLocation: Boolean = true,
enableAutoParallelism: Boolean = false,
Expand Down Expand Up @@ -76,6 +78,10 @@ class ContextFactory {
useGlobalIrCacheLocation.toString
)
.option(RuntimeOptions.DISABLE_IR_CACHES, (!enableIrCaches).toString)
.option(
RuntimeOptions.DISABLE_PRIVATE_CHECK,
disablePrivateCheck.toString
)
.option(DebugServerInfo.ENABLE_OPTION, "true")
.option(RuntimeOptions.LOG_MASKING, logMasking.toString)
.options(options)
Expand Down
5 changes: 5 additions & 0 deletions engine/runner/src/main/scala/org/enso/runner/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ object Main {
private val IR_CACHES_OPTION = "ir-caches"
private val NO_IR_CACHES_OPTION = "no-ir-caches"
private val NO_READ_IR_CACHES_OPTION = "no-read-ir-caches"
private val DISABLE_PRIVATE_CHECK_OPTION = "disable-private-check"
private val COMPILE_OPTION = "compile"
private val NO_COMPILE_DEPENDENCIES_OPTION = "no-compile-dependencies"
private val NO_GLOBAL_CACHE_OPTION = "no-global-cache"
Expand Down Expand Up @@ -586,6 +587,7 @@ object Main {
* @param logLevel log level to set for the engine runtime
* @param logMasking is the log masking enabled
* @param enableIrCaches are IR caches enabled
* @param disablePrivateCheck Is private modules check disabled. If yes, `private` keyword is ignored.
* @param inspect shall inspect option be enabled
* @param dump shall graphs be sent to the IGV
* @param executionEnvironment optional name of the execution environment to use during execution
Expand All @@ -597,6 +599,7 @@ object Main {
logLevel: Level,
logMasking: Boolean,
enableIrCaches: Boolean,
disablePrivateCheck: Boolean,
enableAutoParallelism: Boolean,
inspect: Boolean,
dump: Boolean,
Expand Down Expand Up @@ -639,6 +642,7 @@ object Main {
logLevel,
logMasking,
enableIrCaches,
disablePrivateCheck,
strictErrors = true,
enableAutoParallelism = enableAutoParallelism,
executionEnvironment = executionEnvironment,
Expand Down Expand Up @@ -1171,6 +1175,7 @@ object Main {
logLevel,
logMasking,
shouldEnableIrCaches(line),
line.hasOption(DISABLE_PRIVATE_CHECK_OPTION),
line.hasOption(AUTO_PARALLELISM_OPTION),
line.hasOption(INSPECT_OPTION),
line.hasOption(DUMP_GRAPHS_OPTION),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
public interface CompilerContext {
boolean isIrCachingDisabled();

boolean isPrivateCheckDisabled();

boolean isUseGlobalCacheLocations();

boolean isInteractiveMode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,13 @@ class Passes(
OperatorToFunction,
LambdaShorthandToLambda,
ImportSymbolAnalysis,
AmbiguousImportsAnalysis,
PrivateModuleAnalysis.INSTANCE,
AmbiguousImportsAnalysis
) ++ (if (config.privateCheckEnabled) {
Copy link
Member Author

Choose a reason for hiding this comment

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

So far, we only implement private modules via PrivateModuleAnalysis compiler pass, there is no runtime checking yet. Thus, this conditional adding of PrivateModuleAnalysis pass effectively disables the checks. In the future, when we will have more private stuff, like types, we will need some more sophisticated way to disable these checks. That is why the option is also in EnsoLanguage and not only in CompilerConfig.

List(
PrivateModuleAnalysis.INSTANCE
)
} else List())
++ List(
ExportSymbolAnalysis.INSTANCE,
ShadowedPatternFields,
UnreachableMatchBranches,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import java.io.PrintStream
* @param autoParallelismEnabled whether or not automatic parallelism detection
* is enabled.
* @param warningsEnabled whether or not warnings are enabled
* @param privateCheckEnabled whether or not private keyword is enabled
* @param isStrictErrors if true, presence of any Error in IR will result in an exception
* @param outputRedirect redirection of the output of warnings and errors of compiler
*/
case class CompilerConfig(
autoParallelismEnabled: Boolean = false,
warningsEnabled: Boolean = true,
privateCheckEnabled: Boolean = true,
isStrictErrors: Boolean = false,
outputRedirect: Option[PrintStream] = None
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ protected ExecutableNode parse(InlineParsingRequest request) throws InlineParsin
false,
false,
true,
true,
scala.Option.apply(new PrintStream(outputRedirect))
);
var moduleContext = new ModuleContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public final class EnsoContext {
private final EnsoLanguage language;
private final Env environment;
private final boolean assertionsEnabled;
private final boolean isPrivateCheckDisabled;
private @CompilationFinal
Compiler compiler;
private final PrintStream out;
Expand Down Expand Up @@ -140,6 +141,7 @@ public EnsoContext(
var isParallelismEnabled = getOption(RuntimeOptions.ENABLE_AUTO_PARALLELISM_KEY);
this.isIrCachingDisabled
= getOption(RuntimeOptions.DISABLE_IR_CACHES_KEY) || isParallelismEnabled;
this.isPrivateCheckDisabled = getOption(RuntimeOptions.DISABLE_PRIVATE_CHECK_KEY);
this.executionEnvironment = getOption(EnsoLanguage.EXECUTION_ENVIRONMENT);
this.assertionsEnabled = shouldAssertionsBeEnabled();
this.shouldWaitForPendingSerializationJobs
Expand All @@ -148,6 +150,7 @@ public EnsoContext(
= new CompilerConfig(
isParallelismEnabled,
true,
!isPrivateCheckDisabled,
getOption(RuntimeOptions.STRICT_ERRORS_KEY),
scala.Option.empty());
this.home = home;
Expand Down Expand Up @@ -729,6 +732,13 @@ public boolean isInlineCachingDisabled() {
return isInlineCachingDisabled;
}

/**
* @return when {@code private} keyword should be checked.
*/
public boolean isPrivateCheckDisabled() {
return isPrivateCheckDisabled;
}

/**
* @return whether IR caching should be disabled for this context.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public boolean isIrCachingDisabled() {
return context.isIrCachingDisabled();
}

@Override
public boolean isPrivateCheckDisabled() {
return context.isPrivateCheckDisabled();
}

@Override
public boolean isUseGlobalCacheLocations() {
return context.isUseGlobalCache();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ import java.util.logging.Level
trait PackageTest extends AnyFlatSpec with Matchers with ValueEquality {
val output = new ByteArrayOutputStream()

def evalTestProject(name: String): Value = {
def evalTestProject(
name: String,
customOptions: Map[String, String] = Map.empty
): Value = {
val pkgPath =
new File(getClass.getClassLoader.getResource(name).getPath)
val pkg = PackageManager.Default.fromDirectory(pkgPath).get
val mainFile = pkg.mainFile
val mainModule = pkg.moduleNameForFile(mainFile)
val context = Context
val ctxBuilder = Context
.newBuilder(LanguageInfo.ID)
.allowExperimentalOptions(true)
.allowAllAccess(true)
Expand All @@ -38,7 +41,10 @@ trait PackageTest extends AnyFlatSpec with Matchers with ValueEquality {
.in(System.in)
.option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName())
.logHandler(System.err)
.build()
for ((key, value) <- customOptions) {
ctxBuilder.option(key, value)
}
val context = ctxBuilder.build()
context.initialize(LanguageInfo.ID)
val executionContext = new PolyglotContext(context)
InterpreterException.rethrowPolyglot(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.enso.interpreter.test.semantic

import org.enso.interpreter.test.{InterpreterException, PackageTest}
import org.enso.polyglot.RuntimeOptions

class ImportsTest extends PackageTest {
implicit def messagingNatureOInterpreterException
Expand Down Expand Up @@ -256,4 +257,18 @@ class ImportsTest extends PackageTest {
"Test_Private_Modules_5"
) shouldEqual 42
}

"Private modules" should "be able to mix private and public submodules when private checks are disabled" in {
evalTestProject(
"Test_Private_Modules_4",
Map(RuntimeOptions.DISABLE_PRIVATE_CHECK -> "true")
) shouldEqual 42
}

"Private modules" should "be able to import private modules from different project when private checks are disabled" in {
evalTestProject(
"Test_Private_Modules_3",
Map(RuntimeOptions.DISABLE_PRIVATE_CHECK -> "true")
) shouldEqual "Success"
}
}
Loading