Skip to content

C++: Fix name clash in data flow imports#681

Merged
adityasharad merged 1 commit into
github:rc/1.19from
jbj:import-dataflow-clash
Dec 13, 2018
Merged

C++: Fix name clash in data flow imports#681
adityasharad merged 1 commit into
github:rc/1.19from
jbj:import-dataflow-clash

Conversation

@jbj

@jbj jbj commented Dec 13, 2018

Copy link
Copy Markdown
Contributor

The AST-based data flow libraries and the IR-based ones both define modules DataFlow, DataFlow2, etc. This caused ImportAdditionalLibraries.ql to fail in compilation.

The AST-based data flow libraries and the IR-based ones both define
modules `DataFlow`, `DataFlow2`, etc. This caused
`ImportAdditionalLibraries.ql` to fail in compilation.
@jbj jbj added the C++ label Dec 13, 2018
@jbj jbj added this to the 1.19 milestone Dec 13, 2018
@jbj jbj requested a review from a team as a code owner December 13, 2018 09:56
@geoffw0

geoffw0 commented Dec 13, 2018

Copy link
Copy Markdown
Contributor

The description of ImportAdditionalLibraries.ql says it "imports some libraries we would like to make available in the LGTM query console even if they are not used by any queries." Does the module statement around the import affect the way LGTM users access these additional libraries?

@jbj

jbj commented Dec 13, 2018

Copy link
Copy Markdown
Contributor Author

@geoffw0 ImportAdditionalLibraries.ql is only there to introduce file-level dependencies on otherwise unused qll files. No names in it are visible to anything else.

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@adityasharad adityasharad merged commit fb82811 into github:rc/1.19 Dec 13, 2018
@jbj

jbj commented Dec 13, 2018

Copy link
Copy Markdown
Contributor Author

Cc @dave-bartolomeo. I've opened https://git.semmle.com/Semmle/code/pull/29761 to prevent this from happening again.

cklin pushed a commit that referenced this pull request May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants