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

[CFE] Issue when library and its part define the same variable #54836

Open
sgrekhov opened this issue Feb 6, 2024 · 2 comments
Open

[CFE] Issue when library and its part define the same variable #54836

sgrekhov opened this issue Feb 6, 2024 · 2 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-messages Poor/undesirable messaging in errors/warnings emitted by the CFE. type-question A question about expected behavior or functionality

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Feb 6, 2024

Consider the case when one part is used by two owners

// multiple_owners_part.dart
part of multiple_owners_lib;
final definedInPart = "Lily was here";

// multiple_owners_owner_1.dart
library multiple_owners_lib;
part 'multiple_owners_part.dart';

var definedInPart = "";

main() {
}

// multiple_owners_owner_2.dart
library multiple_owners_lib;
part 'multiple_owners_part.dart';

main() {
  print(definedInPart);
}

Now CFE report the following error for multiple_owners_owner_1.dart

$ out/ReleaseX64/dart tests/co19/src/Language/Libraries_and_Scripts/Parts/multiple_owners_owner_1.dart
tests/co19/src/Language/Libraries_and_Scripts/Parts/multiple_owners_part.dart:7:7: Error: 'definedInPart' is already declared in this scope.
final definedInPart = "Lily was here";
      ^^^^^^^^^^^^^
tests/co19/src/Language/Libraries_and_Scripts/Parts/multiple_owners_owner_1.dart:18:5: Context: Previous declaration of 'definedInPart'.
var definedInPart = "";
    ^^^^^^^^^^^^^

So, CFE reports that the error occurs in part. But if we take a look at multiple_owners_part.dart or at multiple_owners_owner_2.dart we won't see anything wrong in part itself or in another library that use it (multiple_owners_owner_2.dart). So, may be it makes sense to report that error occurs in multiple_owners_owner_1.dart, not in multiple_owners_part.dart?

@sgrekhov sgrekhov added area-front-end Use area-front-end for front end / CFE / kernel format related issues. type-question A question about expected behavior or functionality labels Feb 6, 2024
@eernstg
Copy link
Member

eernstg commented Feb 6, 2024

The wording 'Previous declaration' implies that the part file is considered to "come after" the library that owns this part. Another hint is that if we declare two local variables with the same name in the same function body then the error is reported for the one that occurs after the other one, even though it could just as well have been reported for the first one (or for both, except that this would probably not be a good User Experience ;-).

The syntax of a library and of a part file has a tiny amount of orderedness (for example, a library has imports before declarations, not the other way around), but it is in general possible to reorder top level declarations without changing the static analysis results and the semantics of the library.

I think this implies that it is undetermined whether the part declarations should be considered as occurring before or after the declarations of the owning library, and also in which order the part declarations are located in case we have multiple parts.

However, it would make sense to say that the part declarations are inserted into the owning library such that they are a replacement for the part directive. The parts would then be ordered the say way as their part directives, and their declarations would occur before the declarations of the owning library.

@johnniwinther, WDYT? Would this be a disruptive change or a small adjustment?

@johnniwinther
Copy link
Member

I don't think there is an inherent ordering to the conflicting members. Both locations are error points.

We could change to error to be reported on the member in the main library file but that wouldn't help in case of part vs part conflict.

The wording should be better, though, not using "previous" but probably just "other".

@johnniwinther johnniwinther added the cfe-messages Poor/undesirable messaging in errors/warnings emitted by the CFE. label Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-messages Poor/undesirable messaging in errors/warnings emitted by the CFE. type-question A question about expected behavior or functionality
Projects
None yet
Development

No branches or pull requests

3 participants