Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Show macro expansions for undeclared identifiers #459

Open
hdamron17 opened this issue May 27, 2020 · 5 comments
Open

Show macro expansions for undeclared identifiers #459

hdamron17 opened this issue May 27, 2020 · 5 comments
Labels
enhancement New feature or request hard Extra attention is needed preprocessor Issue in the preprocessor (probably cycle detection) ui Helpful for user experience when seeing error or warnings

Comments

@hdamron17
Copy link
Collaborator

When an undeclared identifier is found, it would be helpful to show if and where it is expanded from a macro so the user can track it down.

Code

#define y z
#define x y
int main() {
  return x;
}

Expected Result (clang)

evil-test.c:4:10: error: use of undeclared identifier 'z'
  return x;
         ^
evil-test.c:2:11: note: expanded from macro 'x'
#define x y
          ^
evil-test.c:1:11: note: expanded from macro 'y'
#define y z
          ^
1 error generated.

Actual Error

evil-test.c:4:10 error: invalid program: use of undeclared identifier 'z'
  return x;
         ^
1 error generated
@hdamron17 hdamron17 added enhancement New feature or request ui Helpful for user experience when seeing error or warnings preprocessor Issue in the preprocessor (probably cycle detection) labels May 27, 2020
@jyn514
Copy link
Owner

jyn514 commented May 27, 2020

This is a good idea, but it's going to be really hard. Locations will have to be expanded to keep track of each macro along the way, which means they can no longer be Copy and there will suddenly be a lot of lifetime errors.

Maybe there should be some sort of lookup table for locations so that they don't have to be passed along with each struct? I don't think HashMap<dyn Hash, Location> is a thing, though.

@hdamron17 hdamron17 added the hard Extra attention is needed label May 27, 2020
@hdamron17
Copy link
Collaborator Author

The Locations of #defines are stored in a map somewhere already, I assume? So could we instead add an Option<InternedStr> member to Token::Id which points to where it which macro replaced into it? Then if there's an error, we can do a recursive lookup until None.

@hdamron17
Copy link
Collaborator Author

Or if the Locations of #defines aren't already stored somewhere, then lex::replace::Definitions could be changed from

pub type Definitions = HashMap<InternedStr, Definition>;

to

pub type Definitions = HashMap<InternedStr, (Definition, Location)>;

Not sure if this will mess with lifetimes.

@jyn514
Copy link
Owner

jyn514 commented May 27, 2020

@hdamron17 sure, making it a HashMap<InternedStr, Locatable<Definition>> isn't hard. The hard part is tracing back from the replaced version to the original. By the time this error occurs in the parser, all context about macros has been lost, there's only tokens left and you can't tell if they originated from a macro or not.

@jyn514
Copy link
Owner

jyn514 commented May 27, 2020

I guess a linked list isn't the worst thing since most replacements shouldn't be highly nested. I'm a little concerned about memory usage but we can always worry about performance later.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request hard Extra attention is needed preprocessor Issue in the preprocessor (probably cycle detection) ui Helpful for user experience when seeing error or warnings
Projects
None yet
Development

No branches or pull requests

2 participants