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.

Sign up for GitHub

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

Jump to bottom

Relax import/export elision rules for separate compilation #2550

Merged
merged 10 commits into from
Mar 31, 2015
Merged

Relax import/export elision rules for separate compilation #2550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a51f0bf
added relaxed emit rules for separate compilation
vladima Mar 26, 2015
d2d3f1e
Merge branch 'master' into separateCompilation
vladima Mar 30, 2015
8f616ce
fix typos in comments
vladima Mar 30, 2015
4b7e6cf
addressed CR feedback, accepted baselines
vladima Mar 31, 2015
a6c88e2
addressed CR feedback
vladima Mar 31, 2015
955b4c0
addressed CR feedback
vladima Mar 31, 2015
c885f59
do not include declaration files in 'is external module' check
vladima Mar 31, 2015
49b3f85
disallow ambient const enums in 'separate compilation' mode
vladima Mar 31, 2015
1803d73
added initial set of unit tests for separate compilation mode
vladima Mar 31, 2015
1bdcaa3
added tests for import\export elision
vladima Mar 31, 2015
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
29 changes: 23 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,8 +713,14 @@ module ts {
function markExportAsReferenced(node: ImportEqualsDeclaration | ExportAssignment | ExportSpecifier) {
let symbol = getSymbolOfNode(node);
let target = resolveAlias(symbol);
if (target && target !== unknownSymbol && target.flags & SymbolFlags.Value && !isConstEnumOrConstEnumOnlyModule(target)) {
markAliasSymbolAsReferenced(symbol);
if (target) {
let markAlias =
(target === unknownSymbol && compilerOptions.separateCompilation) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we can just make this the default behavior. the only cases we would be breaking are error cases, that in the past we elided modules imports, and now we will be writing them. is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually I've re-checked - this might lead to cascading errors so I'll keep relaxed behavior specific to separateCompilation mode

(target !== unknownSymbol && (target.flags & SymbolFlags.Value) && !isConstEnumOrConstEnumOnlyModule(target));

if (markAlias) {
markAliasSymbolAsReferenced(symbol);
}
}
}

Expand Down Expand Up @@ -9747,7 +9753,9 @@ module ts {

checkKindsOfPropertyMemberOverrides(type, baseType);
}
}

if (type.baseTypes.length || (baseTypeNode && compilerOptions.separateCompilation)) {
// Check that base type can be evaluated as expression
checkExpressionOrQualifiedName(baseTypeNode.typeName);
}
Expand Down Expand Up @@ -10151,6 +10159,11 @@ module ts {

computeEnumMemberValues(node);

let enumIsConst = isConst(node);
if (compilerOptions.separateCompilation && enumIsConst && isInAmbientContext(node)) {
error(node.name, Diagnostics.Ambient_const_enums_are_not_allowed_when_the_separateCompilation_flag_is_provided);
}

// Spec 2014 - Section 9.3:
// It isn't possible for one enum declaration to continue the automatic numbering sequence of another,
// and when an enum type has multiple declarations, only one declaration is permitted to omit a value
Expand All @@ -10161,7 +10174,6 @@ module ts {
let firstDeclaration = getDeclarationOfKind(enumSymbol, node.kind);
if (node === firstDeclaration) {
if (enumSymbol.declarations.length > 1) {
let enumIsConst = isConst(node);
// check that const is placed\omitted on all enum declarations
forEach(enumSymbol.declarations, decl => {
if (isConstEnumDeclaration(decl) !== enumIsConst) {
Expand Down Expand Up @@ -10223,7 +10235,7 @@ module ts {
if (symbol.flags & SymbolFlags.ValueModule
&& symbol.declarations.length > 1
&& !isInAmbientContext(node)
&& isInstantiatedModule(node, compilerOptions.preserveConstEnums)) {
&& isInstantiatedModule(node, compilerOptions.preserveConstEnums || compilerOptions.separateCompilation)) {
let classOrFunc = getFirstNonAmbientClassOrFunctionDeclaration(symbol);
if (classOrFunc) {
if (getSourceFileOfNode(node) !== getSourceFileOfNode(classOrFunc)) {
Expand Down Expand Up @@ -11266,13 +11278,18 @@ module ts {
// parent is not source file or it is not reference to internal module
return false;
}
return isAliasResolvedToValue(getSymbolOfNode(node));

var isValue = isAliasResolvedToValue(getSymbolOfNode(node));
return isValue && node.moduleReference && !nodeIsMissing(node.moduleReference);
}

function isAliasResolvedToValue(symbol: Symbol): boolean {
let target = resolveAlias(symbol);
if (target === unknownSymbol && compilerOptions.separateCompilation) {
return true;
}
// const enums and modules that contain only const enums are not considered values from the emit perespective
return target !== unknownSymbol && target.flags & SymbolFlags.Value && !isConstEnumOrConstEnumOnlyModule(target);
return target !== unknownSymbol && target && target.flags & SymbolFlags.Value && !isConstEnumOrConstEnumOnlyModule(target);
}

function isConstEnumOrConstEnumOnlyModule(s: Symbol): boolean {
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ module ts {
type: "boolean",
description: Diagnostics.Do_not_emit_comments_to_output,
},
{
name: "separateCompilation",
type: "boolean",
},
{
name: "sourceMap",
type: "boolean",
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ module ts {
Decorators_are_only_available_when_targeting_ECMAScript_5_and_higher: { code: 1205, category: DiagnosticCategory.Error, key: "Decorators are only available when targeting ECMAScript 5 and higher." },
Decorators_are_not_valid_here: { code: 1206, category: DiagnosticCategory.Error, key: "Decorators are not valid here." },
Decorators_cannot_be_applied_to_multiple_get_Slashset_accessors_of_the_same_name: { code: 1207, category: DiagnosticCategory.Error, key: "Decorators cannot be applied to multiple get/set accessors of the same name." },
Cannot_compile_non_external_modules_when_the_separateCompilation_flag_is_provided: { code: 1208, category: DiagnosticCategory.Error, key: "Cannot compile non-external modules when the '--separateCompilation' flag is provided." },
Ambient_const_enums_are_not_allowed_when_the_separateCompilation_flag_is_provided: { code: 1209, category: DiagnosticCategory.Error, key: "Ambient const enums are not allowed when the '--separateCompilation' flag is provided." },
Duplicate_identifier_0: { code: 2300, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'." },
Initializer_of_instance_member_variable_0_cannot_reference_identifier_1_declared_in_the_constructor: { code: 2301, category: DiagnosticCategory.Error, key: "Initializer of instance member variable '{0}' cannot reference identifier '{1}' declared in the constructor." },
Static_members_cannot_reference_class_type_parameters: { code: 2302, category: DiagnosticCategory.Error, key: "Static members cannot reference class type parameters." },
Expand Down Expand Up @@ -434,6 +436,11 @@ module ts {
Option_noEmit_cannot_be_specified_with_option_out_or_outDir: { code: 5040, category: DiagnosticCategory.Error, key: "Option 'noEmit' cannot be specified with option 'out' or 'outDir'." },
Option_noEmit_cannot_be_specified_with_option_declaration: { code: 5041, category: DiagnosticCategory.Error, key: "Option 'noEmit' cannot be specified with option 'declaration'." },
Option_project_cannot_be_mixed_with_source_files_on_a_command_line: { code: 5042, category: DiagnosticCategory.Error, key: "Option 'project' cannot be mixed with source files on a command line." },
Option_sourceMap_cannot_be_specified_with_option_separateCompilation: { code: 5043, category: DiagnosticCategory.Error, key: "Option 'sourceMap' cannot be specified with option 'separateCompilation'." },
Option_declaration_cannot_be_specified_with_option_separateCompilation: { code: 5044, category: DiagnosticCategory.Error, key: "Option 'declaration' cannot be specified with option 'separateCompilation'." },
Option_noEmitOnError_cannot_be_specified_with_option_separateCompilation: { code: 5045, category: DiagnosticCategory.Error, key: "Option 'noEmitOnError' cannot be specified with option 'separateCompilation'." },
Option_out_cannot_be_specified_with_option_separateCompilation: { code: 5046, category: DiagnosticCategory.Error, key: "Option 'out' cannot be specified with option 'separateCompilation'." },
Option_separateCompilation_can_only_be_used_when_either_option_module_is_provided_or_option_target_is_ES6_or_higher: { code: 5047, category: DiagnosticCategory.Error, key: "Option 'separateCompilation' can only be used when either option'--module' is provided or option 'target' is 'ES6' or higher." },
Concatenate_and_emit_output_to_single_file: { code: 6001, category: DiagnosticCategory.Message, key: "Concatenate and emit output to single file." },
Generates_corresponding_d_ts_file: { code: 6002, category: DiagnosticCategory.Message, key: "Generates corresponding '.d.ts' file." },
Specifies_the_location_where_debugger_should_locate_map_files_instead_of_generated_locations: { code: 6003, category: DiagnosticCategory.Message, key: "Specifies the location where debugger should locate map files instead of generated locations." },
Expand Down
29 changes: 28 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,14 @@
"category": "Error",
"code": 1207
},

"Cannot compile non-external modules when the '--separateCompilation' flag is provided.": {
"category": "Error",
"code": 1208
},
"Ambient const enums are not allowed when the '--separateCompilation' flag is provided.": {
"category": "Error",
"code": 1209
},
"Duplicate identifier '{0}'.": {
"category": "Error",
"code": 2300
Expand Down Expand Up @@ -1729,6 +1736,26 @@
"category": "Error",
"code": 5042
},
"Option 'sourceMap' cannot be specified with option 'separateCompilation'.": {
"category": "Error",
"code": 5043
},
"Option 'declaration' cannot be specified with option 'separateCompilation'.": {
"category": "Error",
"code": 5044
},
"Option 'noEmitOnError' cannot be specified with option 'separateCompilation'.": {
"category": "Error",
"code": 5045
},
"Option 'out' cannot be specified with option 'separateCompilation'.": {
"category": "Error",
"code": 5046
},
"Option 'separateCompilation' can only be used when either option'--module' is provided or option 'target' is 'ES6' or higher.": {
"category": "Error",
"code": 5047
},
"Concatenate and emit output to single file.": {
"category": "Message",
"code": 6001
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,11 @@ module ts {
}

function tryEmitConstantValue(node: PropertyAccessExpression | ElementAccessExpression): boolean {
if (compilerOptions.separateCompilation) {
// do not inline enum values in separate compilation mode
return false;
}

let constantValue = resolver.getConstantValue(node);
if (constantValue !== undefined) {
write(constantValue.toString());
Expand Down Expand Up @@ -3874,7 +3879,7 @@ module ts {

function shouldEmitEnumDeclaration(node: EnumDeclaration) {
let isConstEnum = isConst(node);
return !isConstEnum || compilerOptions.preserveConstEnums;
return !isConstEnum || compilerOptions.preserveConstEnums || compilerOptions.separateCompilation;
}

function emitEnumDeclaration(node: EnumDeclaration) {
Expand Down Expand Up @@ -3966,7 +3971,7 @@ module ts {
}

function shouldEmitModuleDeclaration(node: ModuleDeclaration) {
return isInstantiatedModule(node, compilerOptions.preserveConstEnums);
return isInstantiatedModule(node, compilerOptions.preserveConstEnums || compilerOptions.separateCompilation);
}

function emitModuleDeclaration(node: ModuleDeclaration) {
Expand Down
39 changes: 33 additions & 6 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,24 @@ module ts {
}

function verifyCompilerOptions() {
if (options.separateCompilation) {
if (options.sourceMap) {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Option_sourceMap_cannot_be_specified_with_option_separateCompilation));
}

if (options.declaration) {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Option_declaration_cannot_be_specified_with_option_separateCompilation));
}

if (options.noEmitOnError) {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Option_noEmitOnError_cannot_be_specified_with_option_separateCompilation));
}

if (options.out) {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Option_out_cannot_be_specified_with_option_separateCompilation));
}
}

if (!options.sourceMap && (options.mapRoot || options.sourceRoot)) {
// Error to specify --mapRoot or --sourceRoot without mapSourceFiles
if (options.mapRoot) {
Expand All @@ -468,24 +486,33 @@ module ts {
let languageVersion = options.target || ScriptTarget.ES3;

let firstExternalModuleSourceFile = forEach(files, f => isExternalModule(f) ? f : undefined);
if (firstExternalModuleSourceFile && !options.module) {
if (options.separateCompilation) {
if (!options.module && languageVersion < ScriptTarget.ES6) {
// We cannot use createDiagnosticFromNode because nodes do not have parents yet
let span = getErrorSpanForNode(firstExternalModuleSourceFile, firstExternalModuleSourceFile.externalModuleIndicator);
diagnostics.add(createFileDiagnostic(firstExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_compile_external_modules_unless_the_module_flag_is_provided));
diagnostics.add(createCompilerDiagnostic(Diagnostics.Option_separateCompilation_can_only_be_used_when_either_option_module_is_provided_or_option_target_is_ES6_or_higher));
}

let firstNonExternalModuleSourceFile = forEach(files, f => !isExternalModule(f) && !isDeclarationFile(f) ? f : undefined);
if (firstNonExternalModuleSourceFile) {
let span = getErrorSpanForNode(firstNonExternalModuleSourceFile, firstNonExternalModuleSourceFile);
diagnostics.add(createFileDiagnostic(firstNonExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_compile_non_external_modules_when_the_separateCompilation_flag_is_provided));
}
}
else if (firstExternalModuleSourceFile && languageVersion < ScriptTarget.ES6 && !options.module) {
// We cannot use createDiagnosticFromNode because nodes do not have parents yet
let span = getErrorSpanForNode(firstExternalModuleSourceFile, firstExternalModuleSourceFile.externalModuleIndicator);
diagnostics.add(createFileDiagnostic(firstExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_compile_external_modules_unless_the_module_flag_is_provided));
}

// Cannot specify module gen target when in es6 or above
if (options.module && languageVersion >= ScriptTarget.ES6) {
diagnostics.add(createCompilerDiagnostic(Diagnostics.Cannot_compile_external_modules_into_amd_or_commonjs_when_targeting_es6_or_higher));
}

// there has to be common source directory if user specified --outdir || --sourcRoot
// there has to be common source directory if user specified --outdir || --sourceRoot
// if user specified --mapRoot, there needs to be common source directory if there would be multiple files being emitted
if (options.outDir || // there is --outDir specified
options.sourceRoot || // there is --sourceRoot specified
(options.mapRoot && // there is --mapRoot Specified and there would be multiple js files generated
(options.mapRoot && // there is --mapRoot specified and there would be multiple js files generated
(!options.out || firstExternalModuleSourceFile !== undefined))) {

let commonPathComponents: string[];
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,7 @@ module ts {
target?: ScriptTarget;
version?: boolean;
watch?: boolean;
separateCompilation?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we need to make use of const enums from an ambient declaration an error while using this module, also consider using of internal modules on the global scope as an error..

also we need to make that available on the command line, as an experimental flag.

/* @internal */ stripInternal?: boolean;
[option: string]: string | number | boolean;
}
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,13 @@ module ts {
export function getErrorSpanForNode(sourceFile: SourceFile, node: Node): TextSpan {
let errorNode = node;
switch (node.kind) {
case SyntaxKind.SourceFile:
let pos = skipTrivia(sourceFile.text, 0, /*stopAfterLineBreak*/ false);
if (pos === sourceFile.text.length) {
// file is empty - return span for the beginning of the file
return createTextSpan(0, 0);
}
return getSpanOfTokenAtPosition(sourceFile, pos);
// This list is a work in progress. Add missing node kinds to improve their error
// spans.
case SyntaxKind.VariableDeclaration:
Expand Down
11 changes: 10 additions & 1 deletion src/harness/harness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,10 @@ module Harness {
options.preserveConstEnums = setting.value === 'true';
break;

case 'separatecompilation':
options.separateCompilation = setting.value === 'true';
break;

case 'suppressimplicitanyindexerrors':
options.suppressImplicitAnyIndexErrors = setting.value === 'true';
break;
Expand Down Expand Up @@ -1461,7 +1465,12 @@ module Harness {
var optionRegex = /^[\/]{2}\s*@(\w+)\s*:\s*(\S*)/gm; // multiple matches on multiple lines

// List of allowed metadata names
var fileMetadataNames = ["filename", "comments", "declaration", "module", "nolib", "sourcemap", "target", "out", "outdir", "noemitonerror", "noimplicitany", "noresolve", "newline", "newlines", "emitbom", "errortruncation", "usecasesensitivefilenames", "preserveconstenums", "includebuiltfile", "suppressimplicitanyindexerrors", "stripinternal"];
var fileMetadataNames = ["filename", "comments", "declaration", "module",
"nolib", "sourcemap", "target", "out", "outdir", "noemitonerror",
"noimplicitany", "noresolve", "newline", "newlines", "emitbom",
"errortruncation", "usecasesensitivefilenames", "preserveconstenums",
"includebuiltfile", "suppressimplicitanyindexerrors", "stripinternal",
"separatecompilation"];

function extractCompilerSettings(content: string): CompilerSetting[] {

Expand Down
Loading